texSubImage3D generates invalid error |
|||||||||
Issue descriptionChrome Version: 69.0.3478.0 (Official Build) canary (64-bit) OS: Any? (Mac/Win) What steps will reproduce the problem? (1) http://jsfiddle.net/greggman/vwkhps65/ What is the expected result? No error What happens instead? Gets an error Works in Firefox
,
Jul 3
,
Jul 3
Here is where this error is generated: WebGL2RenderingContextBase::texSubImage3D -> WebGLRenderingContextBase::TexImageHelperHTMLCanvasElement -> WebGLRenderingContextBase::ValidateTexFunc where we pass in canvas's width and height instead of the texSubImage3D's input param width/height. So here canvas's height is 16512, whereas the max_texture_size is 16384. I still need to understand why we use canvas's width/height for validation though.
,
Jul 3
Ken, I think in WebGLRenderingContextBase::TexImageHelperHTMLCanvasElement() where it calls ValidateTexFunc(), where we pass in canvas->width() and canvas->height(), we should pass in width and height instead. However, I may misunderstand something here because using canvas's width/height seems intentional. Please take a look and advise before I change the code.
,
Jul 3
Conformance test is added here: https://github.com/KhronosGroup/WebGL/pull/2665 CL is here: https://chromium-review.googlesource.com/c/chromium/src/+/1125307
,
Jul 5
Yes, that code looks wrong. When adding support for WebGL 2.0, including uploads of sub-rectangles of DOM elements, and uploads of DOM elements to 3D textures and 2D texture arrays, the code became messy and this was missed. It looks like there are more places where this validation is wrong. Looking at callers of ValidateTexFunc, it may also be wrong for ImageData, HTMLImageElement and HTMLVideoElement. The difficulty is that the source sub-rectangle isn't always known when ValidateTexFunc is called. A "sentinel" empty rectangle is passed in in some cases. For example, for HTMLImageElement, image decoding (image->CachedImage()->GetImage()) has to be done in order to figure out the image's real width and height, so the callers of TexImageHelperHTMLImageElement in WebGLRenderingContextBase pass in an empty sub-rectangle so it's computed later. For the WebGL 2.0 entry points, the user's specified width and height are always used. In short, it might be difficult to fix this bug for ImageData and HTMLImageElement, but the fix for 2D canvas looks good and correct. Studied a bit of Mozilla's validation for uploading DOM elements to textures. It's structured interestingly, where more of the validation code is shared. https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=%2Bfunction%3A%22mozilla%3A%3Awebgl%3A%3ATexUnpackBlob%3A%3ATexUnpackBlob%28const+mozilla%3A%3AWebGLContext+%2A%2C+TexImageTarget%2C+uint32_t%2C+uint32_t%2C+uint32_t%2C+uint32_t%2C+gfxAlphaType%29%22&redirect_type=single#273 https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=%2Bfunction%3A%22%28static+in+dom%2Fcanvas%2FTexUnpackBlob.cpp%29%3A%3Amozilla%3A%3Awebgl%3A%3AValidateUnpackPixels%28mozilla%3A%3AWebGLContext+%2A%2C+const+char+%2A%2C+uint32_t%2C+uint32_t%2C+webgl%3A%3ATexUnpackBlob+%2A%29%22&redirect_type=single#182
,
Jul 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3db41c3913fd21440c3f621722f353b2910b3257 commit 3db41c3913fd21440c3f621722f353b2910b3257 Author: Zhenyao Mo <zmo@chromium.org> Date: Fri Jul 06 21:14:19 2018 Use the input width/height instead of canvas's width/height for tex func validation. BUG= 859400 TEST=webgl_conformance R=kainino@chromium.org Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I5866530c90b5064177f5890c88eece2fb62f770e Reviewed-on: https://chromium-review.googlesource.com/1125307 Reviewed-by: Kai Ninomiya <kainino@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Zhenyao Mo <zmo@chromium.org> Cr-Commit-Position: refs/heads/master@{#573072} [modify] https://crrev.com/3db41c3913fd21440c3f621722f353b2910b3257/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
,
Jul 9
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #69.0.3478.0 Verified the fix on Mac 10.13.3 and win-10 using latest chrome version #69.0.3486.0 as per the comment #0.Attaching screen shot for reference. Did not observe any error. Hence, the fix is working as expected. Adding the verified labels. Thanks...!!
,
Jul 20
This new test is unexpectedly failing on the following configurations: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_angle_rel_ng/2341 https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_optional_gpu_tests_rel/6289 GL passthrough: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_optional_gpu_tests_rel/6289 Suppressing these failures.
,
Jul 20
BTW, here's the failure log from webgl2_conformance_gl_passthrough_tests on NVIDIA GPU on Windows: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win_angle_rel_ng/2139 https://chromium-swarm.appspot.com/task?id=3ed24e80e8183c10&refresh=10&show_raw=1 [41/136] gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglConformance_conformance2_textures_misc_tex_subimage3d_canvas_bug failed unexpectedly 1.2330s: Traceback (most recent call last): _RunGpuTest at content\test\gpu\gpu_tests\gpu_integration_test.py:132 self.RunActualGpuTest(url, *args) RunActualGpuTest at content\test\gpu\gpu_tests\webgl_conformance_integration_test.py:185 getattr(self, test_name)(test_path, *args[1:]) _RunConformanceTest at content\test\gpu\gpu_tests\webgl_conformance_integration_test.py:199 self._CheckTestCompletion() _CheckTestCompletion at content\test\gpu\gpu_tests\webgl_conformance_integration_test.py:195 self.fail(self._WebGLTestMessages(self.tab)) fail at .swarming_module\bin\Lib\unittest\case.py:410 raise self.failureException(msg) AssertionError: getError expected: NO_ERROR. Was INVALID_VALUE : Should be no errors from TexSubImage3D. FAIL getError expected: NO_ERROR. Was INVALID_VALUE : Should be no errors from TexSubImage3D.
,
Jul 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f64860d89fdb6d3397286e722b42f18c7df7db7 commit 6f64860d89fdb6d3397286e722b42f18c7df7db7 Author: Kenneth Russell <kbr@chromium.org> Date: Sat Jul 21 02:54:54 2018 Roll WebGL a5c263c..21dbf06 https://chromium.googlesource.com/external/khronosgroup/webgl.git/+log/a5c263c..21dbf06 Bug: 830901 , 851870, 851873, 859400 , 866089 Tbr: zmo@chromium.org Tbr: kainino@chromium.org Cq-Include-Trybots: luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_angle_rel_ng;luci.chromium.try:win_angle_rel_ng Change-Id: Icef8ddc970ccb6653e95920c28b4d003d1b27100 Reviewed-on: https://chromium-review.googlesource.com/1144306 Commit-Queue: Kenneth Russell <kbr@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#577063} [modify] https://crrev.com/6f64860d89fdb6d3397286e722b42f18c7df7db7/DEPS [modify] https://crrev.com/6f64860d89fdb6d3397286e722b42f18c7df7db7/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py [modify] https://crrev.com/6f64860d89fdb6d3397286e722b42f18c7df7db7/content/test/gpu/gpu_tests/webgl_conformance_revision.txt
,
Nov 27
,
Nov 27
,
Nov 30
,
Nov 30
The test assumes we can can create a 2D canvas larger than MAX_TEXTURE_SIZE, but we limit 2D canvas size to 32768, so on platforms where MAX_TEXTURE_SIZE is >= 32768 this test fails. I'm not sure what to do about it yet.
,
Nov 30
,
Dec 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fd81121bab7d735ed01a8a112df253807c2e77c commit 5fd81121bab7d735ed01a8a112df253807c2e77c Author: James Darpinian <jdarpinian@chromium.org> Date: Mon Dec 03 20:37:16 2018 Increase maximum width/height of canvas. Fixes WebGL conformance test: conformance2/textures/misc/tex-subimage3d-canvas-bug.html The test expects to be able to create a canvas larger than 32767 in height. Skia seems to support this, and there have been user requests for it. Bug: 859400 , 339725 , skia:2122 Change-Id: Ibba89e98178bd73817c7b4cbdf569e89ee5764b1 Reviewed-on: https://chromium-review.googlesource.com/c/1356555 Reviewed-by: Fernando Serboncini <fserb@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: James Darpinian <jdarpinian@chromium.org> Cr-Commit-Position: refs/heads/master@{#613225} [modify] https://crrev.com/5fd81121bab7d735ed01a8a112df253807c2e77c/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py [modify] https://crrev.com/5fd81121bab7d735ed01a8a112df253807c2e77c/third_party/blink/renderer/platform/graphics/skia/skia_utils.h [modify] https://crrev.com/5fd81121bab7d735ed01a8a112df253807c2e77c/third_party/blink/web_tests/fast/canvas/canvas-large-pattern.html
,
Dec 3
,
Dec 18
Issue 861784 has been merged into this issue. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by zmo@chromium.org
, Jul 2Labels: -Pri-3 OS-Mac OS-Windows Pri-2
Owner: zmo@chromium.org
Status: Assigned (was: Available)