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

RESOLVED FIXED in Firefox 51

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: ethlin, Assigned: jgilbert)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 disabled, firefox51 fixed, firefox52 fixed, firefox53 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

a year ago
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.
(Reporter)

Comment 1

a year ago
Created attachment 8808467 [details] [diff] [review]
check max size

We should check the max texture size to return correct error type.
Attachment #8808467 - Flags: review?(jgilbert)
(Reporter)

Comment 2

a year ago
Created attachment 8808504 [details] [diff] [review]
check max size

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)
(Reporter)

Comment 3

a year ago
Created attachment 8808921 [details] [diff] [review]
check max size

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)
(Assignee)

Comment 4

a year ago
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-
(Reporter)

Comment 5

a year ago
(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.
(Reporter)

Comment 6

a year ago
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.
Attachment #8808921 - Attachment is obsolete: true
Attachment #8809292 - Flags: review?(jgilbert)
(Assignee)

Comment 7

a year ago
(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.
(Assignee)

Comment 8

a year ago
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.
(Assignee)

Comment 9

a year ago
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-
(Assignee)

Updated

11 months ago
Blocks: 1281250
(Assignee)

Comment 10

11 months ago
This crashes on Windows+Intel.
(Assignee)

Comment 11

11 months ago
Half the issue is the test is wrong:
https://github.com/KhronosGroup/WebGL/pull/2207
(Assignee)

Updated

11 months ago
Assignee: ethlin → jgilbert
Comment hidden (mozreview-request)

Updated

11 months ago
Duplicate of this bug: 1323641
(Reporter)

Comment 14

11 months ago
mozreview-review
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+

Comment 15

11 months ago
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

Comment 16

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/013060399bd7
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Updated

11 months ago
Attachment #8809292 - Attachment is obsolete: true
(Assignee)

Comment 17

11 months ago
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?
(Assignee)

Updated

11 months ago
status-firefox50: --- → disabled
status-firefox51: --- → affected
status-firefox52: --- → affected

Comment 18

11 months ago
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+
(Assignee)

Comment 19

11 months ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c6107c5fe98e
status-firefox52: affected → fixed
(Assignee)

Comment 20

11 months ago
https://hg.mozilla.org/releases/mozilla-beta/rev/9e8e044f7b78
status-firefox51: affected → fixed
(Assignee)

Updated

11 months ago
Blocks: 1324710
You need to log in before you can comment on or make changes to this bug.