Closed Bug 882890 Opened 6 years ago Closed 6 years ago

WebGL : vertex attribute array 0 should be disable by default

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: guillaume.abadie, Assigned: guillaume.abadie)

Details

Attachments

(1 file, 1 obsolete file)

vertex attribute array 0 should be disable by default
Assignee: nobody → gabadie
Attached patch patch (obsolete) — Splinter Review
patch fixing the issue and get passing a WebGL conformance test 1.0.2
Attachment #762323 - Flags: review?(bjacob)
Attachment #762323 - Attachment is patch: true
Attachment #762323 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 762323 [details] [diff] [review]
patch

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

This doesn't fix what you say it fixes. Rather, it just pretends that the GL state is set properly. Is there a reason we can't just use the driver's value?
Actually it fix it :

In the WebGL conformance test https://www.khronos.org/registry/webgl/conformance-suites/1.0.2/conformance/state/gl-get-calls.html , it fix the only one FAIL remaining "context.getVertexAttrib(ii, context.VERTEX_ATTRIB_ARRAY_ENABLED) should be false. Was true.". This test just check if all vertex attribute are disable right after the WebGL context creation.

Actually, desktop OpenGL driver don't draw anything when the vertex attribute 0 is disable. But WebGL specifications authorize to draw with the vertex attribute 0 disabled. Therefor the WebGL layer always enable it, and fake the disable action of it. But this fake technique has been forgotten in that function. So this patch fix it, passing and the conformance test. That why this patch is very tiny.
Alright, please leave a comment there saying as much.
Attachment #762323 - Flags: review?(bjacob) → review+
(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> Comment on attachment 762323 [details] [diff] [review]
> patch
> 
> Review of attachment 762323 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This doesn't fix what you say it fixes. Rather, it just pretends that the GL
> state is set properly. Is there a reason we can't just use the driver's
> value?

The OpenGL value is different from the WebGL one because of vertex attrib 0 on desktop GL.
enhancing the patch comment for landing
Attachment #762323 - Attachment is obsolete: true
Attachment #763547 - Flags: review?(bjacob)
Attachment #763547 - Flags: review?(bjacob) → review+
Comment on attachment 763547 [details] [diff] [review]
patch for landing

Please just use checkin-needed in the future :)
Attachment #763547 - Flags: checkin? → checkin+
https://hg.mozilla.org/mozilla-central/rev/9b5816c7fa6e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.