Closed Bug 1035801 Opened 7 years ago Closed 7 years ago

Switch tiling logging from PR logging to printf_stderr

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: kats, Assigned: kats)

Details

Attachments

(1 file)

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.
Attached patch PatchSplinter Review
Attachment #8452356 - Flags: review?(chrislord.net)
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.
Attachment #8452356 - Flags: review?(chrislord.net) → review+
(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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
https://hg.mozilla.org/mozilla-central/rev/8df1affbbde7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.