[PATCH] composite: Don't bother copying the pixmap for ForgetGravity windows (v2)

[Jasper St. Pierre]

The compositor sync protocol helps that tremendously, where the client
actually tells the compositor when it can start to use the new pixmap.
I tested with xcompmgr and nautilus, and couldn't see any flickering,
which tells me that clients are actually plenty fast enough already.

But yeah, you might see flickering in a worst case scenario. I'd argue
that isn't much better than the current NorthWestGravity behavior
where we lop off the bottom/right edge of the window. There's no true
way to solve this properly besides a Wayland-approach where the client
just presents the compositor a correctly-sized, finished pixmap.

On Thu, Apr 23, 2015 at 2:13 PM, Aaron Plattner <aplattner at nvidia.com> wrote:
> Does this cause flickering when resizing windows and the compositor reads
> the new window pixmap before the app has a chance to respond to the events?
>
>
> On 04/23/2015 01:24 PM, Jasper St. Pierre wrote:
>>
>> If a window has ForgetGravity in its bitGravity, that very likely
>> means it will repaint on the ConfigureNotify / Expose event, and thus
>> we don't have to copy the old pixmap into the new pixmap, we can simply
>> leave it blank.
>>
>> Preventing this copy is super simple to do and a big win on small
>> devices where these blits can be expensive.
>>
>> A better approach would be to actually obey BitGravity correctly rather
>> than assume NorthWestGravity always, but this is a big speedup for the
>> common case.
>>
>> v2: Check all subwindows to make sure they are also ForgetGravity
>>
>> Cc: Adam Jackson <ajax at redhat.com>
>> Signed-off-by: Jasper St. Pierre <jstpierre at mecheye.net>
>> ---
>>   composite/compalloc.c | 109
>> +++++++++++++++++++++++++++++---------------------
>>   1 file changed, 63 insertions(+), 46 deletions(-)
>>
>> diff --git a/composite/compalloc.c b/composite/compalloc.c
>> index 8daded0..40bf873 100644
>> --- a/composite/compalloc.c
>> +++ b/composite/compalloc.c
>> @@ -526,6 +526,21 @@ compUnredirectOneSubwindow(WindowPtr pParent,
>> WindowPtr pWin)
>>       return Success;
>>   }
>>
>> +static Bool
>> +needsPixmapCopy(WindowPtr pWin)
>> +{
>> +    WindowPtr pChild;
>> +
>> +    if (pWin->bitGravity != ForgetGravity)
>> +        return TRUE;
>> +
>> +    for (pChild = pWin->firstChild; pChild; pChild = pChild->nextSib)
>> +        if (needsPixmapCopy(pChild))
>> +            return TRUE;
>> +
>> +    return FALSE;
>> +}
>> +
>>   static PixmapPtr
>>   compNewPixmap(WindowPtr pWin, int x, int y, int w, int h)
>>   {
>> @@ -542,54 +557,56 @@ compNewPixmap(WindowPtr pWin, int x, int y, int w,
>> int h)
>>       pPixmap->screen_x = x;
>>       pPixmap->screen_y = y;
>>
>> -    if (pParent->drawable.depth == pWin->drawable.depth) {
>> -        GCPtr pGC = GetScratchGC(pWin->drawable.depth, pScreen);
>> -
>> -        if (pGC) {
>> -            ChangeGCVal val;
>> -
>> -            val.val = IncludeInferiors;
>> -            ChangeGC(NullClient, pGC, GCSubwindowMode, &val);
>> -            ValidateGC(&pPixmap->drawable, pGC);
>> -            (*pGC->ops->CopyArea) (&pParent->drawable,
>> -                                   &pPixmap->drawable,
>> -                                   pGC,
>> -                                   x - pParent->drawable.x,
>> -                                   y - pParent->drawable.y, w, h, 0, 0);
>> -            FreeScratchGC(pGC);
>> +    if (needsPixmapCopy(pWin)) {
>> +        if (pParent->drawable.depth == pWin->drawable.depth) {
>> +            GCPtr pGC = GetScratchGC(pWin->drawable.depth, pScreen);
>> +
>> +            if (pGC) {
>> +                ChangeGCVal val;
>> +
>> +                val.val = IncludeInferiors;
>> +                ChangeGC(NullClient, pGC, GCSubwindowMode, &val);
>> +                ValidateGC(&pPixmap->drawable, pGC);
>> +                (*pGC->ops->CopyArea) (&pParent->drawable,
>> +                                       &pPixmap->drawable,
>> +                                       pGC,
>> +                                       x - pParent->drawable.x,
>> +                                       y - pParent->drawable.y, w, h, 0,
>> 0);
>> +                FreeScratchGC(pGC);
>> +            }
>>           }
>> -    }
>> -    else {
>> -        PictFormatPtr pSrcFormat = PictureWindowFormat(pParent);
>> -        PictFormatPtr pDstFormat = PictureWindowFormat(pWin);
>> -        XID inferiors = IncludeInferiors;
>> -        int error;
>> -
>> -        PicturePtr pSrcPicture = CreatePicture(None,
>> -                                               &pParent->drawable,
>> -                                               pSrcFormat,
>> -                                               CPSubwindowMode,
>> -                                               &inferiors,
>> -                                               serverClient, &error);
>> -
>> -        PicturePtr pDstPicture = CreatePicture(None,
>> -                                               &pPixmap->drawable,
>> -                                               pDstFormat,
>> -                                               0, 0,
>> -                                               serverClient, &error);
>> -
>> -        if (pSrcPicture && pDstPicture) {
>> -            CompositePicture(PictOpSrc,
>> -                             pSrcPicture,
>> -                             NULL,
>> -                             pDstPicture,
>> -                             x - pParent->drawable.x,
>> -                             y - pParent->drawable.y, 0, 0, 0, 0, w, h);
>> +        else {
>> +            PictFormatPtr pSrcFormat = PictureWindowFormat(pParent);
>> +            PictFormatPtr pDstFormat = PictureWindowFormat(pWin);
>> +            XID inferiors = IncludeInferiors;
>> +            int error;
>> +
>> +            PicturePtr pSrcPicture = CreatePicture(None,
>> +                                                   &pParent->drawable,
>> +                                                   pSrcFormat,
>> +                                                   CPSubwindowMode,
>> +                                                   &inferiors,
>> +                                                   serverClient, &error);
>> +
>> +            PicturePtr pDstPicture = CreatePicture(None,
>> +                                                   &pPixmap->drawable,
>> +                                                   pDstFormat,
>> +                                                   0, 0,
>> +                                                   serverClient, &error);
>> +
>> +            if (pSrcPicture && pDstPicture) {
>> +                CompositePicture(PictOpSrc,
>> +                                 pSrcPicture,
>> +                                 NULL,
>> +                                 pDstPicture,
>> +                                 x - pParent->drawable.x,
>> +                                 y - pParent->drawable.y, 0, 0, 0, 0, w,
>> h);
>> +            }
>> +            if (pSrcPicture)
>> +                FreePicture(pSrcPicture, 0);
>> +            if (pDstPicture)
>> +                FreePicture(pDstPicture, 0);
>>           }
>> -        if (pSrcPicture)
>> -            FreePicture(pSrcPicture, 0);
>> -        if (pDstPicture)
>> -            FreePicture(pDstPicture, 0);
>>       }
>>       return pPixmap;
>>   }
>
>
> --
> Aaron



-- 
  Jasper