On Mon, Jul 15, 2019 at 8:50 PM Boris Brezillon
<boris.brezil...@collabora.com> wrote:
>
> From: Daniel Stone <dani...@collabora.com>
>
> Add a pipe_screen->set_damage_region() hook to propagate
> set-damage-region requests to the driver, it's then up to the driver to
> decide what to do with this piece of information.
>
> If the hook is left unassigned, the buffer-damage extension is
> considered unsupported.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com>
> Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzw...@collabora.com>
> ---
> Hello Qiang,
>
> I intentionally dropped your R-b/T-b on this patch since the
> ->set_damage_region() prototype has changed. Feel free to add it back.
>
> Regards,
>
> Boris
>
> Changes in v6:
> * Pass pipe_box objects instead ints
> * Document the set_damage_region() hook
>
> Changes in v5:
> * Add Alyssa's R-b
> ---
> src/gallium/include/pipe/p_screen.h | 17 ++++++++++++++
> src/gallium/state_trackers/dri/dri2.c | 34 +++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/src/gallium/include/pipe/p_screen.h
> b/src/gallium/include/pipe/p_screen.h
> index 3f9bad470950..11a6aa939124 100644
> --- a/src/gallium/include/pipe/p_screen.h
> +++ b/src/gallium/include/pipe/p_screen.h
> @@ -464,6 +464,23 @@ struct pipe_screen {
> bool (*is_parallel_shader_compilation_finished)(struct pipe_screen
> *screen,
> void *shader,
> unsigned shader_type);
> +
> + /**
> + * Set the damage region (called when KHR_partial_update() is invoked).
> + * This function is passed an array of rectangles encoding the damage
> area.
> + * rects are using the bottom-left origin convention.
> + * nrects = 0 means 'reset the damage region'. What 'reset' implies is HW
> + * specific. For tile-based renderers, the damage extent is typically set
> + * to cover the whole resource with no damage rect (or a 0-size damage
> + * rect). This way, the existing resource content is reloaded into the
> + * local tile buffer for every tile thus making partial tile update
> + * possible. For HW operating in immediate mode, this reset operation is
> + * likely to be a NOOP.
> + */
> + void (*set_damage_region)(struct pipe_screen *screen,
> + struct pipe_resource *resource,
> + unsigned int nrects,
> + const struct pipe_box *rects);
> };
>
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index 5a7ec878bab0..5273b95cd5fb 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -1807,6 +1807,35 @@ static const __DRI2interopExtension
> dri2InteropExtension = {
> .export_object = dri2_interop_export_object
> };
>
> +/**
> + * \brief the DRI2bufferDamageExtension set_damage_region method
> + */
> +static void
> +dri2_set_damage_region(__DRIdrawable *dPriv, unsigned int nrects, int *rects)
> +{
> + struct dri_drawable *drawable = dri_drawable(dPriv);
> + struct pipe_resource *resource =
> drawable->textures[ST_ATTACHMENT_BACK_LEFT];
> + struct pipe_screen *screen = resource->screen;
> + struct pipe_box *boxes = NULL;
> +
> + if (nrects) {
> + boxes = CALLOC(nrects, sizeof(*boxes));
> + assert(boxes);
Where does this boxes array get freed? I can't find in your patch 6 either.
In fact I prefer the v5 way which just uses `int *rects` to avoid unnecessary
conversion.
Regards,
Qiang
> +
> + for (unsigned int i = 0; i < nrects; i++) {
> + int *rect = &rects[i * 4];
> +
> + u_box_2d(rect[0], rect[1], rect[2], rect[3], &boxes[i]);
> + }
> + }
> +
> + screen->set_damage_region(screen, resource, nrects, boxes);
> +}
> +
> +static __DRI2bufferDamageExtension dri2BufferDamageExtension = {
> + .base = { __DRI2_BUFFER_DAMAGE, 1 },
> +};
> +
> /**
> * \brief the DRI2ConfigQueryExtension configQueryb method
> */
> @@ -1908,6 +1937,7 @@ static const __DRIextension *dri_screen_extensions[] = {
> &dri2GalliumConfigQueryExtension.base,
> &dri2ThrottleExtension.base,
> &dri2FenceExtension.base,
> + &dri2BufferDamageExtension.base,
> &dri2InteropExtension.base,
> &dri2NoErrorExtension.base,
> &driBlobExtension.base,
> @@ -1923,6 +1953,7 @@ static const __DRIextension
> *dri_robust_screen_extensions[] = {
> &dri2ThrottleExtension.base,
> &dri2FenceExtension.base,
> &dri2InteropExtension.base,
> + &dri2BufferDamageExtension.base,
> &dri2Robustness.base,
> &dri2NoErrorExtension.base,
> &driBlobExtension.base,
> @@ -1983,6 +2014,9 @@ dri2_init_screen(__DRIscreen * sPriv)
> }
> }
>
> + if (pscreen->set_damage_region)
> + dri2BufferDamageExtension.set_damage_region = dri2_set_damage_region;
> +
> if (pscreen->get_param(pscreen, PIPE_CAP_DEVICE_RESET_STATUS_QUERY)) {
> sPriv->extensions = dri_robust_screen_extensions;
> screen->has_reset_status_query = true;
> --
> 2.21.0
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
