Actually stop script upon GL error




8 years ago
8 years ago


(Reporter: bjacob, Unassigned)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

Currently, SynthesizeGLError is returning NS_OK, which lets the script continue after an error. For example, this means that we'll let a script do a DrawArrays call with a too small array, which results in a crash in case of software rendering (experienced here).

Comment 1

8 years ago
Created attachment 446857 [details] [diff] [review]
Actually stop script upon error
Attachment #446857 - Flags: review?(vladimir)
Comment on attachment 446857 [details] [diff] [review]
Actually stop script upon error

no, bad -- GL errors are asynchronous, and you're supposed to be able to continue making gl calls even with an outstanding error.

The DrawArrays crash is separate and doesn't depend on an error condition -- it just means we're not properly validating the currently-bound VBOs.  (Hmm, and it also might mean that we're not tracking bindings correctly in the face of errors.)
Attachment #446857 - Flags: review?(vladimir) → review-

Comment 3

8 years ago
Oh right, I understand, sorry!

But this DrawArrays function is extremely unsafe, as here (OSMesa rendering) it's crashing when I try to draw an array bigger than the vertex attribute buffers.

So I see 2 possible ways this can be changed, tell me what you prefer:

1) DrawArrays uses an assertion there, so it aborts instead of emitting a GL error.

2) DrawArray gracefully handles this situation, either by drawing nothing at all, or by drawing only as much as there is in the vertex attribute buffers.

Comment 4

8 years ago
ouch, I guess that 1) is bad as it allows a script to crash firefox, even if that's in a safe way (assertion).

So I'll be preparing a patch for 2). Do you prefer drawing nothing at all, or drawing as much as we have in the vertex attrib buffers?

Comment 5

8 years ago
Ok, sorry, this was an invalid report. Opening another report about the crash I'm actually getting.
Last Resolved: 8 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.