Default to MOZ_GL_DEBUG_ABORT_ON_ERROR=1 for WebGL on DEBUG builds

RESOLVED FIXED in Firefox 50

Status

()

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

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

49 Branch
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

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

Attachments

(9 attachments)

(Assignee)

Description

2 years ago
We should definitely run MOZ_GL_DEBUG_ABORT_ON_ERROR=1 when running CI tests on DEBUG builds.
(Assignee)

Comment 1

2 years ago
Created attachment 8760521 [details]
Bug 1278403 - Clean up gfxEnv.h -

Review commit: https://reviewboard.mozilla.org/r/58104/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58104/
Attachment #8760521 - Flags: review?(milan)
Attachment #8760522 - Flags: review?(jmuizelaar)
Attachment #8760523 - Flags: review?(jmuizelaar)
Attachment #8760524 - Flags: review?(jmuizelaar)
Attachment #8760525 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

2 years ago
Created attachment 8760522 [details]
Bug 1278403 - Spread CreateContextFlags to GLContext::ctor. -

Review commit: https://reviewboard.mozilla.org/r/58106/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58106/
(Assignee)

Comment 3

2 years ago
Created attachment 8760523 [details]
Bug 1278403 - Choose GLContext::DebugFlags based on ContextCreateFlags. -

Review commit: https://reviewboard.mozilla.org/r/58108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58108/
Attachment #8760521 - Flags: review?(milan) → review+
Attachment #8760524 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 9

2 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #7)
> https://reviewboard.mozilla.org/r/58112/#review55772
> 
> Why?

We do all validation in our WebGL layer, we only really generate GL_OOM from the driver, no other errors.

This is opposed to SkiaGL, which may or may not rely on errors returned from the driver. This is why I am not just turning on ABORT_ON_ERROR for all GLContexts.

(Also, later, we should be creating no-error contexts on non-DEBUG builds, so the driver doesn't replicate our validation. ANGLE already supports this)
(Assignee)

Updated

2 years ago
Flags: needinfo?(jmuizelaar)
(In reply to Jeff Gilbert [:jgilbert] from comment #9)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #7)
> > https://reviewboard.mozilla.org/r/58112/#review55772
> > 
> > Why?
> 
> We do all validation in our WebGL layer, we only really generate GL_OOM from
> the driver, no other errors.
> 
> This is opposed to SkiaGL, which may or may not rely on errors returned from
> the driver. This is why I am not just turning on ABORT_ON_ERROR for all
> GLContexts.
> 
> (Also, later, we should be creating no-error contexts on non-DEBUG builds,
> so the driver doesn't replicate our validation. ANGLE already supports this)

This would be valuable information to put in the commit message.
Flags: needinfo?(jmuizelaar)
Attachment #8760523 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8760523 [details]
Bug 1278403 - Choose GLContext::DebugFlags based on ContextCreateFlags. -

https://reviewboard.mozilla.org/r/58108/#review56032
Comment on attachment 8760525 [details]
Bug 1278403 - WebGL doesn't need validation. -

https://reviewboard.mozilla.org/r/58112/#review56038
Attachment #8760525 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8760522 [details]
Bug 1278403 - Spread CreateContextFlags to GLContext::ctor. -

https://reviewboard.mozilla.org/r/58106/#review56074
Attachment #8760522 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 14

2 years ago
Created attachment 8763111 [details]
Bug 1278403 - Fix formatting.

Review commit: https://reviewboard.mozilla.org/r/59440/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59440/
Attachment #8763112 - Flags: review?(jmuizelaar)
Attachment #8763113 - Flags: review?(jmuizelaar)
(Assignee)

Comment 16

2 years ago
Created attachment 8763113 [details]
Bug 1278403 - Disable tests on DEBUG with suspected spurious-error-reporting drivers. -

Review commit: https://reviewboard.mozilla.org/r/59444/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59444/
(Assignee)

Comment 17

2 years ago
Comment on attachment 8760521 [details]
Bug 1278403 - Clean up gfxEnv.h -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58104/diff/1-2/
(Assignee)

Comment 18

2 years ago
Comment on attachment 8760522 [details]
Bug 1278403 - Spread CreateContextFlags to GLContext::ctor. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58106/diff/1-2/
(Assignee)

Comment 19

2 years ago
Comment on attachment 8760523 [details]
Bug 1278403 - Choose GLContext::DebugFlags based on ContextCreateFlags. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58108/diff/1-2/
(Assignee)

Comment 20

2 years ago
Comment on attachment 8760524 [details]
Bug 1278403 - Misc code cleanup. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58110/diff/1-2/
(Assignee)

Comment 21

2 years ago
Comment on attachment 8760525 [details]
Bug 1278403 - WebGL doesn't need validation. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58112/diff/1-2/
(Assignee)

Comment 22

2 years ago
Created attachment 8763753 [details]
Bug 1278403 - Clarify crash message in case anyone unfamiliar runs into this. -

Review commit: https://reviewboard.mozilla.org/r/59902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59902/
Attachment #8763753 - Flags: review?(jmuizelaar)
https://reviewboard.mozilla.org/r/59444/#review56906

Why don't we want to investigate/fix these? They seem like the kind of thing this patchset was designed to catch.
(Assignee)

Comment 24

2 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #23)
> https://reviewboard.mozilla.org/r/59444/#review56906
> 
> Why don't we want to investigate/fix these? They seem like the kind of thing
> this patchset was designed to catch.

Yes and no. This is primarily to prevent regressing from our current state. It's also handy to identify these, but these should be investigated after this bug lands. (relatively low priority, though)
Comment on attachment 8763113 [details]
Bug 1278403 - Disable tests on DEBUG with suspected spurious-error-reporting drivers. -

https://reviewboard.mozilla.org/r/59444/#review56926
Attachment #8763113 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8763753 [details]
Bug 1278403 - Clarify crash message in case anyone unfamiliar runs into this. -

https://reviewboard.mozilla.org/r/59902/#review56930
Attachment #8763753 - Flags: review?(jmuizelaar) → review+
Attachment #8763112 - Flags: review?(jmuizelaar) → review+

Comment 29

2 years ago
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b2f6a194063
Clean up gfxEnv.h - r=milan
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1b7a4207378
Spread CreateContextFlags to GLContext::ctor. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c958e7309e7
Fix formatting.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5383918f68f
Choose GLContext::DebugFlags based on ContextCreateFlags. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfb9047aaa22
Misc code cleanup. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/43de2b8ce001
WebGL doesn't need validation. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e482a936dbc
Mark now-too-slow tests. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9cdd2062581
Disable tests on DEBUG with suspected spurious-error-reporting drivers. - r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/496586403d9d
Clarify crash message in case anyone unfamiliar runs into this. - r=jrmuizel
You need to log in before you can comment on or make changes to this bug.