WebGL's transferToImageBitmap gives black on Mac |
||||
Issue descriptionWe have layout test coverage for this case under: fast/canvas/webgl/offscreenCanvas-transferToImageBitmap.html fast/canvas/webgl/offscreenCanvas-transferToImageBitmap-in-worker.html Both of them run fine on all platforms. However, if I just drag these two files to the browser running with "--enable-experimental-canvas-features", I see black squares, with an error message: gl->CreateAndConsumeTextureCHROMIUM: invalid target. I suspect something is wrong with DrawingBuffer::transferToStaticBitmapImage(), which is likely a mistake that I made earlier. Note that this happens on Mac only, windows, linux and android are fine.
,
Sep 23 2016
loop in danakj@. I looked at DrawingBuffer::transferToStaticBitmapImage(), at line 451, the textureId I got is 1 and 3 when I drag this file to browser: fast/canvas/webgl/offscreenCanvas-transferToImageBitmap.html which indicates that the textureId is valid. Could there be something wrong with prepareTextureMailboxInternal()?
,
Sep 23 2016
,
Sep 23 2016
More investigation: apparently this seems to be a regression. I did bisect on Mac, and it is pointing to this CL: https://codereview.chromium.org/2085573002, which removes this line on Mac: enable_web_gl_image_chromium = false; So I tried to run chromium with --disable-webgl-image-chromium, and then everything is fine, the layout test is correct, the gpu pixel tests for WebGL's commit() API run fine under Mac. kbr@: I don't think we should put that line back because it will introduce more bugs, but I am not sure what should be the right fix.
,
Sep 23 2016
On macOS, DrawingBuffer is using an IOSurface as its backing storage -- this is the WebGLImageChromium path that erikchen@ added. This allows WebGL-rendered canvases to be composited by the OS rather than Chrome. IOSurfaces are only compatible with the GL_TEXTURE_RECTANGLE_ARB binding target, unfortunately, and this has required some specialization of some of the code. I think that the best approach here is to go down the old, non-WebGLImageChromium, code path for OffscreenCanvases. This will avoid the knowledge of GL_TEXTURE_RECTANGLE_ARB type textures being introduced into more areas of the code. While this means that the destination canvas for the OffscreenCanvas will have to be composited by Chrome on macOS, I think that's reasonable, and it could be optimized later if it becomes a bottleneck. Does this explain what's going on? We want to instantiate the DrawingBuffer just for the WebGL-rendered OffscreenCanvas with the WebGLImageChromium code path disabled. I think it's reasonable to do this for all operating systems, not just Mac.
,
Sep 23 2016
kbr's proposal sounds good. Notes for myself: DrawingBuffer::transferToStaticBitmapImage returns a AcceleratedStaticBitmapImage, which is just a wrapper around a texture-backed SkImage. Skia doesn't handle rectangle textures [yet/ever?], so there's no way to pass through an IOSurface. Based on https://developer.mozilla.org/en-US/docs/Web/API/OffscreenCanvas, it seems like the main point of an OffscreenCanvas is to do some rendering, and then do something with the result that isn't just display it on the screen. [pass ImageContext to another canvas, save to Blob, etc.]. As such, it's not particularly important to use an IOSurface for the backing storage.
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ca72b27d628adf45c298f313869f7b20a7e83aa commit 3ca72b27d628adf45c298f313869f7b20a7e83aa Author: xidachen <xidachen@chromium.org> Date: Tue Sep 27 12:30:58 2016 Fix failing transferToImageBitmap layout tests and commit pixel test on Mac Right now these two tests: fast/canvas/webgl/offscreenCanvas-transferToImageBitmap.html fast/canvas/webgl/offscreenCanvas-transferToImageBitmap-in-worker.html are not working properly on Mac if we drag them to the browser. And the reason is that DrawingBuffer()'s transferToStaticImage() returns a StaticBitmapImage that is always black because the WebGLImageChromium flag is enabled. This CL disable this flag when creating a DrawingBuffer() inside the WebGLRenderingContextBase's constructor, if the creation is requested by a offscreenCanvas. This change will also fix the failure of gpu pixel test for commit() on Mac because commit() method depends on DrawingBuffer()'s transferToStaticImage() as well. Moreover, since the layout test doesn't really compare the composited result, we should probably turn the above two layout tests into pixel tests. BUG= 649668 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2365653005 Cr-Commit-Position: refs/heads/master@{#421181} [add] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/content/test/data/gpu/pixel_offscreenCanvas_transferToImageBitmap_main.html [add] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/content/test/data/gpu/pixel_offscreenCanvas_transferToImageBitmap_worker.html [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/content/test/gpu/page_sets/pixel_tests.py [delete] https://crrev.com/e6a38be833c08272f2aa356601ff663078876553/third_party/WebKit/LayoutTests/fast/canvas/webgl/offscreenCanvas-transferToImageBitmap-expected.html [delete] https://crrev.com/e6a38be833c08272f2aa356601ff663078876553/third_party/WebKit/LayoutTests/fast/canvas/webgl/offscreenCanvas-transferToImageBitmap-in-worker-expected.html [delete] https://crrev.com/e6a38be833c08272f2aa356601ff663078876553/third_party/WebKit/LayoutTests/fast/canvas/webgl/offscreenCanvas-transferToImageBitmap-in-worker.html [delete] https://crrev.com/e6a38be833c08272f2aa356601ff663078876553/third_party/WebKit/LayoutTests/fast/canvas/webgl/offscreenCanvas-transferToImageBitmap.html [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTest.cpp [modify] https://crrev.com/3ca72b27d628adf45c298f313869f7b20a7e83aa/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferTestHelpers.h
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1d6bec29e841217f8fef71aaacad6a458d4a1e5 commit a1d6bec29e841217f8fef71aaacad6a458d4a1e5 Author: xidachen <xidachen@chromium.org> Date: Tue Sep 27 16:53:01 2016 Remove some failure entry in gpu pixel test expectations In a previous CL: https://codereview.chromium.org/2365653005/, we fixed gpu pixel test for WebGL's commit API on Mac, so we mark those tests as failure. We have also added two tests to transferToImageBitmap API in the same CL. Now that the CL has been picked up by all GPU bots, and I have checked all reference images, it is time to remove the failure entry. TBR=kbr@chromium.org, zmo@chromium.org, bajones@chromium.org BUG= 649668 CQ_INCLUDE_TRYBOTS=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;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2376533003 Cr-Commit-Position: refs/heads/master@{#421235} [modify] https://crrev.com/a1d6bec29e841217f8fef71aaacad6a458d4a1e5/content/test/gpu/gpu_tests/pixel_expectations.py
,
Sep 27 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by xidac...@chromium.org
, Sep 23 2016