From 188a15c6786087d9be2b0fd92ef05d73b3e8fb27 Mon Sep 17 00:00:00 2001 From: vmiura Date: Thu, 25 Dec 2014 15:11:14 -0800 Subject: [PATCH] cc: TextureUploader - Reset GL_UNPACK_ALIGNMENT after Gpu Raster. TextureUploader assumes GL_UNPACK_ALIGNMENT is 4 for performance reasons however Ganesh may modify GL_UNPACK_ALIGNMENT. Explicitly reset GL_UNPACK_ALIGNMENT to 4 after Gpu Raster. BUG=442966 Review URL: https://codereview.chromium.org/817673004 Cr-Commit-Position: refs/heads/master@{#309653} --- cc/BUILD.gn | 1 + cc/cc_tests.gyp | 1 + cc/output/gl_renderer.cc | 23 ++++++++++-------- cc/resources/scoped_gpu_raster.cc | 7 ++++-- cc/resources/scoped_gpu_raster_unittest.cc | 39 ++++++++++++++++++++++++++++++ cc/test/test_gles2_interface.cc | 4 +++ cc/test/test_gles2_interface.h | 2 ++ cc/test/test_web_graphics_context_3d.cc | 25 +++++++++++++++++++ cc/test/test_web_graphics_context_3d.h | 2 ++ 9 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 cc/resources/scoped_gpu_raster_unittest.cc diff --git a/cc/BUILD.gn b/cc/BUILD.gn index 5795f90ddbcf..7ae0b29f78f1 100644 --- a/cc/BUILD.gn +++ b/cc/BUILD.gn @@ -784,6 +784,7 @@ test("cc_unittests") { "resources/tile_task_worker_pool_unittest.cc", "resources/resource_provider_unittest.cc", "resources/resource_update_controller_unittest.cc", + "resources/scoped_gpu_raster_unittest.cc", "resources/scoped_resource_unittest.cc", "resources/task_graph_runner_unittest.cc", "resources/texture_mailbox_deleter_unittest.cc", diff --git a/cc/cc_tests.gyp b/cc/cc_tests.gyp index 181af37d1b2a..73c72a845dae 100644 --- a/cc/cc_tests.gyp +++ b/cc/cc_tests.gyp @@ -85,6 +85,7 @@ 'resources/tile_task_worker_pool_unittest.cc', 'resources/resource_provider_unittest.cc', 'resources/resource_update_controller_unittest.cc', + 'resources/scoped_gpu_raster_unittest.cc', 'resources/scoped_resource_unittest.cc', 'resources/task_graph_runner_unittest.cc', 'resources/texture_mailbox_deleter_unittest.cc', diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc index 2c1e3c875344..9c856e86786c 100644 --- a/cc/output/gl_renderer.cc +++ b/cc/output/gl_renderer.cc @@ -27,6 +27,7 @@ #include "cc/quads/stream_video_draw_quad.h" #include "cc/quads/texture_draw_quad.h" #include "cc/resources/layer_quad.h" +#include "cc/resources/scoped_gpu_raster.h" #include "cc/resources/scoped_resource.h" #include "cc/resources/texture_mailbox_deleter.h" #include "gpu/GLES2/gl2extchromium.h" @@ -158,7 +159,12 @@ class GLRenderer::ScopedUseGrContext { return make_scoped_ptr(new ScopedUseGrContext(renderer, frame)); } - ~ScopedUseGrContext() { PassControlToGLRenderer(); } + ~ScopedUseGrContext() { + // Pass context control back to GLrenderer. + scoped_gpu_raster_ = nullptr; + renderer_->RestoreGLState(); + renderer_->RestoreFramebuffer(frame_); + } GrContext* context() const { return renderer_->output_surface_->context_provider()->GrContext(); @@ -166,17 +172,14 @@ class GLRenderer::ScopedUseGrContext { private: ScopedUseGrContext(GLRenderer* renderer, DrawingFrame* frame) - : renderer_(renderer), frame_(frame) { - PassControlToSkia(); - } - - void PassControlToSkia() { context()->resetContext(); } - - void PassControlToGLRenderer() { - renderer_->RestoreGLState(); - renderer_->RestoreFramebuffer(frame_); + : scoped_gpu_raster_( + new ScopedGpuRaster(renderer->output_surface_->context_provider())), + renderer_(renderer), + frame_(frame) { + // scoped_gpu_raster_ passes context control to Skia. } + scoped_ptr scoped_gpu_raster_; GLRenderer* renderer_; DrawingFrame* frame_; diff --git a/cc/resources/scoped_gpu_raster.cc b/cc/resources/scoped_gpu_raster.cc index 9d1dfe53dc60..2f5c5c9c3426 100644 --- a/cc/resources/scoped_gpu_raster.cc +++ b/cc/resources/scoped_gpu_raster.cc @@ -34,11 +34,14 @@ void ScopedGpuRaster::BeginGpuRaster() { } void ScopedGpuRaster::EndGpuRaster() { - GLES2Interface* gl = context_provider_->ContextGL(); - class GrContext* gr_context = context_provider_->GrContext(); gr_context->flush(); + GLES2Interface* gl = context_provider_->ContextGL(); + + // Restore default GL unpack alignment. TextureUploader expects this. + gl->PixelStorei(GL_UNPACK_ALIGNMENT, 4); + // TODO(alokp): Use a trace macro to push/pop markers. // Using push/pop functions directly incurs cost to evaluate function // arguments even when tracing is disabled. diff --git a/cc/resources/scoped_gpu_raster_unittest.cc b/cc/resources/scoped_gpu_raster_unittest.cc new file mode 100644 index 000000000000..b1f60a3c29b0 --- /dev/null +++ b/cc/resources/scoped_gpu_raster_unittest.cc @@ -0,0 +1,39 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "cc/resources/scoped_gpu_raster.h" +#include "cc/test/test_context_provider.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace cc { +namespace { + +class ScopedGpuRasterTest : public testing::Test { + public: + ScopedGpuRasterTest() {} +}; + +// Releasing ScopedGpuRaster should restore GL_UNPACK_ALIGNMENT == 4. +TEST(ScopedGpuRasterTest, RestoresUnpackAlignment) { + scoped_refptr provider = TestContextProvider::Create(); + EXPECT_TRUE(provider->BindToCurrentThread()); + gpu::gles2::GLES2Interface* gl = provider->ContextGL(); + GLint unpack_alignment = 0; + gl->GetIntegerv(GL_UNPACK_ALIGNMENT, &unpack_alignment); + EXPECT_EQ(4, unpack_alignment); + + { + scoped_ptr scoped_gpu_raster( + new ScopedGpuRaster(provider.get())); + gl->PixelStorei(GL_UNPACK_ALIGNMENT, 1); + gl->GetIntegerv(GL_UNPACK_ALIGNMENT, &unpack_alignment); + EXPECT_EQ(1, unpack_alignment); + } + + gl->GetIntegerv(GL_UNPACK_ALIGNMENT, &unpack_alignment); + EXPECT_EQ(4, unpack_alignment); +} + +} // namespace +} // namespace cc diff --git a/cc/test/test_gles2_interface.cc b/cc/test/test_gles2_interface.cc index 0a29fbee116f..38ada9883d6b 100644 --- a/cc/test/test_gles2_interface.cc +++ b/cc/test/test_gles2_interface.cc @@ -178,6 +178,10 @@ void TestGLES2Interface::BindBuffer(GLenum target, GLuint buffer) { test_context_->bindBuffer(target, buffer); } +void TestGLES2Interface::PixelStorei(GLenum pname, GLint param) { + test_context_->pixelStorei(pname, param); +} + void TestGLES2Interface::TexImage2D(GLenum target, GLint level, GLint internalformat, diff --git a/cc/test/test_gles2_interface.h b/cc/test/test_gles2_interface.h index a12147a639d5..3a174afd06b6 100644 --- a/cc/test/test_gles2_interface.h +++ b/cc/test/test_gles2_interface.h @@ -64,6 +64,8 @@ class TestGLES2Interface : public gpu::gles2::GLES2InterfaceStub { void BindRenderbuffer(GLenum target, GLuint buffer) override; void BindFramebuffer(GLenum target, GLuint buffer) override; + void PixelStorei(GLenum pname, GLint param) override; + void TexImage2D(GLenum target, GLint level, GLint internalformat, diff --git a/cc/test/test_web_graphics_context_3d.cc b/cc/test/test_web_graphics_context_3d.cc index 9fae995ec6f8..2d82f89a71cd 100644 --- a/cc/test/test_web_graphics_context_3d.cc +++ b/cc/test/test_web_graphics_context_3d.cc @@ -67,6 +67,7 @@ TestWebGraphicsContext3D::TestWebGraphicsContext3D() last_update_type_(NoUpdate), next_insert_sync_point_(1), last_waited_sync_point_(0), + unpack_alignment_(4), bound_buffer_(0), weak_ptr_factory_(this) { CreateNamespace(); @@ -367,6 +368,8 @@ void TestWebGraphicsContext3D::getIntegerv( *value = max_texture_size_; else if (pname == GL_ACTIVE_TEXTURE) *value = GL_TEXTURE0; + else if (pname == GL_UNPACK_ALIGNMENT) + *value = unpack_alignment_; } void TestWebGraphicsContext3D::getProgramiv(GLuint program, @@ -518,6 +521,28 @@ void TestWebGraphicsContext3D::bufferData(GLenum target, current_used_transfer_buffer_usage_bytes_); } +void TestWebGraphicsContext3D::pixelStorei(GLenum pname, GLint param) { + switch (pname) { + case GL_UNPACK_ALIGNMENT: + // Param should be a power of two <= 8. + EXPECT_EQ(0, param & (param - 1)); + EXPECT_GE(8, param); + switch (param) { + case 1: + case 2: + case 4: + case 8: + unpack_alignment_ = param; + break; + default: + break; + } + break; + default: + break; + } +} + void* TestWebGraphicsContext3D::mapBufferCHROMIUM(GLenum target, GLenum access) { base::AutoLock lock(namespace_->lock); diff --git a/cc/test/test_web_graphics_context_3d.h b/cc/test/test_web_graphics_context_3d.h index 56c17c097a2c..0c001d2e5276 100644 --- a/cc/test/test_web_graphics_context_3d.h +++ b/cc/test/test_web_graphics_context_3d.h @@ -248,6 +248,7 @@ class TestWebGraphicsContext3D { GLsizeiptr size, const void* data, GLenum usage); + virtual void pixelStorei(GLenum pname, GLint param); virtual void* mapBufferCHROMIUM(GLenum target, GLenum access); virtual GLboolean unmapBufferCHROMIUM(GLenum target); @@ -463,6 +464,7 @@ class TestWebGraphicsContext3D { UpdateType last_update_type_; unsigned next_insert_sync_point_; unsigned last_waited_sync_point_; + int unpack_alignment_; unsigned bound_buffer_; TextureTargets texture_targets_; -- 2.11.4.GIT