xserver: Branch 'master' - 5 commits

Keith Packard keithp at kemper.freedesktop.org
Thu Jul 17 11:21:01 PDT 2014


 hw/xfree86/common/xf86VGAarbiterPriv.h |   10 ++-
 hw/xfree86/modes/xf86Rotate.c          |   18 +++++-
 include/scrnintstr.h                   |   90 +++++++++++++++++++++++++++++++++
 mi/misprite.c                          |    4 -
 4 files changed, 113 insertions(+), 9 deletions(-)

New commits:
commit 3319e7041ff89bb01b16a1dbbac85e28b1976ae3
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Apr 18 14:24:29 2014 -0700

    hw/xfree86: Let xf86Rotate leave the BlockHandler unwrapped when possible
    
    When no shadow frame buffer is needed, the rotate block handler
    doesn't need to be called any more. Remove it from the chain.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>

diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c
index 99dcb43..1627e61 100644
--- a/hw/xfree86/modes/xf86Rotate.c
+++ b/hw/xfree86/modes/xf86Rotate.c
@@ -243,9 +243,13 @@ xf86RotateBlockHandler(ScreenPtr pScreen,
     xf86RotateRedisplay(pScreen);
 
     (*pScreen->BlockHandler) (pScreen, pTimeout, pReadmask);
-    /* cannot avoid re-wrapping until all wrapping is audited */
-    xf86_config->BlockHandler = pScreen->BlockHandler;
-    pScreen->BlockHandler = xf86RotateBlockHandler;
+
+    /* Re-wrap if we still need this hook */
+    if (xf86_config->rotation_damage != NULL) {
+        xf86_config->BlockHandler = pScreen->BlockHandler;
+        pScreen->BlockHandler = xf86RotateBlockHandler;
+    } else
+        xf86_config->BlockHandler = NULL;
 }
 
 void
commit 79a2733005202af43821d8fd8e4c9fb77bf8f69e
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Apr 18 14:11:17 2014 -0700

    hw/xfree86: Fix VGA arbiter screen proc wrapping
    
    Change the screen proc epilog code to re-fetch the current screen
    function in case a nested proc changes how things work. This isn't a
    problem with the current code as all of the wrapping layers that are
    set up at server init time (like the VGA arbiter) leave themselves in
    the screen proc chain forever. But, this makes the code conform with
    the expected norms.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>

diff --git a/hw/xfree86/common/xf86VGAarbiterPriv.h b/hw/xfree86/common/xf86VGAarbiterPriv.h
index ec21bc2..b832c9a 100644
--- a/hw/xfree86/common/xf86VGAarbiterPriv.h
+++ b/hw/xfree86/common/xf86VGAarbiterPriv.h
@@ -49,10 +49,14 @@
 
 #define UNWRAP_SCREEN(x) pScreen->x = pScreenPriv->x
 
-#define SCREEN_PROLOG(x) pScreen->x = ((VGAarbiterScreenPtr) \
-    dixLookupPrivate(&(pScreen)->devPrivates, VGAarbiterScreenKey))->x
+#define SCREEN_PRIV()   ((VGAarbiterScreenPtr) dixLookupPrivate(&(pScreen)->devPrivates, VGAarbiterScreenKey))
 
-#define SCREEN_EPILOG(x,y) pScreen->x = y;
+#define SCREEN_PROLOG(x) (pScreen->x = SCREEN_PRIV()->x)
+
+#define SCREEN_EPILOG(x,y) do {                 \
+        SCREEN_PRIV()->x = pScreen->x;          \
+        pScreen->x = y;                         \
+    } while (0)
 
 #define WRAP_PICT(x,y) if (ps) {pScreenPriv->x = ps->x;\
     ps->x = y;}
commit a1189fe322724ab1b524aaad5b700287777252bd
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Apr 18 13:57:55 2014 -0700

    mi: Fix block handler wrapping in miSprite
    
    miSpriteBlockHandler was leaving the BlockHandler wrapped until just
    before calling any nested block handler. If any code executed before
    that added or removed block handlers, the wrapping chain would have
    been broken.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>

diff --git a/mi/misprite.c b/mi/misprite.c
index eea731a..68a49be 100644
--- a/mi/misprite.c
+++ b/mi/misprite.c
@@ -520,6 +520,8 @@ miSpriteBlockHandler(ScreenPtr pScreen, void *pTimeout,
     miCursorInfoPtr pCursorInfo;
     Bool WorkToDo = FALSE;
 
+    SCREEN_PROLOGUE(pPriv, pScreen, BlockHandler);
+
     for (pDev = inputInfo.devices; pDev; pDev = pDev->next) {
         if (DevHasCursor(pDev)) {
             pCursorInfo = MISPRITE(pDev);
@@ -543,8 +545,6 @@ miSpriteBlockHandler(ScreenPtr pScreen, void *pTimeout,
         }
     }
 
-    SCREEN_PROLOGUE(pPriv, pScreen, BlockHandler);
-
     (*pScreen->BlockHandler) (pScreen, pTimeout, pReadmask);
 
     if (WorkToDo)
commit 08fc33042c858568e7244eb9ad25a8d0270754f0
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Apr 18 13:55:50 2014 -0700

    hw/xfree86: Fix block handler wrapping in xf86Rotate
    
    xf86Rotate, it was delaying unwrapping the BlockHandler until after
    calling xf86RotateRedisplay. If there was a software cursor on the
    screen, the redisplay operation would cause cursor to be removed from
    the frame buffer and the misprite block handler to be inserted into
    the block handler chain with the misprite screen private saved block
    handler now set to xf86RotateBlockHandler.
    
    When xf86RotateRedisplay returned, xf86RotateBlockHandler would then
    set screen->BlockHandler to its saved value, call down and then reset
    screen->BlockHandler to xf86RotateBlockHandler. miSpriteBlockHandler
    would never be called after that, which meant that the software cursor
    will now disappear from the screen whenever rendering overlapped and
    would only reappear when the cursor was moved.
    
    To correct this, all that is needed is to move the restoration of
    screen->BlockHandler to the top of xf86RotateBlockHandler, before the
    call to xf86RotateRedisplay.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>

diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c
index 0ddd840..99dcb43 100644
--- a/hw/xfree86/modes/xf86Rotate.c
+++ b/hw/xfree86/modes/xf86Rotate.c
@@ -234,8 +234,14 @@ xf86RotateBlockHandler(ScreenPtr pScreen,
     ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
     xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
 
-    xf86RotateRedisplay(pScreen);
+    /* Unwrap before redisplay in case the software
+     * cursor layer wants to add its block handler to the
+     * chain
+     */
     pScreen->BlockHandler = xf86_config->BlockHandler;
+
+    xf86RotateRedisplay(pScreen);
+
     (*pScreen->BlockHandler) (pScreen, pTimeout, pReadmask);
     /* cannot avoid re-wrapping until all wrapping is audited */
     xf86_config->BlockHandler = pScreen->BlockHandler;
commit c75fee79ace394f6f51aa6fdda1c0436eb8a2026
Author: Keith Packard <keithp at keithp.com>
Date:   Fri Apr 18 13:54:11 2014 -0700

    Document how to correctly wrap screen procedures
    
    This adds a large comment to include/scrnintstr.h which should serve
    to document the correct way to wrap any screen procedure, with a
    particular focus on how to dynamically add/remove wrapping layers
    while the server is running.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Eric Anholt <eric at anholt.net>

diff --git a/include/scrnintstr.h b/include/scrnintstr.h
index 5312781..6955e77 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -358,6 +358,96 @@ typedef WindowPtr (*XYToWindowProcPtr)(ScreenPtr pScreen,
 
 typedef int (*NameWindowPixmapProcPtr)(WindowPtr, PixmapPtr, CARD32);
 
+/* Wrapping Screen procedures
+
+   There are a few modules in the X server which dynamically add and
+    remove themselves from various screen procedure call chains.
+
+    For example, the BlockHandler is dynamically modified by:
+
+     * xf86Rotate
+     * miSprite
+     * composite
+     * render (for animated cursors)
+
+    Correctly manipulating this chain is complicated by the fact that
+    the chain is constructed through a sequence of screen private
+    structures, each holding the next screen->proc pointer.
+
+    To add a module to a screen->proc chain is fairly simple; just save
+    the current screen->proc value in the module screen private
+    and store the module's function in the screen->proc location.
+
+    Removing a screen proc is a bit trickier. It seems like all you
+    need to do is set the screen->proc pointer back to the value saved
+    in your screen private. However, if some other module has come
+    along and wrapped on top of you, then the right place to store the
+    previous screen->proc value is actually in the wrapping module's
+    screen private structure(!). Of course, you have no idea what
+    other module may have wrapped on top, nor could you poke inside
+    its screen private in any case.
+
+    To make this work, we restrict the unwrapping process to happen
+    during the invocation of the screen proc itself, and then we
+    require the screen proc to take some care when manipulating the
+    screen proc functions pointers.
+
+    The requirements are:
+
+     1) The screen proc must set the screen->proc pointer back to the
+        value saved in its screen private before calling outside its
+        module.
+
+     2a) If the screen proc wants to be remove itself from the chain,
+         it must not manipulate screen->proc pointer again before
+         returning.
+
+     2b) If the screen proc wants to remain in the chain, it must:
+
+       2b.1) Re-fetch the screen->proc pointer and store that in
+             its screen private. This ensures that any changes
+             to the chain will be preserved.
+
+       2b.2) Set screen->proc back to itself
+
+    One key requirement here is that these steps must wrap not just
+    any invocation of the nested screen->proc value, but must nest
+    essentially any calls outside the current module. This ensures
+    that other modules can reliably manipulate screen->proc wrapping
+    using these same rules.
+
+    For example, the animated cursor code in render has two macros,
+    Wrap and Unwrap.
+
+        #define Unwrap(as,s,elt)    ((s)->elt = (as)->elt)
+
+    Unwrap takes the screen private (as), the screen (s) and the
+    member name (elt), and restores screen->proc to that saved in the
+    screen private.
+
+        #define Wrap(as,s,elt,func) (((as)->elt = (s)->elt), (s)->elt = func)
+
+    Wrap takes the screen private (as), the screen (s), the member
+    name (elt) and the wrapping function (func). It saves the
+    current screen->proc value in the screen private, and then sets the
+    screen->proc to the local wrapping function.
+
+    Within each of these functions, there's a pretty simple pattern:
+
+        Unwrap(as, pScreen, UnrealizeCursor);
+
+        // Do local stuff, including possibly calling down through
+        // pScreen->UnrealizeCursor
+
+        Wrap(as, pScreen, UnrealizeCursor, AnimCurUnrealizeCursor);
+
+    The wrapping block handler is a bit different; it does the Unwrap,
+    the local operations and then only re-Wraps if the hook is still
+    required. Unwrap occurrs at the top of each function, just after
+    entry, and Wrap occurrs at the bottom of each function, just
+    before returning.
+ */
+
 typedef struct _Screen {
     int myNum;                  /* index of this instance in Screens[] */
     ATOM id;


More information about the xorg-commit mailing list