• Keith Packard's avatar
    glamor: Fix temp picture coordinates in glamor_composite_clipped_region · 55f5bfb5
    Keith Packard authored
    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's avatarKeith Packard <keithp@keithp.com>
    Reviewed-by: default avatarMarkus Wick <markus@selfnet.de>
glamor_render.c 70.4 KB