1315866 - Pass WebGL2 conformance deqp/functional/gles3/negativetextureapi.html

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Ethan Lin[:ethlin]]

failed: negativeTextureApi.copytexsubimage2d_max_level: Expected gl.INVALID_VALUE, but got gl.INVALID_OPERATION.
    failed: negativeTextureApi.texsubimage2d_max_level: Expected gl.INVALID_VALUE, but got gl.INVALID_OPERATION.
    failed: negativeTextureApi.texsubimage3d_max_level: Expected gl.INVALID_VALUE, but got gl.INVALID_OPERATION.
    failed: negativeTextureApi.copytexsubimage3d_max_level: Expected gl.INVALID_VALUE, but got gl.INVALID_OPERATION.

Comment 1

[Ethan Lin[:ethlin]]

Attachment: check max size

We should check the max texture size to return correct error type.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: check max size

There is some tabs in the original patch. Replace them to spaces.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: check max size

Sorry, the last patch was wrong. This should be the correct one.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8808921 [details] [diff] [review]
check max size

Review of attachment 8808921 [details] [diff] [review]:
-----------------------------------------------------------------

This should already be handled properly in ValidateTexImageSpecification. Please re-check.

Comment 5

[Ethan Lin[:ethlin]]

(In reply to Jeff Gilbert [:jgilbert] from comment #4)
> Comment on attachment 8808921 [details] [diff] [review]
> check max size
> 
> Review of attachment 8808921 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This should already be handled properly in ValidateTexImageSpecification.
> Please re-check.

Right, I should reuse the code in ValidateTexImageSpecification.

Comment 6

[Ethan Lin[:ethlin]]

Attachment: check max size

I move the check from ValidateTexImageSpecification to ValidateTexImage. So ValidateTexImageSelection (for TexSubImage and CompressedTexSubImage) can do this check too.

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Ethan Lin[:ethlin] from comment #6)
> Created attachment 8809292 [details] [diff] [review]
> check max size
> 
> I move the check from ValidateTexImageSpecification to ValidateTexImage. So
> ValidateTexImageSelection (for TexSubImage and CompressedTexSubImage) can do
> this check too.

It should only be necessary for Specification. Too large width/height/depth will get automatically checked by Selection checking against the previously-specified extents.

Comment 8

[Kelsey Gilbert [:jgilbert]]

It's likely this test is wrong. What's the actual invocation here? It's possible it's wrong in more than one way. Occasionally the conformance tests accidentally assume that some errors will occur before others, when actually any applicable error can be returned.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8809292 [details] [diff] [review]
check max size

Review of attachment 8809292 [details] [diff] [review]:
-----------------------------------------------------------------

r- for now, because I don't think this is the correct solution.

Comment 10

[Kelsey Gilbert [:jgilbert]]

This crashes on Windows+Intel.

Comment 11

[Kelsey Gilbert [:jgilbert]]

Half the issue is the test is wrong:
https://github.com/KhronosGroup/WebGL/pull/2207

Comment 12

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1315866 - Always-too-large level during tex image specification is INVALID_VALUE. -

Review commit: https://reviewboard.mozilla.org/r/98710/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98710/

Comment 14

[Ethan Lin[:ethlin]]

Comment on attachment 8818769 [details]
Bug 1315866 - Always-too-large level during tex image specification is INVALID_VALUE. -

https://reviewboard.mozilla.org/r/98710/#review99230

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/013060399bd7
Always-too-large level during tex image specification is INVALID_VALUE. - r=ethlin

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/013060399bd7

Comment 17

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8818769 [details]
Bug 1315866 - Always-too-large level during tex image specification is INVALID_VALUE. -

Approval Request Comment
[Feature/Bug causing the regression]: webgl2
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 18

[Gerry Chang [:gchang]]

Comment on attachment 8818769 [details]
Bug 1315866 - Always-too-large level during tex image specification is INVALID_VALUE. -

Fix a WebGL 2 related issue. Beta51+ and Aurora52+. Should be in 51 beta 10.

Comment 19

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c6107c5fe98e

Comment 20

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9e8e044f7b78