[PATCH 1/2] DIX: Make PrintWindowTree actually useful

[Peter Hutterer]

sorry, hit the wrong key and the other email got sent too early

On Tue, Jun 14, 2011 at 07:30:48PM +0100, Daniel Stone wrote:
> Rewrite PrintWindowTree to make it actually tell you what you want to
> know.
> 
> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
>  dix/window.c     |  125 ++++++++++++++++++++++++++++++++++++++++++------------
>  include/window.h |    2 +
>  2 files changed, 100 insertions(+), 27 deletions(-)
> 
> diff --git a/dix/window.c b/dix/window.c
> index 5defe58..0643b65 100644
> --- a/dix/window.c
> +++ b/dix/window.c
> @@ -108,6 +108,7 @@ Equipment Corporation.
>  #include "regionstr.h"
>  #include "validate.h"
>  #include "windowstr.h"
> +#include "propertyst.h"
>  #include "input.h"
>  #include "inputstr.h"
>  #include "resource.h"
> @@ -124,10 +125,13 @@ Equipment Corporation.
>  #include "dixevents.h"
>  #include "globals.h"
>  #include "mi.h" /* miPaintWindow */
> +#include "compint.h"
>  
>  #include "privates.h"
>  #include "xace.h"
>  
> +#include <X11/Xatom.h> /* must come after server includes */
> +
>  /******
>   * Window stuff for server 
>   *
> @@ -176,46 +180,113 @@ static Bool TileScreenSaver(ScreenPtr pScreen, int kind);
>  
>  #define SubStrSend(pWin,pParent) (StrSend(pWin) || SubSend(pParent))
>  
> -#ifdef DEBUG
> -/******
> - * PrintWindowTree
> - *    For debugging only
> - ******/
> +static const char *overlay_win_name = "(composite overlay)";

bikeshed: i'd probably prefer <composite overlay> to avoid the
double-parens in the output. and <> is often used to signal special value.

> -static void
> -PrintChildren(WindowPtr p1, int indent)
> +static const char *
> +get_window_name(WindowPtr pWin)
>  {
> -    WindowPtr p2;
> -    int i;
> +    PropertyPtr prop;
> +    CompScreenPtr comp_screen = GetCompScreen(pWin->drawable.pScreen);
> +
> +    if (comp_screen && pWin == comp_screen->pOverlayWin)
> +        return overlay_win_name;
>  
> -    while (p1)
> +    for (prop = wUserProps(pWin); prop; prop = prop->next)
>      {
> -	p2 = p1->firstChild;
> -        ErrorF("[dix] ");
> -	for (i=0; i<indent; i++) ErrorF(" ");
> -	ErrorF("%lx\n", p1->drawable.id);
> -	RegionPrint(&p1->clipList);
> -	PrintChildren(p2, indent+4);
> -	p1 = p1->nextSib;
> +        if (prop->type != XA_STRING || !prop->data ||
> +            prop->propertyName != XA_WM_NAME)
> +            continue;
>      }
> +    if (!prop)
> +        return NULL;
> +
> +    return prop->data;
>  }

I don't seem to have a single window with a name. Can't spot the bug at a
glance though 

fwiw, xprop lists WM_NAME(STRING) = "xeyes" with WM_NAME being XA_WM_NAME

> -static void
> +void
>  PrintWindowTree(void)
>  {
> -    int i;
> -    WindowPtr pWin, p1;
> +    int i, scrnum, depth;
> +    const char *win_name, *visibility;
> +    ScreenPtr pScreen;
> +    WindowPtr pWin;
> +    BoxPtr rects;
>  
> -    for (i=0; i<screenInfo.numScreens; i++)
> +    for (scrnum = 0; scrnum < screenInfo.numScreens; scrnum++)
>      {
> -	ErrorF("[dix] WINDOW %d\n", i);
> -	pWin = screenInfo.screens[i]->root;
> -	RegionPrint(&pWin->clipList);
> -	p1 = pWin->firstChild;
> -	PrintChildren(p1, 4);
> +        pScreen = screenInfo.screens[scrnum];
> +        ErrorF("[dix] Dumping windows for screen %d (pixmap %lx):\n", scrnum,
> +               pScreen->GetScreenPixmap(pScreen)->drawable.id);
> +        pWin = pScreen->root;
> +        depth = 1;
> +        while (pWin)

I'm not a big fan of nesting two loops. Why not move this out into a
separate function, it'd make the code much easier to understand.
Or at least move the printing code out and leave the looping code here, so
the code ends up being:

for each screen {
        PrintWindow(win)
        win = next sibling or child
}

with all the ErrorF in PrintWindow().

> +        {
> +            for (i = 0; i < (depth << 2); i++)
> +                ErrorF(" ");
> +
> +            win_name = get_window_name(pWin);
> +            ErrorF("win %lx (%s), [%d, %d] to [%d, %d]",
> +                   pWin->drawable.id,
> +                   win_name ? win_name : "no name",
> +                   pWin->drawable.x, pWin->drawable.y,
> +                   pWin->drawable.x + pWin->drawable.width,
> +                   pWin->drawable.y + pWin->drawable.height);
> +
> +            if (pWin->overrideRedirect)
> +                ErrorF(" (override redirect)");
> +            if (pWin->redirectDraw)
> +                ErrorF(" (%s compositing: pixmap %lx)",
> +                       (pWin->redirectDraw == RedirectDrawAutomatic) ?
> +                        "automatic" : "manual",
> +                       pScreen->GetWindowPixmap(pWin)->drawable.id);
> +
> +            switch (pWin->visibility)
> +            {
> +            case VisibilityUnobscured:
> +                 visibility = "unobscured";
> +                 break;
> +            case VisibilityPartiallyObscured:
> +                 visibility = "partially obscured";
> +                 break;
> +            case VisibilityFullyObscured:
> +                 visibility = "fully obscured";
> +                 break;
> +            case VisibilityNotViewable:
> +                 visibility = "unviewable";
> +                 break;
> +            }
> +            ErrorF(", %s", visibility);
> +
> +            if (REGION_NOTEMPTY(pScreen, &pWin->clipList))
> +            {
> +                ErrorF(", clip list:");
> +                rects = REGION_RECTS(&pWin->clipList);
> +                for (i = 0; i < REGION_NUM_RECTS(&pWin->clipList); i++)
> +                    ErrorF(" [(%d, %d) to (%d, %d)]",
> +                           rects[i].x1, rects[i].y1,
> +                           rects[i].x2, rects[i].y2);
> +                ErrorF("; extents [(%d, %d) to (%d, %d)]",
> +                       pWin->clipList.extents.x1, pWin->clipList.extents.y1,
> +                       pWin->clipList.extents.x2, pWin->clipList.extents.y2);
> +            }


ErrorF("\n") is missing here, the output is a tad hard to parse otherwise

Cheers,
  Peter

> +
> +            if (pWin->firstChild)
> +            {
> +                pWin = pWin->firstChild;
> +                depth++;
> +                continue;
> +            }
> +            while (pWin && !pWin->nextSib)
> +            {
> +                pWin = pWin->parent;
> +                depth--;
> +            }
> +            if (!pWin)
> +                break;
> +            pWin = pWin->nextSib;
> +	}
>      }
>  }
> -#endif
>  
>  int
>  TraverseTree(WindowPtr pWin, VisitWindowProcPtr func, pointer data)
> diff --git a/include/window.h b/include/window.h
> index 1e4e9bf..e13598b 100644
> --- a/include/window.h
> +++ b/include/window.h
> @@ -267,4 +267,6 @@ extern _X_EXPORT void EnableMapUnmapEvents(
>      WindowPtr /* pWin */ );
>  
>  extern _X_EXPORT void SetRootClip(ScreenPtr pScreen, Bool enable);
> +extern _X_EXPORT void PrintWindowTree(void);
> +
>  #endif /* WINDOW_H */
> -- 
> 1.7.5.3
>