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

[Keith Packard]

Adam Jackson  writes:

> I think this still needs to address what I wrote in paragraph 2 here:
>
> http://lists.freedesktop.org/archives/xorg-devel/2015-May/046376.html

Yeah, I was just looking at the implementation and not the architecture...

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

[Adam Jackson]

On Wed, 2015-11-18 at 21:43 -0800, Keith Packard wrote:
> "Jasper St. Pierre"  writes:
> 
> > +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;
> > +}
> 
> I think you can use TraverseTree here; that has the advantage of not
> being recursive.

I think this still needs to address what I wrote in paragraph 2 here:

http://lists.freedesktop.org/archives/xorg-devel/2015-May/046376.html

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

[Jasper St. Pierre]

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
v3: Destroy old pixmap before creating the new one

Cc: Adam Jackson 
Signed-off-by: Jasper St. Pierre 
---
 composite/compalloc.c | 113 ++
 1 file changed, 67 insertions(+), 46 deletions(-)

diff --git a/composite/compalloc.c b/composite/compalloc.c
index 8daded0..06b54e9 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, );
-ValidateGC(>drawable, pGC);
-(*pGC->ops->CopyArea) (>drawable,
-   >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, );
+ValidateGC(>drawable, pGC);
+(*pGC->ops->CopyArea) (>drawable,
+   >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,
-   >drawable,
-   pSrcFormat,
-   CPSubwindowMode,
-   ,
-   serverClient, );
-
-PicturePtr pDstPicture = CreatePicture(None,
-   >drawable,
-   pDstFormat,
-   0, 0,
-   serverClient, );
-
-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,
+   >drawable,
+   pSrcFormat,
+   CPSubwindowMode,
+   ,
+   serverClient, );
+
+PicturePtr pDstPicture = CreatePicture(None,
+   >drawable,
+   pDstFormat,
+

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

[Keith Packard]

"Jasper St. Pierre"  writes:

> +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;
> +}

I think you can use TraverseTree here; that has the advantage of not
being recursive.

> +
>  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, );
> -ValidateGC(>drawable, pGC);
> -(*pGC->ops->CopyArea) (>drawable,
> -   >drawable,
> -   pGC,
> -   x - pParent->drawable.x,
> -   y - pParent->drawable.y, w, h, 0, 0);
> -FreeScratchGC(pGC);
> +if (needsPixmapCopy(pWin)) {

I think this part could be replaced with

if (!needsPixmapCopy(pWin))
return pPixmap;

That would, at least, be easier to review.

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

[Adam Jackson]

On Thu, 2015-04-23 at 13:24 -0700, 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.

I would prefer to see the can we optimize this check done in
compReallocPixmap, the way my initial patch did. That way we can
destroy the old pixmap before allocating the new one, which is a nice
reduction in memory pressure (in both address space and dirty
cacheline senses).

I think we also want to treat the combination of {auto redirection,
forget gravity, null background} as needs-copy. xcompmgr -a should
look as much like uncomposited X as possible. If we skipped the copy
in this case, we would also skip the background paint, but the window
would still be dirty from composite's perspective, so the next
screen update will paint whatever garbage is in the uninitialized new
pixmap. That wouldn't match classic X's the existing screen contents
are not altered semantics.

I don't think the flicker thing Aaron mentioned is an issue.  The copy
is a waste for Forget windows in any case, because it's immediately
followed by a window-sized background paint (assuming defined
background).  So auto is already flickery, and manual already has the
sync protocol.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

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

[Jasper St. Pierre]

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 a...@redhat.com
Signed-off-by: Jasper St. Pierre jstpie...@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,
+ 

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

[Aaron Plattner]

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 a...@redhat.com
Signed-off-by: Jasper St. Pierre jstpie...@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,
+   

Re: [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 aplatt...@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 a...@redhat.com
 Signed-off-by: Jasper St. Pierre jstpie...@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,
 -   

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

[Jasper St. Pierre]

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.
---
 composite/compalloc.c | 94 ++-
 1 file changed, 48 insertions(+), 46 deletions(-)

diff --git a/composite/compalloc.c b/composite/compalloc.c
index 8daded0..70c83f0 100644
--- a/composite/compalloc.c
+++ b/composite/compalloc.c
@@ -542,54 +542,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 (pWin-bitGravity != ForgetGravity) {
+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)
+