[PATCH v2 4/4] composite: Copy the window contents back from the pixmap

[Ville Syrjälä]

On Mon, Dec 27, 2010 at 10:00:30AM -0800, ext Keith Packard wrote:
> On Mon, 27 Dec 2010 14:40:14 +0000, Daniel Stone <daniel at fooishbar.org> wrote:
> > Hi,
> > 
> > On Mon, Dec 27, 2010 at 03:24:13PM +0200, ville.syrjala at nokia.com wrote:
> > > +void
> > > +compRestoreWindow (WindowPtr pWin, PixmapPtr pPixmap)
> > > +{
> > > +    ScreenPtr pScreen = pWin->drawable.pScreen;
> > > +    WindowPtr pParent = pWin->parent;
> > > +
> > > +    if (pParent->drawable.depth == pWin->drawable.depth) {
> > > +	GCPtr pGC = GetScratchGC (pWin->drawable.depth, pScreen);
> > > +	int bw = (int) pWin->borderWidth;
> > > +	int x = bw;
> > > +	int y = bw;
> > > +	int w = pWin->drawable.width;
> > > +	int h = pWin->drawable.height;
> > > +
> > > +	if (pGC) {
> > > +	    ChangeGCVal val;
> > > +	    val.val = IncludeInferiors;
> > > +	    ChangeGC (NullClient, pGC, GCSubwindowMode, &val);
> > > +	    ValidateGC(&pWin->drawable, pGC);
> > > +	    (*pGC->ops->CopyArea) (&pPixmap->drawable,
> > > +				   &pWin->drawable,
> > > +				   pGC,
> > > +				   x, y, w, h, 0, 0);
> > > +	    FreeScratchGC (pGC);
> > > +	}
> > > +    }
> > > +}
> > 
> > It might be nice to get the Render fallback path for non-matching depths
> > here; aside from that, for the entire series:
> 
> What's confusing here is that there is a test for matching depth, but
> no 'else' branch to cover the other case. I don't think this test is
> needed as the pixmap was the previous window pixmap, and depths for that
> must match the window depth. If that wasn't the case, then all
> drawing to windows would fail, as Pictures and GCs are validated against
> the depth of the window, not the pixmap.
> 
> Seems like simply removing the test would be fine; it's not like you're
> requiring some new invariant in the code here.

We're going through this codepath when getting rid of an implicitly
redirected window. That is at least one case where the depths won't
match.

Breakpoint 1, compRestoreWindow (pWin=0x3dded0, pPixmap=0x3ddde8) at ../../composite/compalloc.c:212
212	in ../../composite/compalloc.c
(gdb) bt
#0  compRestoreWindow (pWin=0x3dded0, pPixmap=0x3ddde8) at ../../composite/compalloc.c:212
#1  0x002160a0 in compCheckRedirect (pWin=0x3dded0) at ../../composite/compwindow.c:167
#2  0x00216520 in compUnrealizeWindow (pWin=0x3dded0) at ../../composite/compwindow.c:265
#3  0x00027e00 in UnrealizeTree (pWin=0x3dded0, fromConfigure=0) at ../../dix/window.c:2783
#4  0x00028128 in UnmapWindow (pWin=0x3dded0, fromConfigure=0) at ../../dix/window.c:2841
#5  0x00022f88 in DeleteWindow (value=0x3dded0, wid=4194306) at ../../dix/window.c:905
#6  0x00030030 in FreeClientResources (client=0x3eac88) at ../../dix/resource.c:859
#7  0x0007c700 in CloseDownClient (client=0x3eac88) at ../../dix/dispatch.c:3490
#8  0x00073494 in Dispatch () at ../../dix/dispatch.c:417
#9  0x000207ac in main (argc=16, argv=0xaec4cdd4, envp=0xaec4ce18) at ../../dix/main.c:291
(gdb) p pWin->drawable.depth
$5 = 32 ' '
(gdb) p pWin->parent->drawable.depth
$6 = 16 '\020'
(gdb) 

-- 
Ville Syrjälä