GPU-GPU copies of video textures to GL_RED format textures also copy the green channel |
||||
Issue descriptionIn the bug fix for Issue 701060 , and specifically this CL: https://codereview.chromium.org/2791813003/ Failures of the following WebGL conformance tests were observed on macOS: WebglConformance_conformance2_textures_video_tex_2d_r16f_red_float WebglConformance_conformance2_textures_video_tex_2d_r16f_red_half_float WebglConformance_conformance2_textures_video_tex_2d_r32f_red_float WebglConformance_conformance2_textures_video_tex_2d_r8_red_unsigned_byte WebglConformance_conformance2_textures_video_tex_2d_r8ui_red_integer_unsigned_byte with the following failure mode: WebglConformance_conformance2_textures_video_tex_2d_r32f_red_float failed unexpectedly 4.2208s: Traceback (most recent call last): _RunGpuTest at content/test/gpu/gpu_tests/gpu_integration_test.py:73 self.RunActualGpuTest(url, *args) RunActualGpuTest at content/test/gpu/gpu_tests/webgl_conformance_integration_test.py:203 getattr(self, test_name)(test_path, *args[1:]) _RunConformanceTest at content/test/gpu/gpu_tests/webgl_conformance_integration_test.py:217 self._CheckTestCompletion() _CheckTestCompletion at content/test/gpu/gpu_tests/webgl_conformance_integration_test.py:213 self.fail(self._WebGLTestMessages(self.tab)) fail at /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/case.py:412 raise self.failureException(msg) AssertionError: shouldBe 0,0,0 at (4, 24) expected: 0,0,0 was 0,255,0 testing: video/mp4; codecs="avc1.42E01E, mp4a.40.2" Testing texImage2D with flipY=true bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner Testing texImage2D with flipY=false bindingTarget=TEXTURE_2D Checking lower left corner Checking upper left corner FAIL shouldBe 0,0,0 at (4, 24) expected: 0,0,0 was 0,255,0 Basically: the destination texture is not supposed to contain a green channel, but for some reason it does. This behavior was introduced in https://codereview.chromium.org/2738163002 , when enabling GPU-GPU texture copies in more cases in Blink. It turns out that explicitly disallowing the use of GPU-GPU texture copies for GL_RED format textures works around the problem, so I'm introducing that in the fix for Issue 701060 and adding TODOs to track down the deeper root cause and fix it. The problem was only observed on macOS, but it might affect other operating systems. It might also be the root cause of Issue 710392. I've confirmed that the renderer-side code in this case thinks that it's allocating for example a GL_R8 / GL_RED / GL_UNSIGNED_BYTE texture. Not sure whether the GPU process-side code might be allocating a different texture, or whether the OS might be emulating these texture formats with ones that contain more color channels. The GPU-GPU blit in gles2_cmd_copy_texture_chromium.cc seems to be copying all color channels all the time, rather than taking into account the destination texture's format and explicitly writing zeros for the channels which don't exist.
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca29dd7baa80c00d30039af7dcbdfc1abca6864c commit ca29dd7baa80c00d30039af7dcbdfc1abca6864c Author: jiajia.qin <jiajia.qin@intel.com> Date: Wed Jul 19 22:34:38 2017 Re-enable GPU-GPU copies of video textures to GL_RED The CL https://codereview.chromium.org/2791813003/ introduced two bugs 710673 and 710874. So currently, for format=GL_RED/GL_RED_INTEGER or type=GL_FLOAT, it will not go GPU-GPU path. But by investigating that CL, we still can go GPU-GPU path using CopySubTextureCHROMIUM instead of CopyTextureCHROMIUM like the earlier code did. Using copySubTextureCHROMIUM can bypass the potential bug in CopyTextureCHROMIUM in MacOS. But still need to figure out the reason. BUG= 710673 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 Review-Url: https://codereview.chromium.org/2896553003 Cr-Commit-Position: refs/heads/master@{#488014} [modify] https://crrev.com/ca29dd7baa80c00d30039af7dcbdfc1abca6864c/media/renderers/skcanvas_video_renderer.cc [modify] https://crrev.com/ca29dd7baa80c00d30039af7dcbdfc1abca6864c/media/renderers/skcanvas_video_renderer.h [modify] https://crrev.com/ca29dd7baa80c00d30039af7dcbdfc1abca6864c/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7aeebf8adb4b2529af4d2f3833398c5fbc33e54e commit 7aeebf8adb4b2529af4d2f3833398c5fbc33e54e Author: Geoff Lang <geofflang@chromium.org> Date: Fri Jul 21 18:21:55 2017 Disable GPU-GPU video uploads for R8UI textures. R=kbr@chromium.org BUG= 710673 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: I323c5c020f62e4d388a5cae547b93887fc375640 Reviewed-on: https://chromium-review.googlesource.com/581348 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> Cr-Commit-Position: refs/heads/master@{#488713} [modify] https://crrev.com/7aeebf8adb4b2529af4d2f3833398c5fbc33e54e/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
,
Aug 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be8f68faf9872f0da290a58ac310964b823cc3bd commit be8f68faf9872f0da290a58ac310964b823cc3bd Author: Jiajia Qin <jiajia.qin@intel.com> Date: Wed Aug 09 07:14:35 2017 Fixed the incorrect copy in DoCopyTextureCHROMIUM In GLES2DecoderImpl::DoCopyTextureCHROMIUM, it will try to use GLImage::CopyTexImage when possible. However, GLImage::CopyTexImage always copy same format image data to target texture and don't do format translation. So we need to check whether the dest target's internal format is same with source target's internal format when we use GLImage::CopyTexImage. This is also the root cause of BUG 710673 . This CL also switch back CopyTextureCHROMIUM since CopySubTextureCHROMIUM may bring performance regression. See bug 750182 . One possible reason is that CopyTextureCHROMIUM can use CopyTexImage on Mac. However, CopySubTexImage hasn't been implemented on Mac. BUG= 710673 , 750182 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: Ia171f0b3db70ee02925a4ee15447f9c0ceda1dae Reviewed-on: https://chromium-review.googlesource.com/582160 Reviewed-by: Kenneth Russell <kbr@chromium.org> Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Jiajia Qin <jiajia.qin@intel.com> Cr-Commit-Position: refs/heads/master@{#492879} [modify] https://crrev.com/be8f68faf9872f0da290a58ac310964b823cc3bd/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/be8f68faf9872f0da290a58ac310964b823cc3bd/media/renderers/skcanvas_video_renderer.cc [modify] https://crrev.com/be8f68faf9872f0da290a58ac310964b823cc3bd/media/renderers/skcanvas_video_renderer.h
,
Aug 9 2017
Thank you Jiajia for tracking this down and fixing it!
,
Aug 10 2017
,
Nov 21 2017
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9300c95d365861c2f0165787bff9fd03e1f6d1ed commit 9300c95d365861c2f0165787bff9fd03e1f6d1ed Author: Victor Miura <vmiura@chromium.org> Date: Tue Nov 21 21:49:27 2017 Add missing BindTexture in PaintCanvasVideoRenderer. PaintCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture runs TexImage2D without first binding the target texture. This could result in the clobbering of other textures. BUG= 787333 , 710673 Change-Id: Ieff4c3f6068ad3682b3bd356491b6979624dfd66 Reviewed-on: https://chromium-review.googlesource.com/781422 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Commit-Queue: Victor Miura <vmiura@chromium.org> Cr-Commit-Position: refs/heads/master@{#518408} [modify] https://crrev.com/9300c95d365861c2f0165787bff9fd03e1f6d1ed/media/renderers/paint_canvas_video_renderer.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by lafo...@chromium.org
, Jun 20 2017