diff options
author | jmadill <jmadill@chromium.org> | 2015-08-17 07:20:47 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-17 14:21:26 +0000 |
commit | a814ea088d8c8f85485b3588996605767081cd79 (patch) | |
tree | 2866f49edbd4ed3193c0dd75f0812d54fa569a5e /gpu | |
parent | e5e7aa9323e8391a36a2e2a624078279362f62c6 (diff) | |
download | chromium_src-a814ea088d8c8f85485b3588996605767081cd79.zip chromium_src-a814ea088d8c8f85485b3588996605767081cd79.tar.gz chromium_src-a814ea088d8c8f85485b3588996605767081cd79.tar.bz2 |
Revert of gpu: workaround force_cube_map_positive_x_allocation fixes Android crash. (patchset #10 id:180001 of https://codereview.chromium.org/1280163004/ )
Reason for revert:
Causing failures on the Linux AMD GPU FYI bot.
[ RUN ] GLCubeMapTextureTests/GLCubeMapTextureTest.ReadPixelsFromIncompleteFBO/5
Received signal 11 SEGV_MAPERR 000000000058
#0 0x000000486a35 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7fe9b624ecb0 <unknown>
#2 0x7fe9b31f6b9e <unknown>
#3 0x7fe9b3209948 <unknown>
#4 0x7fe9b31d9d02 <unknown>
#5 0x7fe9b31d426e <unknown>
#6 0x7fe9b2fec29b <unknown>
#7 0x7fe9b268badd <unknown>
#8 0x0000005138c6 gpu::gles2::Framebuffer::GetStatus()
#9 0x000000537936 gpu::gles2::GLES2DecoderImpl::CheckFramebufferValid()
Original issue's description:
> gpu: workaround force_cube_map_positive_x_allocation fixes Android crash.
>
> Adreno Android (e.g. Nexus5) crashes on allocating a cube map texture bound to FBO, if
> the CUBE_MAP_POSITIVE_X texture is not allocated yet. The Workaround forces to allocate
> the CUBE_MAP_POSITIVE_X texture.
>
> Add new unittests to prevent regression. ANGLE crashes on GLCubeMapTextureTest.ReadPixels
> It's will be fixed in ANGLE project.
>
> TEST=gl_tests.GLCubeMapTextureTests.*
> BUG=518889
>
> Committed: https://crrev.com/1eebeac3e05c2035139c04226b34cef745cfd822
> Cr-Commit-Position: refs/heads/master@{#343600}
TBR=piman@chromium.org,zmo@google.com,kbr@chromium.org,sievers@chromium.org,dongseong.hwang@intel.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=518889
Review URL: https://codereview.chromium.org/1293123002
Cr-Commit-Position: refs/heads/master@{#343676}
Diffstat (limited to 'gpu')
-rw-r--r-- | gpu/BUILD.gn | 1 | ||||
-rw-r--r-- | gpu/command_buffer/service/gles2_cmd_decoder.cc | 4 | ||||
-rw-r--r-- | gpu/command_buffer/service/texture_manager.cc | 50 | ||||
-rw-r--r-- | gpu/command_buffer/service/texture_manager.h | 17 | ||||
-rw-r--r-- | gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc | 138 | ||||
-rw-r--r-- | gpu/config/gpu_driver_bug_list_json.cc | 14 | ||||
-rw-r--r-- | gpu/config/gpu_driver_bug_workaround_type.h | 2 | ||||
-rw-r--r-- | gpu/gpu.gyp | 1 |
8 files changed, 6 insertions, 221 deletions
diff --git a/gpu/BUILD.gn b/gpu/BUILD.gn index 3d1f3f8..1f7f5fe 100644 --- a/gpu/BUILD.gn +++ b/gpu/BUILD.gn @@ -85,7 +85,6 @@ test("gl_tests") { "command_buffer/tests/gl_clear_framebuffer_unittest.cc", "command_buffer/tests/gl_compressed_copy_texture_CHROMIUM_unittest.cc", "command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc", - "command_buffer/tests/gl_cube_map_texture_unittest.cc", "command_buffer/tests/gl_depth_texture_unittest.cc", "command_buffer/tests/gl_gpu_memory_buffer_unittest.cc", "command_buffer/tests/gl_lose_context_chromium_unittest.cc", diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc index 2de670a..db91adf 100644 --- a/gpu/command_buffer/service/gles2_cmd_decoder.cc +++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc @@ -2577,7 +2577,9 @@ GLES2DecoderImpl::GLES2DecoderImpl(ContextGroup* group) : false), viewport_max_width_(0), viewport_max_height_(0), - texture_state_(group_->feature_info()->workarounds()), + texture_state_(group_->feature_info() + ->workarounds() + .texsubimage_faster_than_teximage), cb_command_trace_category_(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED( TRACE_DISABLED_BY_DEFAULT("cb_command"))), gpu_decoder_category_(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED( diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc index 53715c4..ed0fd10 100644 --- a/gpu/command_buffer/service/texture_manager.cc +++ b/gpu/command_buffer/service/texture_manager.cc @@ -1939,14 +1939,6 @@ void TextureManager::ValidateAndDoTexImage( return; } - if (texture_state->force_cube_map_positive_x_allocation) { - if (!DoTexImage2DCubeMapPositiveXIfNeeded( - texture_state, state->GetErrorState(), framebuffer_state, - function_name, texture_ref, args)) { - return; - } - } - DoTexImage(texture_state, state->GetErrorState(), framebuffer_state, function_name, texture_ref, args); } @@ -1963,48 +1955,6 @@ GLenum TextureManager::AdjustTexFormat(GLenum format) const { return format; } -// Nexus 5 crashes on allocating a cube map texture bound to FBO, if -// the CUBE_MAP_POSITIVE_X texture is not allocated yet. -bool TextureManager::DoTexImage2DCubeMapPositiveXIfNeeded( - DecoderTextureState* texture_state, - ErrorState* error_state, - DecoderFramebufferState* framebuffer_state, - const char* function_name, - TextureRef* texture_ref, - const DoTexImageArguments& args) { - switch (args.target) { - case GL_TEXTURE_CUBE_MAP_NEGATIVE_X: - case GL_TEXTURE_CUBE_MAP_POSITIVE_Y: - case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y: - case GL_TEXTURE_CUBE_MAP_POSITIVE_Z: - case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z: - break; - default: - return true; - } - - // ValidateTexImage is passed already. - Texture* texture = texture_ref->texture(); - int width = 0; - int height = 0; - bool positive_x_level_defined = texture->GetLevelSize( - GL_TEXTURE_CUBE_MAP_POSITIVE_X, args.level, &width, &height, nullptr); - if (!positive_x_level_defined) { - DoTexImageArguments positive_x_args = args; - positive_x_args.target = GL_TEXTURE_CUBE_MAP_POSITIVE_X; - positive_x_args.pixels = nullptr; - if (!memory_tracker_managed_->EnsureGPUMemoryAvailable(2 * - args.pixels_size)) { - ERRORSTATE_SET_GL_ERROR(error_state, GL_OUT_OF_MEMORY, function_name, - "out of memory"); - return false; - } - DoTexImage(texture_state, error_state, framebuffer_state, function_name, - texture_ref, positive_x_args); - } - return true; -} - void TextureManager::DoTexImage( DecoderTextureState* texture_state, ErrorState* error_state, diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h index 49571d1..e97feb3 100644 --- a/gpu/command_buffer/service/texture_manager.h +++ b/gpu/command_buffer/service/texture_manager.h @@ -14,7 +14,6 @@ #include "base/containers/hash_tables.h" #include "base/memory/ref_counted.h" #include "gpu/command_buffer/service/async_pixel_transfer_delegate.h" -#include "gpu/command_buffer/service/feature_info.h" #include "gpu/command_buffer/service/gl_utils.h" #include "gpu/command_buffer/service/memory_tracking.h" #include "gpu/gpu_export.h" @@ -533,13 +532,10 @@ class GPU_EXPORT TextureRef : public base::RefCounted<TextureRef> { struct DecoderTextureState { // total_texture_upload_time automatically initialized to 0 in default // constructor. - explicit DecoderTextureState(const FeatureInfo::Workarounds& workarounds) + explicit DecoderTextureState(bool texsubimage_faster_than_teximage) : tex_image_failed(false), texture_upload_count(0), - texsubimage_faster_than_teximage( - workarounds.texsubimage_faster_than_teximage), - force_cube_map_positive_x_allocation( - workarounds.force_cube_map_positive_x_allocation) {} + texsubimage_faster_than_teximage(texsubimage_faster_than_teximage) {} // This indicates all the following texSubImage*D calls that are part of the // failed texImage*D call should be ignored. The client calls have a lock @@ -552,7 +548,6 @@ struct DecoderTextureState { base::TimeDelta total_texture_upload_time; bool texsubimage_faster_than_teximage; - bool force_cube_map_positive_x_allocation; }; // This class keeps track of the textures and their sizes so we can do NPOT and @@ -893,14 +888,6 @@ class GPU_EXPORT TextureManager : public base::trace_event::MemoryDumpProvider { TextureRef* texture_ref, const DoTexImageArguments& args); - bool DoTexImage2DCubeMapPositiveXIfNeeded( - DecoderTextureState* texture_state, - ErrorState* error_state, - DecoderFramebufferState* framebuffer_state, - const char* function_name, - TextureRef* texture_ref, - const DoTexImageArguments& args); - void StartTracking(TextureRef* texture); void StopTracking(TextureRef* texture); diff --git a/gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc b/gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc deleted file mode 100644 index 7644a82..0000000 --- a/gpu/command_buffer/tests/gl_cube_map_texture_unittest.cc +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2015 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 <GLES2/gl2.h> - -#include "gpu/command_buffer/tests/gl_manager.h" -#include "gpu/command_buffer/tests/gl_test_utils.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace gpu { - -namespace { -const GLenum kCubeMapTextureTargets[] = { - GL_TEXTURE_CUBE_MAP_POSITIVE_X, GL_TEXTURE_CUBE_MAP_NEGATIVE_X, - GL_TEXTURE_CUBE_MAP_POSITIVE_Y, GL_TEXTURE_CUBE_MAP_NEGATIVE_Y, - GL_TEXTURE_CUBE_MAP_POSITIVE_Z, GL_TEXTURE_CUBE_MAP_NEGATIVE_Z, -}; -} // namespace - -// A collection of tests that exercise the cube map texture. -class GLCubeMapTextureTest : public testing::TestWithParam<GLenum> { - protected: - void SetUp() override { - gl_.Initialize(GLManager::Options()); - for (int i = 0; i < 256; i++) { - pixels_[i * 4] = 255u; - pixels_[(i * 4) + 1] = 0; - pixels_[(i * 4) + 2] = 0; - pixels_[(i * 4) + 3] = 255u; - } - - glGenTextures(1, &texture_); - glBindTexture(GL_TEXTURE_CUBE_MAP, texture_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); - glTexParameterf(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); - glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - glGenFramebuffers(1, &framebuffer_id_); - } - - void TearDown() override { - glDeleteTextures(1, &texture_); - glDeleteFramebuffers(1, &framebuffer_id_); - gl_.Destroy(); - } - - GLManager gl_; - uint8 pixels_[256 * 4]; - const int width_ = 16; - GLuint texture_; - GLuint framebuffer_id_; -}; - -INSTANTIATE_TEST_CASE_P(GLCubeMapTextureTests, - GLCubeMapTextureTest, - ::testing::ValuesIn(kCubeMapTextureTargets)); - -TEST_P(GLCubeMapTextureTest, TexImage2DAfterFBOBinding) { - GLenum cube_map_target = GetParam(); - - glBindFramebuffer(GL_FRAMEBUFFER, framebuffer_id_); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, cube_map_target, - texture_, 0); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - glBindTexture(GL_TEXTURE_CUBE_MAP, texture_); - // force_gl_finish_after_compositing workaround prevents Nexus 5 crash. - // TODO(dshwang): remove the workaround when it's fixed. crbug.com/518889 - glTexImage2D(cube_map_target, 0, GL_RGBA, width_, width_, 0, GL_RGBA, - GL_UNSIGNED_BYTE, pixels_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); -} - -TEST_P(GLCubeMapTextureTest, ReadPixels) { - GLenum cube_map_target = GetParam(); - - glBindTexture(GL_TEXTURE_CUBE_MAP, texture_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - // Make a cube texture complete - for (unsigned i = 0; i < arraysize(kCubeMapTextureTargets); i++) { - glTexImage2D(kCubeMapTextureTargets[i], 0, GL_RGBA, width_, width_, 0, - GL_RGBA, GL_UNSIGNED_BYTE, pixels_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - } - - glBindFramebuffer(GL_FRAMEBUFFER, framebuffer_id_); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, cube_map_target, - texture_, 0); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - // Check that FB is complete. - EXPECT_EQ(static_cast<GLenum>(GL_FRAMEBUFFER_COMPLETE), - glCheckFramebufferStatus(GL_FRAMEBUFFER)); - - GLTestHelper::CheckPixels(0, 0, width_, width_, 0, pixels_); - EXPECT_TRUE(GL_NO_ERROR == glGetError()); -} - -#if defined(OS_WIN) -// TODO(dshwang): ANGLE crashes. crbug.com/518889 -TEST_P(GLCubeMapTextureTest, DISABLED_ReadPixelsFromIncompleteFBO) { -#else -TEST_P(GLCubeMapTextureTest, ReadPixelsFromIncompleteFBO) { -#endif - GLenum cube_map_target = GetParam(); - - glBindTexture(GL_TEXTURE_CUBE_MAP, texture_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - glTexImage2D(cube_map_target, 0, GL_RGBA, width_, width_, 0, GL_RGBA, - GL_UNSIGNED_BYTE, pixels_); - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - glBindFramebuffer(GL_FRAMEBUFFER, framebuffer_id_); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, cube_map_target, - texture_, 0); - - // force_gl_finish_after_compositing workaround prevents Nexus 5 crash. - // TODO(dshwang): remove the workaround when it's fixed. crbug.com/518889 - EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError()); - - GLsizei size = width_ * width_ * 4; - scoped_ptr<uint8[]> pixels(new uint8[size]); - // ANGLE crashes on glReadPixels() from incomplete FBO. - glReadPixels(0, 0, width_, width_, GL_RGBA, GL_UNSIGNED_BYTE, pixels.get()); - // TODO(dshwang): According to spec, above glReadPixels() should fail and - // glGetError() should report an error, but some drivers are fine to bind - // incomplete cube map texture to FBO. crbug.com/517548 - glGetError(); -} - -} // namespace gpu diff --git a/gpu/config/gpu_driver_bug_list_json.cc b/gpu/config/gpu_driver_bug_list_json.cc index 3b24cdc..ca8fcc1 100644 --- a/gpu/config/gpu_driver_bug_list_json.cc +++ b/gpu/config/gpu_driver_bug_list_json.cc @@ -19,7 +19,7 @@ const char kGpuDriverBugListJson[] = LONG_STRING_CONST( { "name": "gpu driver bug list", // Please update the version number whenever you change this file. - "version": "8.23", + "version": "8.22", "entries": [ { "id": 1, @@ -1481,18 +1481,6 @@ LONG_STRING_CONST( "features": [ "disable_program_cache" ] - }, - { - "id": 127, - "description": "Crash on binding cube map texture to FBO", - "cr_bugs": [518889], - "os": { - "type": "android" - }, - "gl_renderer": "Adreno.*", - "features": [ - "force_cube_map_positive_x_allocation" - ] } ] } diff --git a/gpu/config/gpu_driver_bug_workaround_type.h b/gpu/config/gpu_driver_bug_workaround_type.h index c657e84..418391c 100644 --- a/gpu/config/gpu_driver_bug_workaround_type.h +++ b/gpu/config/gpu_driver_bug_workaround_type.h @@ -52,8 +52,6 @@ etc1_power_of_two_only) \ GPU_OP(EXIT_ON_CONTEXT_LOST, \ exit_on_context_lost) \ - GPU_OP(FORCE_CUBE_MAP_POSITIVE_X_ALLOCATION, \ - force_cube_map_positive_x_allocation) \ GPU_OP(FORCE_DISCRETE_GPU, \ force_discrete_gpu) \ GPU_OP(FORCE_GL_FINISH_AFTER_COMPOSITING, \ diff --git a/gpu/gpu.gyp b/gpu/gpu.gyp index 2a98fd9..bffaf94 100644 --- a/gpu/gpu.gyp +++ b/gpu/gpu.gyp @@ -362,7 +362,6 @@ 'command_buffer/tests/gl_clear_framebuffer_unittest.cc', 'command_buffer/tests/gl_compressed_copy_texture_CHROMIUM_unittest.cc', 'command_buffer/tests/gl_copy_texture_CHROMIUM_unittest.cc', - 'command_buffer/tests/gl_cube_map_texture_unittest.cc', 'command_buffer/tests/gl_depth_texture_unittest.cc', 'command_buffer/tests/gl_gpu_memory_buffer_unittest.cc', 'command_buffer/tests/gl_lose_context_chromium_unittest.cc', |