[PATCH 1/8] dix: Remove cleverness from MoveWindowInStack

Jasper St. Pierre jstpierre at mecheye.net
Tue Aug 20 12:45:24 PDT 2013


Should you have to bump the ABI version for this? It seems
MoveWindowInStack is exported. Or will you bump it after the series is
done, at release time?


On Tue, Aug 20, 2013 at 3:33 PM, Adam Jackson <ajax at redhat.com> wrote:

> This looks like it removes an optimisation, but this turns out not to be
> the case!  For 1024x768x32 Xvfb on a 1.7GHz Ivybridge:
>
>       before      after
>     --------  --------- --------   --------------------------
>     204437.1  1001917.1 (  4.90)   Circulate window (4 kids)
>     130656.5   971734.9 (  7.44)   Circulate window (16 kids)
>     127631.0   934420.7 (  7.32)   Circulate window (25 kids)
>     122622.6   843778.1 (  6.88)   Circulate window (50 kids)
>     116319.1   778275.1 (  6.69)   Circulate window (75 kids)
>     109929.4   723284.9 (  6.58)   Circulate window (100 kids)
>      83153.3   504731.3 (  6.07)   Circulate window (200 kids)
>
> So let's just tear that out, and in doing so, remove a #ifdef ROOTLESS
> from dix code.
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  dix/window.c     | 82
> ++++----------------------------------------------------
>  include/window.h |  4 +--
>  mi/mioverlay.c   |  8 +++---
>  mi/miwindow.c    |  8 +++---
>  4 files changed, 15 insertions(+), 87 deletions(-)
>
> diff --git a/dix/window.c b/dix/window.c
> index 8950f97..4e3da64 100644
> --- a/dix/window.c
> +++ b/dix/window.c
> @@ -1513,85 +1513,17 @@ GetWindowAttributes(WindowPtr pWin, ClientPtr
> client,
>      wa->visualID = wVisual(pWin);
>  }
>
> -WindowPtr
> +void
>  MoveWindowInStack(WindowPtr pWin, WindowPtr pNextSib)
>  {
> -    WindowPtr pParent = pWin->parent;
> -    WindowPtr pFirstChange = pWin;      /* highest window where list
> changes */
> -
> -    if (pWin->nextSib != pNextSib) {
> -        WindowPtr pOldNextSib = pWin->nextSib;
> -
> -        if (!pNextSib) {        /* move to bottom */
> -            if (pParent->firstChild == pWin)
> -                pParent->firstChild = pWin->nextSib;
> -            /* if (pWin->nextSib) *//* is always True: pNextSib == NULL
> -             * and pWin->nextSib != pNextSib
> -             * therefore pWin->nextSib != NULL */
> -            pFirstChange = pWin->nextSib;
> -            pWin->nextSib->prevSib = pWin->prevSib;
> -            if (pWin->prevSib)
> -                pWin->prevSib->nextSib = pWin->nextSib;
> -            pParent->lastChild->nextSib = pWin;
> -            pWin->prevSib = pParent->lastChild;
> -            pWin->nextSib = NullWindow;
> -            pParent->lastChild = pWin;
> -        }
> -        else if (pParent->firstChild == pNextSib) {     /* move to top */
> -            pFirstChange = pWin;
> -            if (pParent->lastChild == pWin)
> -                pParent->lastChild = pWin->prevSib;
> -            if (pWin->nextSib)
> -                pWin->nextSib->prevSib = pWin->prevSib;
> -            if (pWin->prevSib)
> -                pWin->prevSib->nextSib = pWin->nextSib;
> -            pWin->nextSib = pParent->firstChild;
> -            pWin->prevSib = (WindowPtr) NULL;
> -            pNextSib->prevSib = pWin;
> -            pParent->firstChild = pWin;
> -        }
> -        else {                  /* move in middle of list */
> -
> -            WindowPtr pOldNext = pWin->nextSib;
> -
> -            pFirstChange = NullWindow;
> -            if (pParent->firstChild == pWin)
> -                pFirstChange = pParent->firstChild = pWin->nextSib;
> -            if (pParent->lastChild == pWin) {
> -                pFirstChange = pWin;
> -                pParent->lastChild = pWin->prevSib;
> -            }
> -            if (pWin->nextSib)
> -                pWin->nextSib->prevSib = pWin->prevSib;
> -            if (pWin->prevSib)
> -                pWin->prevSib->nextSib = pWin->nextSib;
> -            pWin->nextSib = pNextSib;
> -            pWin->prevSib = pNextSib->prevSib;
> -            if (pNextSib->prevSib)
> -                pNextSib->prevSib->nextSib = pWin;
> -            pNextSib->prevSib = pWin;
> -            if (!pFirstChange) {        /* do we know it yet? */
> -                pFirstChange = pParent->firstChild;     /* no, search
> from top */
> -                while ((pFirstChange != pWin) && (pFirstChange !=
> pOldNext))
> -                    pFirstChange = pFirstChange->nextSib;
> -            }
> -        }
> -        if (pWin->drawable.pScreen->RestackWindow)
> -            (*pWin->drawable.pScreen->RestackWindow) (pWin, pOldNextSib);
> -    }
> -
> -#ifdef ROOTLESS
>      /*
>       * In rootless mode we can't optimize away window restacks.
>       * There may be non-X windows around, so even if the window
>       * is in the correct position from X's point of view,
>       * the underlying window system may want to reorder it.
>       */
> -    else if (pWin->drawable.pScreen->RestackWindow)
> +    if (pWin->drawable.pScreen->RestackWindow)
>          (*pWin->drawable.pScreen->RestackWindow) (pWin, pWin->nextSib);
> -#endif
> -
> -    return pFirstChange;
>  }
>
>  void
> @@ -2039,7 +1971,7 @@ ReflectStackChange(WindowPtr pWin, WindowPtr pSib,
> VTKind kind)
>
>      Bool WasViewable = (Bool) pWin->viewable;
>      Bool anyMarked;
> -    WindowPtr pFirstChange;
> +    WindowPtr pFirstChange = pWin;
>      WindowPtr pLayerWin;
>      ScreenPtr pScreen = pWin->drawable.pScreen;
>
> @@ -2047,7 +1979,7 @@ ReflectStackChange(WindowPtr pWin, WindowPtr pSib,
> VTKind kind)
>      if (!pWin->parent)
>          return;
>
> -    pFirstChange = MoveWindowInStack(pWin, pSib);
> +    MoveWindowInStack(pWin, pSib);
>
>      if (WasViewable) {
>          anyMarked = (*pScreen->MarkOverlappedWindows) (pWin, pFirstChange,
> @@ -2236,11 +2168,7 @@ ConfigureWindow(WindowPtr pWin, Mask mask, XID
> *vlist, ClientPtr client)
>      if ((mask & CWBorderWidth) && (bw != wBorderWidth(pWin)))
>          goto ActuallyDoSomething;
>      if (mask & CWStackMode) {
> -#ifndef ROOTLESS
> -        /* See above for why we always reorder in rootless mode. */
> -        if (pWin->nextSib != pSib)
> -#endif
> -            goto ActuallyDoSomething;
> +        goto ActuallyDoSomething;
>      }
>      return Success;
>
> diff --git a/include/window.h b/include/window.h
> index b6d61c3..df06b75 100644
> --- a/include/window.h
> +++ b/include/window.h
> @@ -201,8 +201,8 @@ extern _X_EXPORT void
> CheckWindowOptionalNeed(WindowPtr /*w */ );
>
>  extern _X_EXPORT Bool MakeWindowOptional(WindowPtr /*pWin */ );
>
> -extern _X_EXPORT WindowPtr MoveWindowInStack(WindowPtr /*pWin */ ,
> -                                             WindowPtr /*pNextSib */ );
> +extern _X_EXPORT void MoveWindowInStack(WindowPtr /*pWin */ ,
> +                                        WindowPtr /*pNextSib */ );
>
>  extern _X_EXPORT void SetWinSize(WindowPtr /*pWin */ );
>
> diff --git a/mi/mioverlay.c b/mi/mioverlay.c
> index 2bfd5e4..b014d3d 100644
> --- a/mi/mioverlay.c
> +++ b/mi/mioverlay.c
> @@ -911,7 +911,7 @@ miOverlayMoveWindow(WindowPtr pWin,
>  {
>      ScreenPtr pScreen = pWin->drawable.pScreen;
>      miOverlayTreePtr pTree = MIOVERLAY_GET_WINDOW_TREE(pWin);
> -    WindowPtr pParent, windowToValidate;
> +    WindowPtr pParent, windowToValidate = pWin;
>      Bool WasViewable = (Bool) (pWin->viewable);
>      short bw;
>      RegionRec overReg, underReg;
> @@ -946,7 +946,7 @@ miOverlayMoveWindow(WindowPtr pWin,
>
>      (*pScreen->PositionWindow) (pWin, x, y);
>
> -    windowToValidate = MoveWindowInStack(pWin, pNextSib);
> +    MoveWindowInStack(pWin, pNextSib);
>
>      ResizeChildrenWinSize(pWin, x - oldpt.x, y - oldpt.y, 0, 0);
>
> @@ -1101,7 +1101,7 @@ miOverlayResizeWindow(WindowPtr pWin,
>      short dw, dh;
>      DDXPointRec oldpt;
>      RegionPtr oldRegion = NULL, oldRegion2 = NULL;
> -    WindowPtr pFirstChange;
> +    WindowPtr pFirstChange = pWin;
>      WindowPtr pChild;
>      RegionPtr gravitate[StaticGravity + 1];
>      RegionPtr gravitate2[StaticGravity + 1];
> @@ -1225,7 +1225,7 @@ miOverlayResizeWindow(WindowPtr pWin,
>      /* let the hardware adjust background and border pixmaps, if any */
>      (*pScreen->PositionWindow) (pWin, x, y);
>
> -    pFirstChange = MoveWindowInStack(pWin, pSib);
> +    MoveWindowInStack(pWin, pSib);
>
>      if (WasViewable) {
>          pRegion = RegionCreate(NullBox, 1);
> diff --git a/mi/miwindow.c b/mi/miwindow.c
> index 8dd99db..8adafdc 100644
> --- a/mi/miwindow.c
> +++ b/mi/miwindow.c
> @@ -247,7 +247,7 @@ miMoveWindow(WindowPtr pWin, int x, int y, WindowPtr
> pNextSib, VTKind kind)
>      DDXPointRec oldpt;
>      Bool anyMarked = FALSE;
>      ScreenPtr pScreen;
> -    WindowPtr windowToValidate;
> +    WindowPtr windowToValidate = pWin;
>      WindowPtr pLayerWin;
>
>      /* if this is a root window, can't be moved */
> @@ -273,7 +273,7 @@ miMoveWindow(WindowPtr pWin, int x, int y, WindowPtr
> pNextSib, VTKind kind)
>
>      (*pScreen->PositionWindow) (pWin, x, y);
>
> -    windowToValidate = MoveWindowInStack(pWin, pNextSib);
> +    MoveWindowInStack(pWin, pNextSib);
>
>      ResizeChildrenWinSize(pWin, x - oldpt.x, y - oldpt.y, 0, 0);
>
> @@ -350,7 +350,7 @@ miSlideAndSizeWindow(WindowPtr pWin,
>      RegionPtr oldRegion = NULL;
>      Bool anyMarked = FALSE;
>      ScreenPtr pScreen;
> -    WindowPtr pFirstChange;
> +    WindowPtr pFirstChange = pWin;
>      WindowPtr pChild;
>      RegionPtr gravitate[StaticGravity + 1];
>      unsigned g;
> @@ -444,7 +444,7 @@ miSlideAndSizeWindow(WindowPtr pWin,
>      /* let the hardware adjust background and border pixmaps, if any */
>      (*pScreen->PositionWindow) (pWin, x, y);
>
> -    pFirstChange = MoveWindowInStack(pWin, pSib);
> +    MoveWindowInStack(pWin, pSib);
>
>      if (WasViewable) {
>          pRegion = RegionCreate(NullBox, 1);
> --
> 1.8.3.1
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>



-- 
  Jasper
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130820/c6d9ff2b/attachment-0001.html>


More information about the xorg-devel mailing list