Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make renderbufferStorageMultisample with DEPTH_STENCIL and samples>0 generate INVALID_OPERATION. #3031

Merged
merged 1 commit into from Mar 6, 2020

Conversation

kdashg
Copy link
Contributor

@kdashg kdashg commented Feb 28, 2020

Firefox allows DEPTH_STENCIL with samples >= 0.
Chrome allows samples==0, but INVALID_ENUM for samples >= 1.

@kdashg
Copy link
Contributor Author

kdashg commented Feb 28, 2020

The other option is to make format==DEPTH_STENCIL && samples > 0 generate INVALID_OPERATION, since the entrypoint accepts the enum (INVALID_ENUM), just not the way it is used. (INVALID_OPERATION)

@kdashg
Copy link
Contributor Author

kdashg commented Feb 28, 2020

This is a resolution for #2902.

@kenrussell
Copy link
Member

My preference would be to reject the unsized DEPTH_STENCIL enum with samples > 0 with INVALID_OPERATION. Would you please consider updating the test to assert that instead? We'll be happy to change Chrome's implementation to generate INVALID_OPERATION instead of INVALID_ENUM to follow.

CC @jdarpinian @shrekshao for any thoughts.

@kdashg
Copy link
Contributor Author

kdashg commented Feb 28, 2020

Sure!

@kdashg kdashg changed the title Test that DEPTH_STENCIL works with renderbufferStorageMultisample, ev… Make renderbufferStorageMultisample with DEPTH_STENCIL and samples>0 generate INVALID_OPERATION. Feb 29, 2020
@kdashg
Copy link
Contributor Author

kdashg commented Feb 29, 2020

PTAL!

Copy link
Member

@kenrussell kenrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM per discussion above.

@kenrussell
Copy link
Member

@jdarpinian @shrekshao heads up that we need to file a bug about changing this validation in Chrome because this test will start failing during the next conformance roll.

@kenrussell kenrussell merged commit a22e3fc into KhronosGroup:master Mar 6, 2020
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Oct 29, 2020
WebGL2 does not allow allocating multisample renderbuffers with the GL_DEPTH_STENCIL
format but Chrome has been generating the wrong error since the WebGL test was updated in
KhronosGroup/WebGL#3031.

TBR=kbr@chromium.org

Bug: chromium:1082455
Bug: angleproject:5227
Change-Id: Ia9818968578a47023958c35c9b037e8907f76091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2498111
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: James Darpinian <jdarpinian@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822304}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
WebGL2 does not allow allocating multisample renderbuffers with the GL_DEPTH_STENCIL
format but Chrome has been generating the wrong error since the WebGL test was updated in
KhronosGroup/WebGL#3031.

TBR=kbr@chromium.org

Bug: chromium:1082455
Bug: angleproject:5227
Change-Id: Ia9818968578a47023958c35c9b037e8907f76091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2498111
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: James Darpinian <jdarpinian@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822304}
GitOrigin-RevId: bd3558e3a9fd6f96c938a47f5467d9b765c641d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants