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

[Roland Scheidegger]

> On 07/27/2011 01:25 AM, Roland Scheidegger wrote:
> >> 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.
> That's because it's a little hard to find any application at all
> using
> explicit EXT_framebuffer_multisample (wine will use it soon though).
> 
> But, as Marek Olšák said on IRC,
> "it's not about apps, it's about making our OpenGL implementation
> compliant and fast"
> 
> Now, I just discovered that madness doesn't stop with what I already
> added to the resolve function: we even have
> EXT_framebuffer_multisample_blit_scaled.
> 
> It seems at least someone must find it useful to resolve to a buffer
> of
> different size ...
Sounds possibly more useful than some of the other stuff you can do with glBlitFramebuffer and multisampled buffers (like the blit to different dst level), at least if applied to color buffers...


> 
> Jacob Bornecrantz suggested on IRC that I pack all the arguments in a
> struct like for draw_vbo, so I'll rework my patches for this, and to
> incorporate scaling.
> For the latter I will add a PIPE_CAP_SCALED_RESOLVE.

It really sounds more like multisample_blit uber-blit op though now rather than resource_resolve...
Considering we don't have any pipe driver function for normal (scaled) blits a bit odd but ah well.

Roland