Damage as a DIX notion

[Eric Anholt]

Keith Packard <keithp at keithp.com> writes:

> Adam, Eric and I were chatting at XDC about how to make damage
> computations more efficient and more useful.
>
> The impetus of this was that glamor on tilers would like to have the
> bounds of every operation to reduce the number of tiles processed for
> small operations. Damage already computes the bounds of every operation,
> but that bounds is not available to glamor. Re-computing the bounds
> would be wasteful, so instead we want to capture the computation done
> (potentially) by damage instead of redoing it.
>
> I've taken a couple of stabs at a design, and I figured I'd share my
> current ideas to get some feedback.
>
> First off, I think I need a set of bounds computing functions, one per
> rendering operation. I've started writing these to compute a simple
> bounding box for an entire operation. This may be larger than what
> Damage currently computes, but I don't think a closer bounds is
> interesting for most operations.
>
> Next, I think we just want to pass any computed bounds along with the
> rendering operation. This means tacking on another argument to all of
> them. A NULL value means that no bounds have been computed yet.
>
> Finally, each element in the stack which needs the operation bounds will
> have a local BoxRec to hold a bounds, and then call the
> operation-specific bounds function, which will either use the existing
> bounds (if non-NULL), or fill in the local BoxRec and return that.
>
> Here's a sample of these new functions:
>
>         BoxPtr
>         bounds_poly_point(DrawablePtr drawable,
>                           GCPtr gc,
>                           int npoint,
>                           const xPoint *points,
>                           BoxPtr bounds,
>                           BoxPtr local_bounds)
>         {
>             int i;
>
>             /* Are the operation bounds already computed? */
>             if (bounds != NULL)
>                 return bounds;
>
>             /* Compute bounds of this operation */
>             bounds = local_bounds;
>             if (npoint == 0) {
>                 *bounds = bounds_empty();
>                 return bounds;
>             }
>
>             open_rect_to_box(bounds, points[0].x, points[0].y, 1, 1);
>             for (i = 1; i < npoint; i++)
>                 open_rect_union(bounds, bounds, points[i].x, points[i].y, 1, 1);
>             bounds_trim_gc(drawable, gc, bounds);
>             return bounds;
>         }
>
> Here's how the Damage layer uses this:
>
>         static void
>         damagePolyPoint(DrawablePtr pDrawable,
>                         GCPtr pGC, int npt, const xPoint * ppt,
>                         BoxPtr bounds)
>         {
>             BoxRec local_bounds;
>             DAMAGE_GC_OP_PROLOGUE(pGC, pDrawable);
>
>             if (checkGCDamage(pDamage, pGC)) {
>                 bounds = bounds_poly_point(pDrawable, pGC, npt, ppt, bounds, &local_bounds);
>                 if (!bounds_is_empty(bounds))
>                     damageDamageRelBox(pDrawable, bounds, pGC->subWindowMode);
>             }
>             (*pGC->ops->PolyPoint) (pDrawable, pGC, npt, ppt, bounds);
>             damageRegionProcessPending(pDamage);
>             DAMAGE_GC_OP_EPILOGUE(pGC, pDrawable);
>         }

(I assume there's a missing BoxPtr bounds = NULL at the top of this function)

> We can obviously change the internals of Damage so that we just pass the
> damage box to damageRegionProcessPending; that will greatly simplify the
> internals of that code.
>
> Now, down in glamor, we can do the same thing to compute the bounds as
> needed. Here's how a glamor fallback works:
>
>         static void
>         glamor_poly_fill_rect_bail(DrawablePtr drawable,
>                                    GCPtr gc, int nrect, const xRectangle *prect,
>                                    BoxPtr bounds)
>         {
>             BoxRec local_bounds;
>
>             glamor_fallback("to %p (%c)\n", drawable,
>                             glamor_get_drawable_location(drawable));
>             bounds = bounds_poly_fill_rect(drawable, gc, nrect, prect, bounds, &local_bounds);
>             if (glamor_prepare_access_box(drawable, GLAMOR_ACCESS_RW, bounds) &&
>                 glamor_prepare_access_gc(gc))
>             {
>                 fbPolyFillRect(drawable, gc, nrect, prect, bounds);
>             }
>             glamor_finish_access_gc(gc);
>             glamor_finish_access(drawable);
>         }
>
> If Damage has already computed the bounds of the operation, then glamor
> can re-use that value. Otherwise, it will compute the bounds itself to
> reduce the amount of data shuffled.
>
> I've written just a smattering of the code as I'm assuming we'll improve
> this design and probably end up replacing all of the code. I had
> initially thought that we'd want to take advantage of the patches I've
> sent around which make the rendering data immutable, but that isn't
> necessary for this design.
>
> The current design has one serious problem, and I don't have a solution
> in mind yet. When you draw something that is implemented in terms of
> other drawing operations (like mi lines/points/arcs etc), and if Damage
> isn't being tracked on the drawable, then the bounds are never computed
> for the higher-level operation, and instead you end up computing bounds
> for all of the low level objects instead (e.g. spans).
>
> What we might want is for the lower level operation to somehow get ahold
> of the highest level operation and use that to compute the bounds, but
> that seems like a lot of bookkeeping for an unusual case.
>
> I'm tempted to just ignore the problem for now; the spans will get
> bounds compute as necessary and that will work fine. However, if someone
> has a great idea, I'd love to hear it.

In talking with ajax, I came around to "just compute the bounds up
front, always."

All glamor ops will want the damage for scissoring (we're already
setting scissors all the time, so now we just intersect the new incoming
box with the scissor we were going to set).  All window ops want the
damage for compositing.  Software cursor wants it for screen.  Basically
what get from the kinda complicated lazy damage API is avoiding damage
computation for pixmap-only rendering.

I think the approach outlined here would also work.  I'm just not
convinced it's necessary.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160925/ea5f719a/attachment.sig>