+Marek (looks like I forgot to Cc you on this v6 :-/). On Mon, 22 Jul 2019 09:49:31 +0200 Boris Brezillon <boris.brezil...@collabora.com> wrote:
> Hi Qiang, > > On Sun, 21 Jul 2019 17:02:54 +0800 > Qiang Yu <yuq...@gmail.com> wrote: > > > 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. > > > > Indeed, the FREE() is missing. > > > In fact I prefer the v5 way which just uses `int *rects` to avoid > > unnecessary > > conversion. > > Well, Erik suggested to pass an array of pipe_boxe objects to make > things clearer, and I can of agree with him. Moreover, I'd expect the *kind of > extra allocation + pipe_box init overhead to be negligible. Erik, Qiang, Marek, Any comment on this v5. Should I send a v6 adding the missing FREE() call. Anything else you'd like me to change? Thanks, Boris _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev