[Mesa-dev] [PATCH v4 4/5] st/dri2: Implement DRI2bufferDamageExtension

[Erik Faye-Lund]

On Wed, 2019-07-03 at 12:33 +0200, Boris Brezillon wrote:
> On Wed, 03 Jul 2019 11:54:29 +0200
> Erik Faye-Lund <erik.faye-lund at collabora.com> wrote:
> 
> > On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> > > Ah-ha, now we're into parts of the stack I can claim to
> > > understand!  
> > > > :)  
> > > 
> > > Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
> > > 
> > > On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon
> > > wrote:  
> > > > From: Daniel Stone <daniels at 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 <daniels at collabora.com>
> > > > Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> > > > ---
> > > >  src/gallium/include/pipe/p_screen.h   |  7 +++++++
> > > >  src/gallium/state_trackers/dri/dri2.c | 22
> > > > ++++++++++++++++++++++
> > > >  2 files changed, 29 insertions(+)
> > > > 
> > > > diff --git a/src/gallium/include/pipe/p_screen.h
> > > > b/src/gallium/include/pipe/p_screen.h
> > > > index 3f9bad470950..8df12ee4f865 100644
> > > > --- a/src/gallium/include/pipe/p_screen.h
> > > > +++ b/src/gallium/include/pipe/p_screen.h
> > > > @@ -464,6 +464,13 @@ struct pipe_screen {
> > > >     bool (*is_parallel_shader_compilation_finished)(struct
> > > > pipe_screen *screen,
> > > >                                                     void
> > > > *shader,
> > > >                                                     unsigned
> > > > shader_type);
> > > > +
> > > > +   /**
> > > > +    * Set damage region.
> > > > +    */
> > > > +   void (*set_damage_region)(struct pipe_screen *screen,
> > > > +                             struct pipe_resource *resource,
> > > > +                             unsigned int nrects, int
> > > > *rects);  
> > 
> > I would kinda have expected rects to be an array of pipe_box
> > instead of
> > just an array of integers, as that'd be a bit easier to know the
> > semantics of...
> 
> Sure, I can do that. Should I do the Y-flip as part of the ints ->
> box
> conversion or should I keep the "origin is bottom-left" semantic?

Good question? :)

My instinct would be to have the regions in "memory layout order"
rather than "window sytem presentation order", which I guess would be
without any sort of y-flipping or rotation applied. I guess that would
make the answer "no"? But I'm not sure, I haven't read up on the spec
here.