1089018 - Centralize our logic for spewing from GLContext (and related)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Centralize-GLContext-spew-control.patch

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.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

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?

Comment 3

[Kelsey Gilbert [:jgilbert]]

(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.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Centralize-GLContext-spew-control.patch

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 6

[Kelsey Gilbert [:jgilbert]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8890eab664bf

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8890eab664bf