WebGL : vertex attribute array 0 should be disable by default

RESOLVED FIXED in mozilla24

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Guillaume Abadie, Assigned: Guillaume Abadie)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.11 KB, patch
bjacob
: review+
RyanVM
: checkin+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
vertex attribute array 0 should be disable by default
(Assignee)

Updated

4 years ago
Assignee: nobody → gabadie
(Assignee)

Comment 1

4 years ago
Created attachment 762323 [details] [diff] [review]
patch

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

Comment 3

4 years ago
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.
(Assignee)

Comment 6

4 years ago
Created attachment 763547 [details] [diff] [review]
patch for landing

enhancing the patch comment for landing
Attachment #762323 - Attachment is obsolete: true
Attachment #763547 - Flags: review?(bjacob)
Attachment #763547 - Flags: review?(bjacob) → review+
Attachment #763547 - Flags: checkin?
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b5816c7fa6e
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.