[PATCH] remove damagePostOp() from DamageDamageRegion()

[Michel Dänzer]

On Wed, 2008-08-27 at 15:19 -0700, Aaron Plattner wrote:
> On Wed, Aug 27, 2008 at 07:07:40PM +0200, Michel D?nzer wrote:
> > On Wed, 2008-08-27 at 09:56 -0700, Aaron Plattner wrote:
> > > On Tue, Aug 26, 2008 at 11:58:28PM +0200, Maarten Maathuis wrote:
> > > > This is a patch based on a suggestion by Michael Danzer, because it's
> > > > non-trivial i'm posting it first. The idea is that PostOp only needs
> > > > to be called after doing an actual rendering operation. Please voice
> > > > any concerns you might have.
> > > >
> > > > I CC'ed Aaron Plattner because "he" might be the only out of tree user
> > > > of this symbol.
> > > >
> > > > Maarten.
> > > 
> > > Thanks for the heads-up, Maarten.  We do indeed use DamageDamageRegion to
> > > report damage from things like Xv and OpenGL rendering, and rely on it
> > > actually sending the events.  While we can certainly add calls to
> > > DamageDamagePostOp where necessary, it seems unfortunate to break existing
> > > drivers when it would be trivial to add a new function.
> > 
> > The problem is that DamageDamageRegion calling damageReportPostOp isn't
> > really useful: If DamageDamageRegion is called before a rendering
> > operation, that breaks damage records interested in damage only after
> > rendering operations. If it's called afterwards, that breaks damage
> > records that need pending damage before rendering operations. The EXA
> > pixmap migration logic happens to match both descriptions.
> 
> Right for X rendering.  However, the current DamageDamageRegion behavior is
> exactly what's needed for direct-rendering clients like OpenGL, which is
> what we use it for in the NVIDIA driver.  

I'm not sure how it can be "exactly what's needed" for that. At what
point exactly are you calling DamageDamageRegion, before emitting the
rendering operations or after that or after they have completed? And do
you really need both the pending damage and ReportPostOp semantics in
one go?

> What I meant was that it shouldn't be too hard to leave
> DamageDamageRegion the way it is and migrate the currently incorrect
> users of that function to some new thing that doesn't call
> damageReportPostOp.

As you can see from Maarten's patch and my review of that, it would
require fixing basically all in-tree callers, maybe with a few
exceptions. Not sure that's the best approach.


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer