1035801 - Switch tiling logging from PR logging to printf_stderr

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

When I added the tiling logging code in bug 1018387 B2G builds were nonunified, so you could #define FORCE_PR_LOG in any .cpp file. However now B2G builds are unified and using FORCE_PR_LOG is much more of a pain; the files have to be moved to a non-unified section of moz.build. The only other way to enable the PR logging is by doing a debug build, AFAIK, which is horribly slow on B2G.

For ease of use I'd rather just stop using nspr logging for the tiling code and just have a simple #define that I can flip on and off.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 2

[Chris Lord [:cwiiis]]

Comment on attachment 8452356 [details] [diff] [review]
Patch

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

LGTM.

::: gfx/layers/TiledLayerBuffer.h
@@ +29,5 @@
> +// changing the 0 to a 1 in the following #define.
> +#define ENABLE_TILING_LOG 0
> +
> +#if ENABLE_TILING_LOG
> +#  define TILING_LOG(_args) printf_stderr _args ;

What's with the missing brackets after printf_stderr? I assume this works and there's something here I don't understand.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

(In reply to Chris Lord [:cwiiis] from comment #2)
> Comment on attachment 8452356 [details] [diff] [review]
> Patch
> 
> Review of attachment 8452356 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM.
> 
> ::: gfx/layers/TiledLayerBuffer.h
> @@ +29,5 @@
> > +// changing the 0 to a 1 in the following #define.
> > +#define ENABLE_TILING_LOG 0
> > +
> > +#if ENABLE_TILING_LOG
> > +#  define TILING_LOG(_args) printf_stderr _args ;
> 
> What's with the missing brackets after printf_stderr? I assume this works
> and there's something here I don't understand.

The call sites have two sets of parens, so when the macro is expanded it all works out. All the NSPR log stuff works the same way.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8df1affbbde7

Comment 5

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> (In reply to Chris Lord [:cwiiis] from comment #2)
> > Comment on attachment 8452356 [details] [diff] [review]
> > Patch
> > 
> > Review of attachment 8452356 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > LGTM.
> > 
> > ::: gfx/layers/TiledLayerBuffer.h
> > @@ +29,5 @@
> > > +// changing the 0 to a 1 in the following #define.
> > > +#define ENABLE_TILING_LOG 0
> > > +
> > > +#if ENABLE_TILING_LOG
> > > +#  define TILING_LOG(_args) printf_stderr _args ;
> > 
> > What's with the missing brackets after printf_stderr? I assume this works
> > and there's something here I don't understand.
> 
> The call sites have two sets of parens, so when the macro is expanded it all
> works out. All the NSPR log stuff works the same way.

It might be nice to document this at the definition site of TILING_LOG, as PR_LOG does.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8df1affbbde7