Submitted By: Ken Moffat Date: 2014-08-18 Initial Package Version: 1.16.0 Upstream Status: Applied Origin: Upstream Description: Fixes bug in glamor. From 3c0431b8911241552a15a43e4279c50658b50a18 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 16 Jul 2014 16:03:23 -0700 Subject: glamor: Fix temp picture coordinates in glamor_composite_clipped_region To understand this patch, let's start at the protocol interface where the relationship between the coordinate spaces is documented: static Bool _glamor_composite(CARD8 op, PicturePtr source, PicturePtr mask, PicturePtr dest, INT16 x_source, INT16 y_source, INT16 x_mask, INT16 y_mask, INT16 x_dest, INT16 y_dest, CARD16 width, CARD16 height, Bool fallback) The coordinates are passed to this function directly off the wire and are all relative to their respective drawables. For Windows, this means that they are relative to the upper left corner of the window, in whatever pixmap that window is getting drawn to. _glamor_composite calls miComputeCompositeRegion to construct a clipped region to actually render to. In reality, miComputeCompositeRegion clips only to the destination these days; source clip region based clipping would have to respect the transform, which isn't really possible. The returned region is relative to the screen in which dest lives; offset by dest->drawable.x and dest->drawable.y. What is important to realize here is that, because of clipping, the composite region may not have the same position within the destination drawable as x_dest, y_dest. The protocol coordinates now exist solely to 'pin' the three objects together. extents->x1,y1 Screen origin of clipped operation width,height Extents of the clipped operation x_dest,y_dest Unclipped destination-relative operation coordinate x_source,y_source Unclipped source-relative operation coordinate x_mask,y_mask Unclipped mask-relative operation coordinate One thing we want to know is what the offset is from the original operation origin to the clipped origin Destination drawable relative coordinates of the clipped operation: x_dest_clipped = extents->x1 - dest->drawable.x y_dest_clipped = extents->y1 - dest->drawable.y Offset from the original operation origin: x_off_clipped = x_dest_clipped - x_dest y_off_clipped = y_dest_clipped - y_dest Source drawable relative coordinates of the clipped operation: x_source_clipped = x_source + x_off_clipped; y_source_clipped = y_source + y_off_clipped; Mask drawable relative coordinates of the clipped operation: x_mask_clipped = x_source + x_off_clipped; y_mask_clipped = y_source + y_off_clipped; This is where the original code fails -- it doesn't subtract the destination drawable location when computing the distance that the operation has been moved by clipping. Here's what it does when constructing a temporary source picture: temp_src = glamor_convert_gradient_picture(screen, source, extent->x1 + x_source - x_dest, extent->y1 + y_source - y_dest, width, height); ... x_temp_src = -extent->x1 + x_dest; y_temp_src = -extent->y1 + y_dest; glamor_convert_gradient_picture needs source drawable relative coordinates, but that is not what it's getting; it's getting screen-relative coordinates for the destination, adjusted by the distance between the provided source and destination operation coordinates. We want x_source_clipped and y_source_clipped: x_source_clipped = x_source + x_off_clipped = x_source + x_dest_clipped - x_dest = x_source + extents->x1 - dest->drawable.x - x_dest x_temp_src/y_temp_src are supposed to be the coordinates of the original operation translated to the temporary picture: x_temp_src = x_source - x_source_clipped; y_temp_src = y_source - y_source_clipped; Note that x_source_clipped/y_source_clipped will never be less than x_source/y_source because all we're doing is clipping. This means that x_temp_src/y_temp_src will always be non-positive; the original source coordinate can never be strictly *inside* the temporary image or we could have made the temporary image smaller. x_temp_src = x_source - x_source_clipped = x_source - (x_source + x_off_clipped) = -x_off_clipped = x_dest - x_dest_clipped = x_dest - (extents->x1 - dest->drawable.x) Again, this is off by the destination origin within the screen coordinate space. The code should look like: temp_src = glamor_convert_gradient_picture(screen, source, extent->x1 + x_source - x_dest - dest->pDrawable->x, extent->y1 + y_source - y_dest - dest->pDrawable->y, width, height); x_temp_src = -extent->x1 + x_dest + dest->pDrawable->x; y_temp_src = -extent->y1 + y_dest + dest->pDrawable->y; Signed-off-by: Keith Packard Reviewed-by: Markus Wick (cherry picked from commit 55f5bfb578e934319d1308cbb56c900c5ac7cfa7) Signed-off-by: Julien Cristau diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 14ab738..e5d5d2c 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -1450,8 +1450,8 @@ glamor_composite_clipped_region(CARD8 op, || source_pixmap->drawable.height != height)))) { temp_src = glamor_convert_gradient_picture(screen, source, - extent->x1 + x_source - x_dest, - extent->y1 + y_source - y_dest, + extent->x1 + x_source - x_dest - dest->pDrawable->x, + extent->y1 + y_source - y_dest - dest->pDrawable->y, width, height); if (!temp_src) { temp_src = source; @@ -1459,8 +1459,8 @@ glamor_composite_clipped_region(CARD8 op, } temp_src_priv = glamor_get_pixmap_private((PixmapPtr) (temp_src->pDrawable)); - x_temp_src = -extent->x1 + x_dest; - y_temp_src = -extent->y1 + y_dest; + x_temp_src = -extent->x1 + x_dest + dest->pDrawable->x; + y_temp_src = -extent->y1 + y_dest + dest->pDrawable->y; } if (mask @@ -1474,8 +1474,8 @@ glamor_composite_clipped_region(CARD8 op, * to do reduce one convertion. */ temp_mask = glamor_convert_gradient_picture(screen, mask, - extent->x1 + x_mask - x_dest, - extent->y1 + y_mask - y_dest, + extent->x1 + x_mask - x_dest - dest->pDrawable->x, + extent->y1 + y_mask - y_dest - dest->pDrawable->y, width, height); if (!temp_mask) { temp_mask = mask; @@ -1483,8 +1483,8 @@ glamor_composite_clipped_region(CARD8 op, } temp_mask_priv = glamor_get_pixmap_private((PixmapPtr) (temp_mask->pDrawable)); - x_temp_mask = -extent->x1 + x_dest; - y_temp_mask = -extent->y1 + y_dest; + x_temp_mask = -extent->x1 + x_dest + dest->pDrawable->x; + y_temp_mask = -extent->y1 + y_dest + dest->pDrawable->y; } /* Do two-pass PictOpOver componentAlpha, until we enable * dual source color blending. -- cgit v0.10.2