Verify that WebGL's implicit clears obey the rasterizer discard state. #3183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@jdarpinian or @shrekshao or @jdashg : please review. Test passes on Firefox; fails on Chrome and Safari. Patches incoming for both. |
shrekshao
approved these changes
Nov 17, 2020
There was a problem hiding this comment.
LGTM. (learn webgl implicit clearing from greggman's answer: https://stackoverflow.com/questions/54947518/requestanimationframe-readpixel-and-implicit-clear)
|
(If a readpixels test needs adding as well together with drawArrays, as James points out in the CL review) |
kdashg
approved these changes
Nov 17, 2020
sdk/tests/conformance2/rendering/rasterizer-discard-and-implicit-clear.html
Outdated
Show resolved
Hide resolved
sdk/tests/conformance2/rendering/rasterizer-discard-and-implicit-clear.html
Outdated
Show resolved
Hide resolved
sdk/tests/conformance2/rendering/rasterizer-discard-and-implicit-clear.html
Show resolved
Hide resolved
sdk/tests/conformance2/rendering/rasterizer-discard-and-implicit-clear.html
Outdated
Show resolved
Hide resolved
sdk/tests/conformance2/rendering/rasterizer-discard-and-implicit-clear.html
Outdated
Show resolved
Hide resolved
|
Thanks everyone for your comments. Addressed @jdashg 's review feedback. Added a test of implicit clears, rasterizer discard and readPixels based on @jdarpinian 's review feedback on the Chromium review request https://chromium-review.googlesource.com/2542770 . The Travis CI system is slow for some reason right now. Squashing and merging this in order to get it in sooner. |
pull bot
pushed a commit
to Alan-love/chromium
that referenced
this pull request
Nov 20, 2020
WebGL conformance test for this is incoming in KhronosGroup/WebGL#3183 . Bug: 1149176 Change-Id: I805ae3dfc63af6afa926599ccc606d11e2e3e20e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542770 Reviewed-by: James Darpinian <jdarpinian@chromium.org> Commit-Queue: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#829527}
caiolima
pushed a commit
to caiolima/webkit
that referenced
this pull request
Dec 1, 2020
https://bugs.webkit.org/show_bug.cgi?id=219061 Patch by Kenneth Russell <kbr@chromium.org> on 2020-11-30 Reviewed by Dean Jackson. When rasterizer discard is enabled, user-level draw calls and clears skip the implicit clear since they have no effect. Readbacks and copies still perform the implicit clear. A new WebGL conformance test has been added for this in KhronosGroup/WebGL#3183 which passes with this fix. WebKit's TestRunner doesn't run the composite phase as the browser or MiniBrowser do, so wouldn't pass this test as integrated as a layout test. Per discussion with dino and kkinnunen on Slack, will address this in follow-on work. * html/canvas/WebGL2RenderingContext.cpp: (WebCore::WebGL2RenderingContext::copyTexSubImage3D): (WebCore::WebGL2RenderingContext::readPixels): * html/canvas/WebGLRenderingContextBase.cpp: (WebCore::ScopedDisableRasterizerDiscard::ScopedDisableRasterizerDiscard): (WebCore::ScopedDisableRasterizerDiscard::~ScopedDisableRasterizerDiscard): (WebCore::WebGLRenderingContextBase::initializeNewContext): (WebCore::WebGLRenderingContextBase::clearIfComposited): (WebCore::WebGLRenderingContextBase::paintRenderingResultsToCanvas): (WebCore::WebGLRenderingContextBase::paintRenderingResultsToImageData): (WebCore::WebGLRenderingContextBase::clear): (WebCore::WebGLRenderingContextBase::copyTexSubImage2D): (WebCore::WebGLRenderingContextBase::disable): (WebCore::WebGLRenderingContextBase::drawArrays): (WebCore::WebGLRenderingContextBase::drawElements): (WebCore::WebGLRenderingContextBase::enable): (WebCore::WebGLRenderingContextBase::readPixels): (WebCore::WebGLRenderingContextBase::copyTexImage2D): (WebCore::WebGLRenderingContextBase::drawArraysInstanced): (WebCore::WebGLRenderingContextBase::drawElementsInstanced): * html/canvas/WebGLRenderingContextBase.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@270253 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=219061 Patch by Kenneth Russell <kbr@chromium.org> on 2020-11-30 Reviewed by Dean Jackson. When rasterizer discard is enabled, user-level draw calls and clears skip the implicit clear since they have no effect. Readbacks and copies still perform the implicit clear. A new WebGL conformance test has been added for this in KhronosGroup/WebGL#3183 which passes with this fix. WebKit's TestRunner doesn't run the composite phase as the browser or MiniBrowser do, so wouldn't pass this test as integrated as a layout test. Per discussion with dino and kkinnunen on Slack, will address this in follow-on work. * html/canvas/WebGL2RenderingContext.cpp: (WebCore::WebGL2RenderingContext::copyTexSubImage3D): (WebCore::WebGL2RenderingContext::readPixels): * html/canvas/WebGLRenderingContextBase.cpp: (WebCore::ScopedDisableRasterizerDiscard::ScopedDisableRasterizerDiscard): (WebCore::ScopedDisableRasterizerDiscard::~ScopedDisableRasterizerDiscard): (WebCore::WebGLRenderingContextBase::initializeNewContext): (WebCore::WebGLRenderingContextBase::clearIfComposited): (WebCore::WebGLRenderingContextBase::paintRenderingResultsToCanvas): (WebCore::WebGLRenderingContextBase::paintRenderingResultsToImageData): (WebCore::WebGLRenderingContextBase::clear): (WebCore::WebGLRenderingContextBase::copyTexSubImage2D): (WebCore::WebGLRenderingContextBase::disable): (WebCore::WebGLRenderingContextBase::drawArrays): (WebCore::WebGLRenderingContextBase::drawElements): (WebCore::WebGLRenderingContextBase::enable): (WebCore::WebGLRenderingContextBase::readPixels): (WebCore::WebGLRenderingContextBase::copyTexImage2D): (WebCore::WebGLRenderingContextBase::drawArraysInstanced): (WebCore::WebGLRenderingContextBase::drawElementsInstanced): * html/canvas/WebGLRenderingContextBase.h: Canonical link: https://commits.webkit.org/231962@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270253 268f45cc-cd09-0410-ab3c-d52691b4dbfc
mjfroman
pushed a commit
to mjfroman/moz-libwebrtc-third-party
that referenced
this pull request
Oct 14, 2022
WebGL conformance test for this is incoming in KhronosGroup/WebGL#3183 . Bug: 1149176 Change-Id: I805ae3dfc63af6afa926599ccc606d11e2e3e20e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542770 Reviewed-by: James Darpinian <jdarpinian@chromium.org> Commit-Queue: Kenneth Russell <kbr@chromium.org> Cr-Commit-Position: refs/heads/master@{#829527} GitOrigin-RevId: 29f71568490de8f3a420580e8497c4a5e73600fb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Regression test for http://crbug.com/1149176 .