Closed Bug 567421 Opened 10 years ago Closed 9 years ago

Layers: Add basic logging

Categories

(Core :: Graphics, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: cjones, Assigned: cjones)

Details

Attachments

(3 files, 5 obsolete files)

I need to check that layer trees are properly synced up across processes.  stdout log is probably the easiest spot check.  The soon-to-be-attached patch adds logging output like the following when then env var NSPR_LOG_MODULES="Layers:5" is set.

-1239615488[214dab0]: ===== BeginTransaction() ====================
-1239615488[214dab0]: BasicLayerManager (0x339d060)
-1239615488[214dab0]:   Subtree:
-1239615488[214dab0]:   ____
-1239615488[214dab0]:   ContainerLayer (0x33b08d0) {
-1239615488[214dab0]:       visible region .. <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=842)> ]>
-1239615488[214dab0]:       transform ....... <gfx3DMatrix [ 1 0 0 0;  0 1 0 0;  0 0 1 0;  0 0 0 1; ]>
-1239615488[214dab0]:       opacity ......... (unused) 1
-1239615488[214dab0]:       clip rect ....... (unused) <nsIntRect @ (x=0, y=0); size (w=0, h=0)>
-1239615488[214dab0]:   }
-1239615488[214dab0]:     |--____
-1239615488[214dab0]:     |--ThebesLayer (0x33b0a20) {
-1239615488[214dab0]:     |--    visible region .. <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=842)> ]>
-1239615488[214dab0]:     |--    transform ....... <gfx3DMatrix [ 1 0 0 0;  0 1 0 0;  0 0 1 0;  0 0 0 1; ]>
-1239615488[214dab0]:     |--    opacity ......... (unused) 1
-1239615488[214dab0]:     |--    clip rect ....... (unused) <nsIntRect @ (x=0, y=0); size (w=0, h=0)>
-1239615488[214dab0]:     |--    valid region .... <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=32)> <nsIntRect @ (x=0, y=32); size (w=891, h=71)> <nsIntRect @ (x=0, y=103); size (w=247, h=1)> <nsIntRect @ (x=247, y=103); size (w=644, h=18)> <nsIntRect @ (x=0, y=104); size (w=24, h=16)> <nsIntRect @ (x=24, y=104); size (w=223, h=17)> <nsIntRect @ (x=0, y=120); size (w=24, h=1)> <nsIntRect @ (x=0, y=121); size (w=891, h=721)> ]>
-1239615488[214dab0]:     |--}
-1239615488[214dab0]: ----- EndTransaction() --------------------
-1239615488[214dab0]: BasicLayerManager (0x339d060)
-1239615488[214dab0]:   Subtree:
-1239615488[214dab0]:   ____
-1239615488[214dab0]:   ContainerLayer (0x33b08d0) {
-1239615488[214dab0]:       visible region .. <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=842)> ]>
-1239615488[214dab0]:       transform ....... <gfx3DMatrix [ 1 0 0 0;  0 1 0 0;  0 0 1 0;  0 0 0 1; ]>
-1239615488[214dab0]:       opacity ......... (unused) 1
-1239615488[214dab0]:       clip rect ....... (unused) <nsIntRect @ (x=0, y=0); size (w=0, h=0)>
-1239615488[214dab0]:   }
-1239615488[214dab0]:     |--____
-1239615488[214dab0]:     |--ThebesLayer (0x33b0a20) {
-1239615488[214dab0]:     |--    visible region .. <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=842)> ]>
-1239615488[214dab0]:     |--    transform ....... <gfx3DMatrix [ 1 0 0 0;  0 1 0 0;  0 0 1 0;  0 0 0 1; ]>
-1239615488[214dab0]:     |--    opacity ......... (unused) 1
-1239615488[214dab0]:     |--    clip rect ....... (unused) <nsIntRect @ (x=0, y=0); size (w=0, h=0)>
-1239615488[214dab0]:     |--    valid region .... <nsIntRegion [ <nsIntRect @ (x=0, y=0); size (w=891, h=32)> <nsIntRect @ (x=0, y=32); size (w=700, h=26)> <nsIntRect @ (x=887, y=32); size (w=4, h=26)> <nsIntRect @ (x=0, y=58); size (w=891, h=45)> <nsIntRect @ (x=0, y=103); size (w=247, h=1)> <nsIntRect @ (x=247, y=103); size (w=644, h=18)> <nsIntRect @ (x=0, y=104); size (w=24, h=16)> <nsIntRect @ (x=24, y=104); size (w=223, h=17)> <nsIntRect @ (x=0, y=120); size (w=24, h=1)> <nsIntRect @ (x=0, y=121); size (w=891, h=721)> ]>
-1239615488[214dab0]:     |--}
Attached patch Add basic logging for layers (obsolete) — Splinter Review
This patch leaves logging capabilities enabled for opt and debug builds.  The "log-enabled" check is a single branch taken once for BeginTransaction() and once for EndTransaction(*), so the cost should be minimal.
Assignee: nobody → jones.chris.g
Attachment #446783 - Flags: review?(roc)
I've got some layer-dumping code in FrameLayerBuilder in my patch queue ... we need to merge that with this somehow. What do you think?
Attachment #446783 - Attachment is obsolete: true
Attachment #449215 - Flags: review?(roc)
Attachment #446783 - Flags: review?(roc)
These have rotted pretty thoroughly.
Attachment #449215 - Attachment is obsolete: true
Attachment #449216 - Attachment is obsolete: true
Attachment #453552 - Flags: review?(roc)
Attachment #449215 - Flags: review?(roc)
Attachment #449216 - Flags: review?(roc)
Is it useful to have layer-manager-specific layer names? If it is, why bother with names for the base layer classes?
(In reply to comment #8)
> Is it useful to have layer-manager-specific layer names? If it is, why bother
> with names for the base layer classes?

I had in mind coexisting Basic/OGL managers.  Mostly to avoid touching layers/opengl and layers/d3d9, initially ;) (I didn't care about either when I wrote these patches).  But I'd be happy to switch the abstract bases to a new LAYER_DECL_TYPE() macro or somesuch and add DECL_NAME() to the other backends, if you prefer that.
Personally I don't think we need per-layer-manager names. We could have names for the layer managers and then logging code would just get the manager from the layer and then get the manager's name.
Something like |printf("layer name: %s%s", layer->Manager()->TypeName(), layer->Name())| ?  That suits me.
Attachment #453552 - Attachment is obsolete: true
Attachment #453553 - Attachment is obsolete: true
Attachment #453684 - Flags: review?(roc)
Attachment #453552 - Flags: review?(roc)
Attachment #453553 - Flags: review?(roc)
Rob, due to the unholy mess that is our logging system, it turns out that rocket science is needed to conditionally declare these logging functions in a way that compiles on all platforms.  To keep this code simple, do you mind having these symbols always declared?  If not, then we have two options for their implementation: either always define them too, or define no-ops in Layers.cpp under the original |ifdef DEBUG || PR_LOGGING| condition.  Do you have a preference?
Requesting approval2.0 for this logging code because it's

  (1) adds interfaces and a new file that I would like to share between m-c and [mumble] when the cross-process layers work "branches" into [mumble].  Not taking it will cause merge headaches.
  (2) is mostly not compiled for release builds and NPOTB regardless.
Attachment #458961 - Flags: approval2.0?
Attachment #458961 - Flags: approval2.0? → approval2.0+
You need to log in before you can comment on or make changes to this bug.