Debugging code to print layer tree

Added some debugging code to print layer tree.

Also added a name attribute to the layer. IF !NDEBUG is defined, layer takes on the name of the view/window which created it

Please comment as to where the debug_utils.cc should be located, it feels like the wrong place.

I have another patch to print the window tree which I'll submit for review once I get this one in.

I think this is a bit simpler and more useful than http://codereview.chromium.org/8366008/

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110528

[tfarina, 2011-11-09 14:30:50]

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc#newcode78
ui/aura/window.cc:78: #if !defined(NDEBUG)
#ifndef NDEBUG ? one or the other for consistency?

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc#newcode80
ui/aura/window.cc:80: if (layer_name.size() == 0)
size() == 0 -> empty()

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.cc
File ui/gfx/compositor/debug_utils.cc (right):

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.c...
ui/gfx/compositor/debug_utils.cc:13: #include "ui/gfx/compositor/layer.h"
sort in alphabetical order.

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.c...
ui/gfx/compositor/debug_utils.cc:15: #include <iomanip>
C++ header includes should become before the project includes.

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.c...
ui/gfx/compositor/debug_utils.cc:88: } // namespace ui
blank line here too please.

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.h
File ui/gfx/compositor/debug_utils.h (right):

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.h...
ui/gfx/compositor/debug_utils.h:5: #ifndef UI_DEBUG_UTILS_H_
UI_GFX_COMPOSITOR_DEBUG_UTILS_H_

http://codereview.chromium.org/8508009/diff/1/ui/gfx/compositor/debug_utils.h...
ui/gfx/compositor/debug_utils.h:17: } // namespace ui
please add a blank line above. and between #endif // NDEBUG too.

[Ben Goodger (Google), 2011-11-09 17:03:51]

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc#newcode85
ui/aura/window.cc:85: base::snprintf(id_buf, sizeof(id_buf), " %d", id());
Can you do all of this in the debug util rather than adding it to the window
type?

[pkotwicz, 2011-11-09 18:56:02]

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8508009/diff/1/ui/aura/window.cc#newcode85
ui/aura/window.cc:85: base::snprintf(id_buf, sizeof(id_buf), " %d", id());
I could do this. But I would be assuming that the window tree is 1:1 with the
layer tree

[tfarina, 2011-11-11 18:46:28]

lgtm, but you need OWNERS approval anyway. Just some nits below. Thanks!

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode78
ui/aura/window.cc:78: #if ndef NDEBUG
nit: #ifndef

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode79
ui/aura/window.cc:79: std::string layer_name(name());
nit: name() -> name_

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode83
ui/aura/window.cc:83: if (id() != -1) {
nit: id() -> id_

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode85
ui/aura/window.cc:85: base::snprintf(id_buf, sizeof(id_buf), " %d", id());
nit: id() -> id_

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/debug_util...
File ui/gfx/compositor/debug_utils.cc (right):

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/debug_util...
ui/gfx/compositor/debug_utils.cc:58: if (translation != gfx::Point(0, 0)) {
nit: gfx::Point(0, 0) -> gfx::Point()?

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/debug_util...
ui/gfx/compositor/debug_utils.cc:79: for (size_t i = 0, count =
layer->children().size(); i < count; i++)
do we really need to cache size() here? is for performance? I really don't care
to much, just wondering.

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/debug_util...
ui/gfx/compositor/debug_utils.cc:79: for (size_t i = 0, count =
layer->children().size(); i < count; i++)
++i

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/debug_util...
ui/gfx/compositor/debug_utils.cc:83: } // namespace
nit: there should be two spaces between } and // not just one.

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/layer.h
File ui/gfx/compositor/layer.h (right):

http://codereview.chromium.org/8508009/diff/6001/ui/gfx/compositor/layer.h#ne...
ui/gfx/compositor/layer.h:309: std::string name_;
nit: add doc?

[tfarina, 2011-11-11 18:48:38]

sorry for the noise, just one more.

http://codereview.chromium.org/8508009/diff/6001/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/8508009/diff/6001/views/view.cc#newcode1784
views/view.cc:1784: #if !defined(NDEBUG)
nit: #ifndef ?

[Ben Goodger (Google), 2011-11-14 22:59:08]

LGTM. Sorry for the delay.

Can you add a hotkey to the shell to do this? :-) (can be a separate change)

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode78
ui/aura/window.cc:78: #if ndef NDEBUG
mostly we do !defined()

http://codereview.chromium.org/8508009/diff/6001/ui/aura/window.cc#newcode86
ui/aura/window.cc:86: layer_name.append(id_buf);
I agree with tfarina's nits here, prevalent style is to use the member vars
directly.

[pkotwicz, 2011-11-15 21:07:28]

What file should I look at for hotkeys? It will save me some time in code search
=]

[Ben Goodger (Google), 2011-11-15 21:23:09]

You can just shove this in Desktop for now near the rotation stuff, or if
mazda beats you to it, wherever he moved the rotation stuff.

-Ben

On Tue, Nov 15, 2011 at 1:07 PM, <pkotwicz@chromium.org> wrote:

> What file should I look at for hotkeys? It will save me some time in code
> search
> =]
>
>
>
>
http://codereview.chromium.**org/8508009/<http://codereview.chromium.org/8508...
>