[PATCH 02/13] glamor: Add glamor_program based copy acceleration

[Eric Anholt]

Looks like my reply last night didn't go through...

Keith Packard <keithp at keithp.com> writes:
> Paints with textures, using a temporary buffer for overlapping copies
>
> Performs CPU to GPU transfers for pixmaps in memory. Accelerates copy
> plane when both objects are in the GPU. Includes copy_window
> acceleration too.
>
> v2: Use NV_texture_barrier for non-overlapping copies within the same
> drawable
>
> v3: Switch to glamor_make_current
>
> v4: Do overlap check on the bounding box of the region rather than
>     on individual boxes
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
>
> fixup copy
>
> Signed-off-by: Keith Packard <keithp at keithp.com>

There's some mess in the commit message from a squash.

The code looks good now and I'd like to just get some comments touched up.
However, regardless of what you decide to do with my comment suggestions,

Reviewed-by: Eric Anholt <eric at anholt.net>

> +/*
> + * Copy from GPU to GPU by using the source
> + * as a texture and painting that into the destination
> + */

Extended comment:

/**
 * Implements CopyPlane and CopyArea from the GPU to the GPU by using
 * the source as a texture and painting that into the destination.
 *
 * This requires that source and dest are different textures, or that
 * (if the copy area doesn't overlap), GL_NV_texture_barrier is used
 * to ensure that the caches are flushed at the right times.
 */

> +static Bool
> +glamor_copy_fbo_fbo_draw(DrawablePtr src,
> +                         DrawablePtr dst,
> +                         GCPtr gc,
> +                         BoxPtr box,
> +                         int nbox,
> +                         int dx,
> +                         int dy,
> +                         Bool reverse,
> +                         Bool upsidedown,
> +                         Pixel bitplane,
> +                         void *closure)

> +/*
> + * Copy from GPU to GPU, but create
> + * a temporary pixmap in the middle to
> + * correctly handle overlapping copies
> + * in systems lacking glCopyPixels
> + */

/**
 * Copies from the GPU to the GPU using a temporary pixmap in between,
 * to correctly handle overlapping copies.
 */

(you're not using copypixels at all any more)

> +static Bool
> +glamor_copy_fbo_fbo_temp(DrawablePtr src,
> +                         DrawablePtr dst,
> +                         GCPtr gc,
> +                         BoxPtr box,
> +                         int nbox,
> +                         int dx,
> +                         int dy,
> +                         Bool reverse,
> +                         Bool upsidedown,
> +                         Pixel bitplane,
> +                         void *closure)

> +/*
> + * Check and see if this copy operation overlaps within
> + * the same pixmap
> + */

/**
 * Returns TRUE if the copy has to be implemented with
 * glamor_copy_fbo_fbo_temp() instead of glamor_copy_fbo_fbo().
 *
 * If the src and dst are in the same pixmap, then glamor_copy_fbo_fbo()'s
 * sampling would give undefined results (since the same texture would be
 * bound as an FBO destination and as a texture source).  However, if we
 * have GL_NV_texture_barrier, we can take advantage of the exception it
 * added:
 *
 *    "- If a texel has been written, then in order to safely read the result
 *       a texel fetch must be in a subsequent Draw separated by the command
 *
 *       void TextureBarrierNV(void);
 *
 *    TextureBarrierNV() will guarantee that writes have completed and caches
 *    have been invalidated before subsequent Draws are executed."
 */

> +static Bool
> +glamor_copy_needs_temp(DrawablePtr src,
> +                       DrawablePtr dst,
> +                       BoxPtr box,
> +                       int nbox,
> +                       int dx,
> +                       int dy)
> +{
> +    PixmapPtr src_pixmap = glamor_get_drawable_pixmap(src);
> +    PixmapPtr dst_pixmap = glamor_get_drawable_pixmap(dst);
> +    ScreenPtr screen = dst->pScreen;
> +    glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
> +    int n;
> +    int dst_off_x, dst_off_y;
> +    int src_off_x, src_off_y;
> +    BoxRec bounds;
> +
> +    if (src_pixmap != dst_pixmap)
> +        return FALSE;
> +
> +    if (nbox == 0)
> +        return FALSE;
> +
> +    if (!glamor_priv->has_nv_texture_barrier)
> +        return TRUE;
> +
> +    glamor_get_drawable_deltas(src, src_pixmap, &src_off_x, &src_off_y);
> +    glamor_get_drawable_deltas(dst, dst_pixmap, &dst_off_x, &dst_off_y);
> +
> +    bounds = box[0];
> +    for (n = 1; n < nbox; n++) {
> +        bounds.x1 = min(bounds.x1, box[n].x1);
> +        bounds.y1 = min(bounds.y1, box[n].y1);
> +
> +        bounds.x2 = max(bounds.x2, box[n].x2);
> +        bounds.y2 = max(bounds.y2, box[n].y2);
> +    }
> +
> +    /* Check to see if the pixmap-relative boxes overlap in both X and Y,
> +     * in which case we can't rely on NV_texture_barrier and must
> +     * make a temporary copy
> +     *
> +     *  dst.x1                     < src.x2 &&
> +     *  src.x1                     < dst.x2 &&
> +     *
> +     *  dst.y1                     < src.y2 &&
> +     *  src.y1                     < dst.y2
> +     */
> +    if (bounds.x1 + dst_off_x      < bounds.x2 + dx + src_off_x &&
> +        bounds.x1 + dx + src_off_x < bounds.x2 + dst_off_x &&
> +
> +        bounds.y1 + dst_off_y      < bounds.y2 + dy + src_off_y &&
> +        bounds.y1 + dy + src_off_y < bounds.y2 + dst_off_y) {
> +        return TRUE;
> +    }
> +
> +    glTextureBarrierNV();
> +
> +    return FALSE;
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140506/82827541/attachment.sig>