Closed Bug 1315866 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- disabled
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: ethlin, Assigned: jgilbert)

References

()

Details

Attachments

(1 file, 4 obsolete files)

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.
Attached patch check max size (obsolete) — Splinter Review
We should check the max texture size to return correct error type.
Attachment #8808467 - Flags: review?(jgilbert)
Attached patch check max size (obsolete) — Splinter Review
There is some tabs in the original patch. Replace them to spaces.
Attachment #8808467 - Attachment is obsolete: true
Attachment #8808467 - Flags: review?(jgilbert)
Attachment #8808504 - Flags: review?(jgilbert)
Attached patch check max size (obsolete) — Splinter Review
Sorry, the last patch was wrong. This should be the correct one.
Attachment #8808504 - Attachment is obsolete: true
Attachment #8808504 - Flags: review?(jgilbert)
Attachment #8808921 - Flags: review?(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.
Attachment #8808921 - Flags: review?(jgilbert) → review-
(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.
Attached patch check max size (obsolete) — Splinter Review
I move the check from ValidateTexImageSpecification to ValidateTexImage. So ValidateTexImageSelection (for TexSubImage and CompressedTexSubImage) can do this check too.
Attachment #8808921 - Attachment is obsolete: true
Attachment #8809292 - Flags: review?(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.
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 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.
Attachment #8809292 - Flags: review?(jgilbert) → review-
This crashes on Windows+Intel.
Assignee: ethlin → jgilbert
Duplicate of this bug: 1323641
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
Attachment #8818769 - Flags: review?(ethlin) → review+
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/013060399bd7
Always-too-large level during tex image specification is INVALID_VALUE. - r=ethlin
https://hg.mozilla.org/mozilla-central/rev/013060399bd7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Attachment #8809292 - Attachment is obsolete: true
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]:
Attachment #8818769 - Flags: approval-mozilla-beta?
Attachment #8818769 - Flags: approval-mozilla-aurora?
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.
Attachment #8818769 - Flags: approval-mozilla-beta?
Attachment #8818769 - Flags: approval-mozilla-beta+
Attachment #8818769 - Flags: approval-mozilla-aurora?
Attachment #8818769 - Flags: approval-mozilla-aurora+
Blocks: 1324710
You need to log in before you can comment on or make changes to this bug.