Using non-zero texture mipmap level without base in framebuffer causes WebGL2 crash
Reported by
j...@joshgroves.com,
May 16 2017
|
|||||||||||
Issue description
UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
Steps to reproduce the problem:
A WebGL crash will occur while executing the attached test case. I believe it should work for any mipmap level greater than zero ('X' below).
1. Create and bind a texture
2. Call texImage2D *only for mipmap level X*
3. Create and bind a framebuffer
4. Call framebufferTexture2D *only for mipmap level X*
5. Call clear (or any other command that will cause ensureRenderTarget to be invoked)
What is the expected behavior?
Fail gracefully like the other invalid attachment errors.
What went wrong?
It appears the storage was not created successfully (`mTexStorage` is nullptr).
In `TextureD3D_2D::initializeStorage`, `gl::NoError()` is returned because it satisfied the condition `!isLevelComplete(getBaseLevel())`.
This occurred because `getLevelZero{Width|Height}` return 0 when called from `TextureD3D_2D::isLevelComplete`. This is because `mImageArray[0]` has `mWidth = 0` and `mHeight = 0`. So `isLevelComplete` returns `false` because it satisfied the condition `width <= 0 || height <= 0`.
Afterwards `UNREACHABLE()` is hit in `TextureD3D::ensureRenderTarget` because `mTexStorage` is nullptr.
Did this work before? N/A
Does this work in other browsers? No
Firefox crashes as well.
Chrome version: 57.0.2987.133 Channel: n/a
OS Version: 10.0
Flash Version:
,
May 16 2017
I've just noticed that I was still on Chrome 57 when I reproduced this. I don't receive the crash on Chrome 58 although no error is generated either. `gl.checkFramebufferStatus(gl.FRAMEBUFFER) returns `gl.FRAMEBUFFER_COMPLETE` in this case -- is this correct even when the base level was not explicitly created with `texImage2D`? I thought the crash was happening in a recent Chromium build also but I may be mistaken. The latest Firefox Nightly still crashes on the example so perhaps I should cross-post it there instead? I think it's possible there may still be an issue in ANGLE.
,
May 16 2017
,
May 16 2017
This sounds like a bug and sounds like our conformance suite does not cover the case (which is a SURPRISE to me). According to the ES3 spec, if a fbo is bound with a texture level other than base_level, then the texture needs to be mipmap complete.
,
May 16 2017
I'll handle fixing the framebuffer completeness check in ANGLE.
,
May 16 2017
Sorry, I can't reproduce the issue you described. i.e., in your example, if I check framebuffer status right after the framebufferTexture2D call, Chrome correctly return FRAMEBUFFER_INCOMPLETE_ATTACHMENT. I am testing it on Linux, but I think on Windows the behavior should be the same because it's generated by the command buffer, not the driver or ANGLE.
,
May 16 2017
Thanks for trying to reproduce on Linux, I've attached my latest output (latest-angle) for Canary and Chromium stable (still Win10) using ANGLE. On the same laptop I receive FRAMEBUFFER_INCOMPLETE_ATTACHMENT when using --use-gl=desktop on Chromium stable (Win10). You can see this in the attachment (latest-opengl). So that's why I think it's an issue with ANGLE. I'll try again with my desktop later.
,
May 16 2017
You are right. Command buffer missed this checking (texture needs to be mipmap complete if level != base_level) and the GL driver did this. That's why it's failing on ANGLE. I'll take the bug and fix it on the command buffer side. Geoff should fix the ANGLE behavior.
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/9f10b775c9b17f901d940157e43e5a74b75c2708 commit 9f10b775c9b17f901d940157e43e5a74b75c2708 Author: Geoff Lang <geofflang@chromium.org> Date: Thu May 18 15:50:14 2017 Refactor checks for incomplete framebuffer attachments. BUG=722684 Change-Id: I6f47d92b450a056a2344767bef28e96c4552f98e Reviewed-on: https://chromium-review.googlesource.com/506927 Reviewed-by: Geoff Lang <geofflang@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/9f10b775c9b17f901d940157e43e5a74b75c2708/src/libANGLE/Framebuffer.cpp
,
May 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2347997fc357bfa237223290cf55b37f9234e478 commit 2347997fc357bfa237223290cf55b37f9234e478 Author: kbr <kbr@chromium.org> Date: Thu May 18 17:50:43 2017 Roll ANGLE 4465330..9f10b77 https://chromium.googlesource.com/angle/angle.git/+log/4465330..9f10b77 BUG=722684, 722345 TBR=geofflang@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2891973002 Cr-Commit-Position: refs/heads/master@{#472867} [modify] https://crrev.com/2347997fc357bfa237223290cf55b37f9234e478/DEPS
,
May 19 2017
What's the situation if fbo is attaching a base level texture image of non-zero-base-level texture? See bug 678526.
,
May 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/ff77c359221485886b8207e8afd23c11e0768cf4 commit ff77c359221485886b8207e8afd23c11e0768cf4 Author: Geoff Lang <geofflang@chromium.org> Date: Thu May 25 19:02:24 2017 Refactor multisample framebuffer completeness checks. BUG=722684 Change-Id: Iebe6968ebd693e7f051394a040eac1cf6f539e6e Reviewed-on: https://chromium-review.googlesource.com/508221 Reviewed-by: Geoff Lang <geofflang@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/ff77c359221485886b8207e8afd23c11e0768cf4/src/libANGLE/Framebuffer.cpp
,
May 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a5b43da4d706751276a46486256ad6677014868 commit 0a5b43da4d706751276a46486256ad6677014868 Author: ynovikov <ynovikov@chromium.org> Date: Fri May 26 00:40:52 2017 Roll ANGLE 9e3bd31..ff77c35 https://chromium.googlesource.com/angle/angle.git/+log/9e3bd31..ff77c35 BUG=None,449754,chromium:723856,chromium:723069,722684,chromium:699479 TBR=jmadill@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2908513003 Cr-Commit-Position: refs/heads/master@{#474852} [modify] https://crrev.com/0a5b43da4d706751276a46486256ad6677014868/DEPS
,
May 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/7d79fe95bd353a9ca0805db7d7b6442b3c5e7469 commit 7d79fe95bd353a9ca0805db7d7b6442b3c5e7469 Author: Geoff Lang <geofflang@chromium.org> Date: Fri May 26 21:54:10 2017 Revert "Refactor multisample framebuffer completeness checks." This reverts commit ff77c359221485886b8207e8afd23c11e0768cf4. Reason for revert: Breaks FramebufferMixedSamplesTest on Android NVIDIA BUG= angleproject:2045 Original change's description: > Refactor multisample framebuffer completeness checks. > > BUG=722684 > > Change-Id: Iebe6968ebd693e7f051394a040eac1cf6f539e6e > Reviewed-on: https://chromium-review.googlesource.com/508221 > Reviewed-by: Geoff Lang <geofflang@chromium.org> > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Commit-Queue: Geoff Lang <geofflang@chromium.org> > TBR=fjhenigman@chromium.org,geofflang@chromium.org,jmadill@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. BUG=722684 Change-Id: I1abab021ba73b218cbbe60d4e7bfdaae56955c64 Reviewed-on: https://chromium-review.googlesource.com/517390 Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/7d79fe95bd353a9ca0805db7d7b6442b3c5e7469/src/libANGLE/Framebuffer.cpp
,
May 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54841760a98142fac982f3ec12064093b8c3a7da commit 54841760a98142fac982f3ec12064093b8c3a7da Author: geofflang <geofflang@chromium.org> Date: Mon May 29 14:50:37 2017 Set the texture base level before generating sRGB mipmaps. GL ES 3.0 considers the framebuffer incomplete if the texture is not mip complete and the bound mip level is not the base level. Set the base level to the attached texture level before rendering each level to avoid this validation. BUG=722684 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/2900403003 Cr-Commit-Position: refs/heads/master@{#475349} [modify] https://crrev.com/54841760a98142fac982f3ec12064093b8c3a7da/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc
,
May 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/a1b9d5a259c196b13eced67053fc650a994e07af commit a1b9d5a259c196b13eced67053fc650a994e07af Author: Geoff Lang <geofflang@chromium.org> Date: Mon May 29 15:11:47 2017 Refactor multisample framebuffer completeness checks. Re-land with protection against div-by-zero in samples compatibility check. BUG=722684 Change-Id: I3d5c310d1f2cb4d8b92d80492435855c3c4ad807 Reviewed-on: https://chromium-review.googlesource.com/517427 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/a1b9d5a259c196b13eced67053fc650a994e07af/src/libANGLE/Framebuffer.cpp
,
May 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/857c09db35792952f7ba022a3274505622e6ed62 commit 857c09db35792952f7ba022a3274505622e6ed62 Author: Geoff Lang <geofflang@chromium.org> Date: Mon May 29 16:00:00 2017 Add missing completeness checks for texture attachments. Texture attachments need to validate that the attached mip level is within the [baseLevel,maxLevel] range and that the texture is complete if the attached mip level is not the base level. BUG=722684 Change-Id: I859766506b295638572b75a0e2e9fed168be047a Reviewed-on: https://chromium-review.googlesource.com/506928 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/857c09db35792952f7ba022a3274505622e6ed62/src/libANGLE/Framebuffer.cpp [modify] https://crrev.com/857c09db35792952f7ba022a3274505622e6ed62/src/tests/gl_tests/FramebufferTest.cpp [modify] https://crrev.com/857c09db35792952f7ba022a3274505622e6ed62/src/libANGLE/Texture.cpp [modify] https://crrev.com/857c09db35792952f7ba022a3274505622e6ed62/src/libANGLE/Texture.h
,
May 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69d6bf0149b35dbe1bcd94d329b03a676e25401b commit 69d6bf0149b35dbe1bcd94d329b03a676e25401b Author: jmadill <jmadill@chromium.org> Date: Mon May 29 16:13:28 2017 Roll ANGLE ff77c35..7d79fe9 https://chromium.googlesource.com/angle/angle.git/+log/ff77c35..7d79fe9 BUG=722684, chromium:724870 TBR=geofflang@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2906353002 Cr-Commit-Position: refs/heads/master@{#475359} [modify] https://crrev.com/69d6bf0149b35dbe1bcd94d329b03a676e25401b/DEPS
,
May 29 2017
I'd like to merge a small fix to the CL in comment #12 that causes crashes on some Android systems. The CL was rolled and branched before the revert could take place. The fix is a very simple one-liner: https://chromium-review.googlesource.com/c/517985/
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07f6758930d23cf2790a7ee97bda1d97889638c9 commit 07f6758930d23cf2790a7ee97bda1d97889638c9 Author: geofflang <geofflang@chromium.org> Date: Tue May 30 15:47:35 2017 Roll ANGLE 7d79fe9..63d8c26 https://chromium.googlesource.com/angle/angle.git/+log/7d79fe9..63d8c26 BUG=722684, chromium:721648 TBR=jmadill@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2912963002 Cr-Commit-Position: refs/heads/master@{#475539} [modify] https://crrev.com/07f6758930d23cf2790a7ee97bda1d97889638c9/DEPS
,
May 31 2017
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 6 2017
josh@ would you mind helping us in verifying the fix in latest canary.
,
Jun 7 2017
Thanks everyone. It behaves as expected (incomplete framebuffer) in the latest Canary update (61.0.3122.0/5978e28d3adc) on Windows through ANGLE.
,
Jun 7 2017
Approving the change in #19 for merge into M60.
,
Jun 7 2017
@bustamante, How do I find out which branch that would be? Omahaproxy implies 3112 but I'm not sure how to confirm.
,
Jun 7 2017
https://omahaproxy.appspot.com/ is the location to confirm the branch #, please check under "true_branch".Once the CL lands you can can also confirm from branch head. https://chromium.googlesource.com/chromium/src.git/+log/refs/branch-heads/3112?n=1000
,
Jun 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0 commit 17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0 Author: Geoff Lang <geofflang@chromium.org> Date: Thu Jun 08 13:18:41 2017 Fix potential div-by-zero when CHROMIUM_framebuffer_mixed_samples is exposed. BUG=722684 Change-Id: I1a3e88ff15580e6e14c21e6cd1da6392e82a092d Reviewed-on: https://chromium-review.googlesource.com/528212 Reviewed-by: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0/src/libANGLE/Framebuffer.cpp
,
Jun 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0 commit 17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0 Author: Geoff Lang <geofflang@chromium.org> Date: Thu Jun 08 13:18:41 2017 Fix potential div-by-zero when CHROMIUM_framebuffer_mixed_samples is exposed. BUG=722684 Change-Id: I1a3e88ff15580e6e14c21e6cd1da6392e82a092d Reviewed-on: https://chromium-review.googlesource.com/528212 Reviewed-by: Geoff Lang <geofflang@chromium.org> [modify] https://crrev.com/17cfb6c47490dc3c6ca84a40de1e2636fcfd49d0/src/libANGLE/Framebuffer.cpp |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by ranjitkan@chromium.org
, May 16 2017