Closed Bug 1089018 Opened 7 years ago Closed 7 years ago

Centralize our logic for spewing from GLContext (and related)

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jgilbert, Assigned: jgilbert)

Details

Attachments

(1 file, 1 obsolete file)

We have a lot of printfs, some of which are enabled when you enter MOZ_GL_DEBUG mode, some just in DEBUG builds.

Instead, let's just have an env var to control whether or not to spew.
Attachment #8511425 - Flags: review?(jmuizelaar)
Comment on attachment 8511425 [details] [diff] [review]
0001-Centralize-GLContext-spew-control.patch

Benoit Jacob is more likely to have an opinion on this than me.
Attachment #8511425 - Flags: review?(jmuizelaar) → review?(bjacob)
Comment on attachment 8511425 [details] [diff] [review]
0001-Centralize-GLContext-spew-control.patch

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

Some questions.

::: gfx/gl/GLContext.cpp
@@ +2459,5 @@
>  
> +/*static*/ bool
> +GLContext::ShouldSpew()
> +{
> +    return PR_GetEnv("MOZ_GL_SPEW");

Is it OK to check the environment variable everytime? It seems like it would be a lot cheaper to store a boolean.

How does MOZ_GL_DEBUG_VERBOSE work with that?
Attachment #8511425 - Flags: review?(bjacob) → review-
(In reply to Benoit Jacob [:bjacob] from comment #2)
> Comment on attachment 8511425 [details] [diff] [review]
> 0001-Centralize-GLContext-spew-control.patch
> 
> Review of attachment 8511425 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Some questions.
> 
> ::: gfx/gl/GLContext.cpp
> @@ +2459,5 @@
> >  
> > +/*static*/ bool
> > +GLContext::ShouldSpew()
> > +{
> > +    return PR_GetEnv("MOZ_GL_SPEW");
> 
> Is it OK to check the environment variable everytime? It seems like it would
> be a lot cheaper to store a boolean.
No, we should cache it.
> 
> How does MOZ_GL_DEBUG_VERBOSE work with that?
It's different, because it's a whole different level of spew.
I intend to keep it functioning as it is today, unchanged.
Attachment #8511425 - Attachment is obsolete: true
Attachment #8512159 - Flags: review?(bjacob)
Comment on attachment 8512159 [details] [diff] [review]
0001-Centralize-GLContext-spew-control.patch

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

Sorry about dropping this. You'll need another reviewer now.
Attachment #8512159 - Flags: review?(bjacob)
Attachment #8512159 - Flags: review?(jmuizelaar)
Attachment #8512159 - Flags: review?(jmuizelaar) → review+
OS: Linux → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/8890eab664bf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.