[Mesa-dev] [PATCH 2/4] gallium: extend resource_resolve to accommodate BlitFramebuffer

[Roland Scheidegger]

> On 07/26/2011 01:49 AM, Roland Scheidegger wrote:
> > (Strange thought sent that before - mail client going crazy...)
> > 
> >> Resolving color buffers is pretty well defined (for standard msaa
> >> at
> >> least). I have no idea how that's supposed to be defined for depth
> >> and stencil values. Frankly I'm not sure what glBlitFramebuffer is
> >> supposed to do here, maybe it's defined somewhere but even
> >> applying
> >> the term "resolve" to a depth buffer seems very iffy. At the very
> >> least it needs to be documented in the gallium docs what
> >> "resolving"
> >> a depth/stencil buffer really means.
> > 
> > Hmm actually it must be like ReadPixels. So it is "recommended"
> > that implementations just use the centermost sample, but this is
> > not required. In particular "any function using depth/stencil
> > values" is valid.
> > Taking this to the extreme, this means just returning 0 is valid (f
> > = 0*sample0 + 0*sample1...) though probably not recommended...
> > Averaging would be allowed as would be any other crazy stuff. In
> > any case calling this step, whatever it does, "resolve" seems
> > abusive with results possibly quite implementation dependent. I
> > have no idea what nv50 does here though I guess given the loose
> > definition it should certainly fit the requirements.
> > 
> But that's great, it means drivers can't do anything wrong here.
> 
> And either way, GL demands that you can "resolve" it so you better
> support it. nv50 gallium would follow the behaviour of the binary
> driver, which is take the value from a single sample.
> Should hw for some reason not be able to do this (I can't imagine
> how),
> well, they can return black. Or whatever the st would do instead.
If this is really useful to anyone, I agree we probably should be able to handle that in the driver (though using the name "resolve" for that operation still irks me but I could live with it). I'd like to hear from others though, certainly I don't really know if it's that useful. State tracker could resolve through using a shader (ok that requires d3d101 feature but it's likely generally a feature which is way more useful than any of this stuff), though certainly a driver could do that internally as well.

> Now again about the rest of the new arguments:
> 
> pipe_resource *aux (from my first reply):
> The are multisampling implementations that store extra sample data
> required for resolve in a separate buffer, nv's coverage sampling for
> example.
> I think the pipe driver can store a link to such a buffer in the
> associated colour resource, so, forget about that one.
Yes I think if it can be hidden it should be hidden, unless we've got some way to deal with such features in some good generic way (which seems hard to do though coverage sampling isn't so special anymore since AMD does the same too nowadays).

> 
> Now the important ones:
> 
> mask: If depth resolve is supported, you are not allowed to clobber
> stencil. You have to allow the pipe driver to not do double the work.
Yes. If we're going to support depth "resolve" there this should be included.

> 
> dst_level:
> If the user resolves to the (n != 0)-th level of a texture, you
> certainly want this; doing an extra copy of several MiB only because
> you
> don't like the looks of an extra parameter in the function
> declaration
> is insane from any angle.
Since source is always level 0, why would you want to resolve to a different than 0 level anyway? I cannot imagine why that would be useful. But maybe my imagination is limited.
There's likely a reason d3d10 decided it didn't want to bother driver writers with this stuff...

> 
> dstx, dsty, box:
> BlitFramebuffer honours scissor state. What if the user is scissoring
> away half of the destination ? Do you really want to push through the
> other half, all that to temporary storage, and then again blit
> several
> MiB of data over, just because you don't like to clutter the
> interface ?
Seems an acceptable tradeoff to me if it's a rare case. It's not like the blit of half a buffer is going to kill your framerate anyway these days. But again I don't know if it's really used that way...
Sort of reminds me of drivers which tried to scissor away things which weren't going to be visible on screen. Just made things way more complex with very little benefits.
But well if you include any of these parameters you can just as well include dst_level too because it won't be a simple
resolve-whole-surface anymore.

> 
> yflip:
> When the user is able to resolve to the window system buffer (if mesa
> is
> kind enough to choose the same component ordering, or if the "same
> format" requirement is interpreted loosely - this is certainly
> possible
> with the binary drivers), then you have to do this flip.
> And you don't want to ALWAYS do double the work. You *really* do not.
> Not !
Yes, I said the yflip seemed the most useful to me. Hopefully you can actually make it without an additional copy...

But really if there is a need for all this stuff I'm not against changing the interface, though it would be really nice if someone else could comment on it. At least you haven't presented some crazy app which actually needs it neither.

Roland