SkColorType SkSurface creation change broke oop raster |
||
Issue descriptionAfter https://chromium-review.googlesource.com/889861 oop raster (--enable-gpu-rasterization --enable-oop-rasterization) no longer works. However, tests still appear to work. When running with these flags after this patch, this spew happens indefinitely: [162769:162769:0206/133040.518847:ERROR:gles2_cmd_decoder.cc(20453)] [.RenderWorker-0x3e12c356ec20]GL ERROR :GL_INVALID_OPERATION : glBeginRasterCHROMIUM: failed to create surface [162769:162769:0206/133040.519042:ERROR:gles2_cmd_decoder.cc(20487)] [.RenderWorker-0x3e12c356ec20]GL ERROR :GL_INVALID_OPERATION : glRasterCHROMIUM: RasterCHROMIUM without BeginRasterCHROMIUM [162769:162769:0206/133040.519261:ERROR:gles2_cmd_decoder.cc(20527)] [.RenderWorker-0x3e12c356ec20]GL ERROR :GL_INVALID_OPERATION : glBeginRasterCHROMIUM: EndRasterCHROMIUM without BeginRasterCHROMIUM
,
Feb 6 2018
I see. Is the texture format really GL_BGRA? I thought that wasn't a valid internal format in non-ES GL.
,
Feb 6 2018
Yes, that's what it's asking for: https://cs.chromium.org/chromium/src/components/viz/common/resources/platform_color.h?sq=package:chromium&l=24 is the code that picks the format. This works out for gpu raster on tot because it's behind the gles command buffer, but when creating surfaces in the gpu service for oop raster, then it's talking to gl directly. Does validate_sized_format need to consider extensions?
,
Feb 7 2018
That function supposed to allow anything that is present in GL/GLES or could be by an extension (recognized by Ganesh). However, AFAIK there is no extension to non-ES GL that adds a GL_BGRA base internal format (or GL_BGRA8 sized internal format). There is GL_EXT_bgra but it just adds GL_BGRA as a <format> param to calls like glTexImage where it refers to the CPU data in calls that transfer pixels in/out of GL objects. It would typically be used when creating a texture like this: glTexImage2D(GL_TEXTURE_2D, 0, /* internalFormat = */ GL_RGBA, w, h, 0, /* format = */ GL_BGRA, GL_UNSIGNED_BYTE, ptrToCPUData);
,
Feb 7 2018
I made a change that will specifies the GrGLTextureInfo's fFormat field as GL_RGBA8 when the context is not ES. It seems to fix the issue on my Linux machine.
,
Feb 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/432fea88a41d74b9848eff7a43ce074df2d166d9 commit 432fea88a41d74b9848eff7a43ce074df2d166d9 Author: Brian Salomon <bsalomon@google.com> Date: Thu Feb 15 18:35:45 2018 Specify OOP raster SkSurface's format as GL_RGBA8 on desktop GL when SkColorType is kBGRA_8888 Bug: 809692 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I3796906eb7bcb64a919591c1e05f97a2ff7e44ee Reviewed-on: https://chromium-review.googlesource.com/905484 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Brian Salomon <bsalomon@chromium.org> Cr-Commit-Position: refs/heads/master@{#537083} [modify] https://crrev.com/432fea88a41d74b9848eff7a43ce074df2d166d9/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/432fea88a41d74b9848eff7a43ce074df2d166d9/ui/gl/gl_gl_api_implementation.cc [modify] https://crrev.com/432fea88a41d74b9848eff7a43ce074df2d166d9/ui/gl/gl_gl_api_implementation.h
,
Feb 15 2018
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/848aaee2c40cd2c5dfa416c1dacebb44f4b6fa2e commit 848aaee2c40cd2c5dfa416c1dacebb44f4b6fa2e Author: Khushal <khushalsagar@chromium.org> Date: Wed Feb 21 01:10:29 2018 content: Add basic integration test for OOP raster. R=enne@chromium.org Bug: 809692 Change-Id: Ib22f6f452046d9989ec9a1ec98fe09acba24c624 Reviewed-on: https://chromium-review.googlesource.com/906184 Commit-Queue: Khushal <khushalsagar@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: enne <enne@chromium.org> Cr-Commit-Position: refs/heads/master@{#537973} [add] https://crrev.com/848aaee2c40cd2c5dfa416c1dacebb44f4b6fa2e/content/browser/oop_browsertest.cc [modify] https://crrev.com/848aaee2c40cd2c5dfa416c1dacebb44f4b6fa2e/content/test/BUILD.gn
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31ae52a0bf9cd65d2af0f471a26f236bfebc5abe commit 31ae52a0bf9cd65d2af0f471a26f236bfebc5abe Author: Maxim Kolosovskiy <kolos@chromium.org> Date: Wed Feb 21 12:13:31 2018 Revert "content: Add basic integration test for OOP raster." This reverts commit 848aaee2c40cd2c5dfa416c1dacebb44f4b6fa2e. Reason for revert: broke OOPBrowserTest.Basic https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests #0 0x000000634a81 in __interceptor_backtrace /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:3955:13 #1 0x00000be3764f in base::debug::StackTrace::StackTrace(unsigned long) ./../../base/debug/stack_trace_posix.cc:808:41 #2 0x00000ab5b871 in content::(anonymous namespace)::DumpStackTraceSignalHandler(int) ./../../content/public/test/browser_test_base.cc:86:5 #3 0x00000065aff9 in SignalHandler(int) /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/msan_interceptors.cc:1049:3 #4 0x7f9e01eaccb0 in killpg ??:? #5 0x7f9e01eaccb0 in ?? ??:0 #6 0x7f9e01f74693 in epoll_wait /build/eglibc-SvCtMH/eglibc-2.19/misc/../sysdeps/unix/syscall-template.S:81:0 #7 0x00000061d970 in __interceptor_epoll_wait /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/msan/msan_interceptors.cc:891:13 #8 0x00000c1936a7 in epoll_dispatch ./../../base/third_party/libevent/epoll.c:198:8 #9 0x00000c186a2c in event_base_loop ./../../base/third_party/libevent/event.c:512:9 #10 0x00000beb8dce in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) ./../../base/message_loop/message_pump_libevent.cc:257:9 #11 0x00000bf5c000 in base::RunLoop::Run() ./../../base/run_loop.cc:133:14 #12 0x00000abdc529 in RunThisRunLoop ./../../content/public/test/test_utils.cc:124:13 #13 0x00000abdc529 in content::MessageLoopRunner::Run() ./../../content/public/test/test_utils.cc:241:0 #14 0x00000ab26d69 in content::NavigateToURLBlockUntilNavigationsComplete(content::Shell*, GURL const&, int) ./../../content/public/test/content_browser_test_utils.cc:49:21 #15 0x0000017ed89f in content::(anonymous namespace)::OOPBrowserTest_Basic_Test::RunTestOnMainThread() ./../../content/browser/oop_browsertest.cc:67:3 #16 0x00000ab5ad40 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() ./../../content/public/test/browser_test_base.cc:353:5 #17 0x00000ad8063f in Run ./../../base/callback.h:124:12 #18 0x00000ad8063f in content::ShellBrowserMainParts::PreMainMessageLoopRun() ./../../content/shell/browser/shell_browser_main_parts.cc:208:0 #19 0x000008847e8c in content::BrowserMainLoop::PreMainMessageLoopRun() ./../../content/browser/browser_main_loop.cc:1105:13 #20 0x000009ce2bd1 in Run ./../../base/callback.h:124:12 #21 0x000009ce2bd1 in content::StartupTaskRunner::RunAllTasksNow() ./../../content/browser/startup_task_runner.cc:45:0 #22 0x00000883f64c in content::BrowserMainLoop::CreateStartupTasks() ./../../content/browser/browser_main_loop.cc:986:25 #23 0x000008855c2b in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) ./../../content/browser/browser_main_runner.cc:139:17 #24 0x00000acfaaf1 in ShellBrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserMainRunner, std::__1::default_delete<content::BrowserMainRunner> > const&) ./../../content/shell/browser/shell_browser_main.cc:23:32 #25 0x00000acc3f93 in content::ShellMainDelegate::RunProcess(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&) ./../../content/shell/app/shell_main_delegate.cc:322:16 #26 0x000008382651 in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) ./../../content/app/content_main_runner.cc:410:35 #27 0x0000083852d0 in content::ContentMainRunnerImpl::Run() ./../../content/app/content_main_runner.cc:713:12 #28 0x0000118257b1 in service_manager::Main(service_manager::MainParams const&) ./../../services/service_manager/embedder/main.cc:456:29 #29 0x000004e857a5 in content::ContentMain(content::ContentMainParams const&) ./../../content/app/content_main.cc:19:10 #30 0x00000ab596db in content::BrowserTestBase::SetUp() ./../../content/public/test/browser_test_base.cc:309:3 #31 0x00000ab2527f in content::ContentBrowserTest::SetUp() ./../../content/public/test/content_browser_test.cc:89:20 #32 0x00000271190c in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 #33 0x0000027158c9 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2645:11 #34 0x0000027172ea in testing::TestCase::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2763:28 #35 0x00000273ad4f in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:4638:43 #36 0x000002739766 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0 #37 0x00000ac499cb in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2297:46 #38 0x00000ac499cb in base::TestSuite::Run() ./../../base/test/test_suite.cc:272:0 #39 0x00000ab3c758 in content::ContentTestLauncherDelegate::RunTestSuite(int, char**) ./../../content/test/content_test_launcher.cc:108:48 #40 0x00000abca59a in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) ./../../content/public/test/test_launcher.cc:631:31 #41 0x00000ab3c5f8 in main ./../../content/test/content_test_launcher.cc:138:10 #42 0x7f9e01e97f45 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0 #43 0x0000005f84aa in _start ??:0:0 Original change's description: > content: Add basic integration test for OOP raster. > > R=enne@chromium.org > > Bug: 809692 > Change-Id: Ib22f6f452046d9989ec9a1ec98fe09acba24c624 > Reviewed-on: https://chromium-review.googlesource.com/906184 > Commit-Queue: Khushal <khushalsagar@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Reviewed-by: enne <enne@chromium.org> > Cr-Commit-Position: refs/heads/master@{#537973} TBR=enne@chromium.org,khushalsagar@chromium.org,piman@chromium.org Change-Id: Iefb06aeb5a6055f34babfd83794cfccf809d7389 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 809692 Reviewed-on: https://chromium-review.googlesource.com/928581 Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org> Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org> Cr-Commit-Position: refs/heads/master@{#538071} [delete] https://crrev.com/6d3cd7563365aed7ea2aba1aff254fe42edece51/content/browser/oop_browsertest.cc [modify] https://crrev.com/31ae52a0bf9cd65d2af0f471a26f236bfebc5abe/content/test/BUILD.gn
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3090730f17666355c48f4fd7ba7c882ffac87a4 commit c3090730f17666355c48f4fd7ba7c882ffac87a4 Author: Khushal <khushalsagar@chromium.org> Date: Thu Feb 22 00:07:36 2018 Reland content: Add basic integration test for OOP raster. This reverts commit 31ae52a0bf9cd65d2af0f471a26f236bfebc5abe. The test fails on MSAN bots because it uses system GL which is not instrumented with MSAN. R=piman@chromium.org Bug: 809692 Change-Id: I4db4b749fa03afbf7b92a2c66ebbe9332098f3cd Reviewed-on: https://chromium-review.googlesource.com/930029 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#538266} [add] https://crrev.com/c3090730f17666355c48f4fd7ba7c882ffac87a4/content/browser/oop_browsertest.cc [modify] https://crrev.com/c3090730f17666355c48f4fd7ba7c882ffac87a4/content/test/BUILD.gn |
||
►
Sign in to add a comment |
||
Comment 1 by enne@chromium.org
, Feb 6 2018