Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

({dev-doc-complete})

Trunk
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

This patch adds a OpenGL debug mode.

* This only has an effect on DEBUG builds. Non-DEBUG builds should be essentially unaffected (just a couple more unused PRPackedBool's in GLContext)

* When using a DEBUG build, you can enable the OpenGL debug mode by defining the environment variable MOZ_GL_DEBUG. This has the following effects on every GL call:
   1. Checks that the GL context is current. This uses a thread-local static variable. Aborts otherwise.
   2. Calls glFinish() after every GL call. This turns OpenGL into a synchronous API. Thanks to that, in case of a crash in the Firefox process caused by a GL call, the stack trace that we get becomes useful.
   3. Checks that no GL error was generated. Prints an error message otherwise.

* An extra verbose mode can be enabled by defining the env var MOZ_GL_DEBUG_VERBOSE. It causes messages to be printed before and after every GL call. This is useful to debug graphics system crashes/lockups that make the computer unusable.

* An extra pedantic mode can be enabled by defining the env var MOZ_GL_DEBUG_ABORT_ON_ERROR. It causes us to abort as soon as a GL error is generated. This is useful in the common case where we know that no error should be generated. It must be noted though that some valid WebGL code may generate OpenGL errors (although most WebGL errors are handled by us before calling OpenGL).

The extra verbose/pedantic modes imply the regular debug mode.

Not adding a pref for this, as it's only a debugging tool and mostly a one-shot tool.

Notice that this patch reimplements GL functions as inline methods in GLContext. Notice that being implemented inline in the class body, the 'inline' keyword should be implicit for them.

Notice that the GL error handling means that we reimplement glGetError.

This patch replaces the patch from bug 595151. No multiple inheritance anymode, instead we have a member struct mSymbols.
(Assignee)

Comment 1

7 years ago
Created attachment 476665 [details] [diff] [review]
OpenGL debug mode
Assignee: nobody → bjacob
Status: NEW → ASSIGNED
Attachment #476665 - Flags: review?(vladimir)
Attachment #476665 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Duplicate of this bug: 595151
(Assignee)

Comment 3

7 years ago
Created attachment 478371 [details] [diff] [review]
updated patch

Updated patch:
 - support Windows
 - adapt to recent changes, especially the Scissor / Viewport stuff

Will do the generate-stuff-with-a-script changes and then will ask for review.
Attachment #476665 - Attachment is obsolete: true
Attachment #476665 - Flags: review?(vladimir)
Attachment #476665 - Flags: approval2.0?
(Assignee)

Comment 4

7 years ago
Created attachment 480252 [details] [diff] [review]
patch v3

Here's a new version. Still no automatic python generation, but made it compile on all platforms by removing the thread-local stuff, adding an assertion instead that all GL calls are made from the main thread. Of course this is inessential so if we eventually need to do GL calls from other threads, we'd need to fix this. Comments added.
Attachment #478371 - Attachment is obsolete: true
Attachment #480252 - Flags: review?(vladimir)
(Assignee)

Updated

7 years ago
Blocks: 601601
Created attachment 480989 [details] [diff] [review]
some small tweaks

some small tweaks to this -- mainly moving things to a .h file, and changing some variables and stuff around
Attachment #480989 - Flags: review?(bjacob)
(Assignee)

Comment 6

7 years ago
What happened to ZeroBase? It disappeared, but you are still calling Zero() though (compilation error).

ZeroBase probably didn't have a good name, but it was already useful not only for the symbols, also for the ContextFormat class.
Created attachment 481054 [details] [diff] [review]
updated and compiling now!

Updated and fixed.. yeah, I nuked ZeroBase -- it had some utility, but it's clearer/easier to just call memset in the two places where it's relevant rather than introduce another base class that people have to figure out when reading the code.  :-)
Attachment #480989 - Attachment is obsolete: true
Attachment #480989 - Flags: review?(bjacob)
(Assignee)

Updated

7 years ago
Attachment #481054 - Flags: review+
(Assignee)

Comment 8

7 years ago
Created attachment 481878 [details] [diff] [review]
updated to apply again

trivial update.
Attachment #481878 - Flags: review+

Updated

7 years ago
Blocks: 594357
Attachment #481878 - Flags: approval2.0+
(Assignee)

Comment 9

7 years ago
Pushed at last!

http://hg.mozilla.org/mozilla-central/rev/d3f448b20f20
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Depends on: 603821
This busted non-libxul debug builds -- filed as Bug 603821.
Keywords: dev-doc-needed
Now documented:

https://developer.mozilla.org/En/Debugging/Debugging_OpenGL

Linked from the main Debugging page at:

https://developer.mozilla.org/En/Debugging
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Updated

6 years ago
Attachment #480252 - Flags: review?(vladimir)
You need to log in before you can comment on or make changes to this bug.