[PATCH] hw/xfree86: Fix block handler wrapping in xf86Rotate

Keith Packard keithp at keithp.com
Tue Apr 8 11:52:08 PDT 2014

There are a few modules in the X server which dynamically add and
remove themselves from the screen block handler chain:

 * 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 block handler pointer.

To add a module to the block handler chain is fairly simple; just save
the current screen->BlockHandler pointer in the module screen private
and store the module's BlockHandler function in screen->BlockHandler.

Removing a block handler is a bit trickier. It seems like what you
want to do is set screen->BlockHandler 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
block handler 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

To make this work, we restrict the unwrapping process to the
BlockHandler itself, and then we require the BlockHandler to take some
care when manipulating the block handler functions pointers.

The requirements are:

 1) The block handler must set the screen->BlockHandler pointer back
    to the value saved in its screen private before calling outside
    its module.

 2a) If the block handler wants to be remove it self from the chain,
     it must not manipulate screen->BlockHandler before returning.

 2b) If the block handler wants to remain in the chain, it must:

   2b.1) Re-fetch the screen->BlockHandler pointer and store that in
         its screen private.

   2b.2) Set screen->BlockHandler back to itself

In the case of xf86Rotate, it was delaying 1) 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.

I've also added to this patch a check to see if the rotate block
handler is still required and only re-insert itself if that is the

Signed-off-by: Keith Packard <keithp at keithp.com>
 hw/xfree86/modes/xf86Rotate.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c
index 0ddd840..1627e61 100644
--- a/hw/xfree86/modes/xf86Rotate.c
+++ b/hw/xfree86/modes/xf86Rotate.c
@@ -234,12 +234,22 @@ 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;
-    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;

More information about the xorg-devel mailing list