I admit I haven't fully understood what's being proposed yet. But just a few quick words.
I always wanted to have a "present" method that ensures that the contents of a resource is made visible to whatever the consumer is (full-screen flip, blit to primary surface, a compositor, etc.). We have been using off-the-side interfaces on Windows to achieve this [1] but I believe that this is a cross-platform problem so there should be something in Gallium interfaces for that. So I'd be supportive if there is a proposal that caters for everything. I'd hate to have a new interface aimed at a single use case. Jose [1] See stw_winsys::present and stw_winsys::compose in http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/wgl/stw_winsys.h ----- Original Message ----- > Let's ignore the function name for now, that's easy to change and I am > open to suggestions. flush_window_resource, flush_shared_resource, and > sync_shared_resource are possible candidates as well as > gonna_put_this_resource_on_the_screen_right_away. :) The idea is to > expose a function which will ensure resource coherency between gallium > and an external client (it could be another process or another > device). > > Gallium drivers really have no way to know which resource is the > window front or back buffer. It has always been like that since I > joined Mesa (about 3.5 years ago). The bind flags won't work, because > there can be multiple resources with that same flag. Even knowing > which resource is front and back is useless, because there is only one > resource which needs to be flushed at a given point and only st/dri > knows which one it is. > > Well, there is at least something good about classic drivers: They > don't have to share their DRI support code with everybody else. > > Marek > > On Thu, Jul 11, 2013 at 10:24 PM, Roland Scheidegger <srol...@vmware.com> > wrote: > > Am 11.07.2013 20:15, schrieb Marek Olšák: > >> Hi Roland, > >> > >> The fast color clear on Radeon doesn't touch the memory of the texture > >> resource. Instead, it changes some GPU meta data that say the resource > >> is cleared (the location of the meta data is stored in pipe_resource). > >> This works fine as long as the gallium pipe_resource structure is used > >> for accessing the resource. That's not the case with the DDX, which is > >> responsible for putting the resource on the screen and it obviously > >> has no idea about the contents of pipe_resource, so it doesn't know > >> that the resource is in a cleared state and a special "flush" > >> operation must be done to actually write the "cleared" pixels (which > >> haven't been overwritten by new geometry of course). > >> > >> The easiest way to solve this is to "flush" the cleared resource in > >> SwapBuffers and where the front buffer is flushed. The Gallium driver > >> can't do it automatically, because it has no notion of front and back > >> buffers nor does it know which resource must be "flushed". That's why > >> a new pipe_context function is being proposed, which was originally my > >> idea. > > Ok I see now what it's used for. I don't like "expand" though this is > > more like a resource flush (i.e. do whatever is necessary to make the > > contents of this resource accessible by some dumb interface which knows > > nothing about resources). > > It isn't quite true that the gallium driver doesn't know this is a > > resource used as a window fb, as those should have the > > PIPE_BIND_DISPLAY_TARGET flag set (or maybe PIPE_BIND_SCANOUT). Don't > > ask me though how they work or if you could figure out what you'd need > > to flush... > > As Christoph has said, there pipe_screen.flush_front_buffer which could > > supposedly do this but it would also present the resource to screen I > > guess (only really used in sw winsys). > > So I'm still not really sure if a new function is really needed, I'm not > > familiar enough with all the interface stuff. If it is though I don't > > like the name, and it also would need documentation. > > > > Roland > > > > > >> > >> This commit only fixes r600g for st/dri. Any other co-state tracker > >> (like st/egl and st/xlib) will be broken if it's used with r600g. I > >> think we can ignore st/xlib. Not sure how important st/egl is (not > >> required for EGL under X). > >> > >> Marek > >> > >> On Wed, Jul 10, 2013 at 7:32 PM, Roland Scheidegger <srol...@vmware.com> > >> wrote: > >>> I don't quite understand what this should do, at first sight it looks > >>> like a ugly hack (which should really not be part of gallium interface) > >>> to make fast color clearing work better with window framebuffers. > >>> Seems to go against the idea of resources (which are immutable, well not > >>> the contents but the properties). > >>> (If anything I wanted an interface to change bind flags for resources > >>> after initialization, because they are near impossible to guarantee with > >>> OpenGL's (or d3d9 for that matter) distinct texture/fb model, but that > >>> would also be quite a hack.) > >>> Could you elaborate with some example what that's supposed to do in > >>> practice? > >>> > >>> Roland > >>> > >>> > >>> Am 10.07.2013 18:20, schrieb Grigori Goronzy: > >>>> This interface is used to expand fast-cleared window system > >>>> colorbuffers. > >>>> --- > >>>> src/gallium/include/pipe/p_context.h | 8 ++++++++ > >>>> src/gallium/state_trackers/dri/common/dri_drawable.c | 4 ++++ > >>>> src/gallium/state_trackers/dri/drm/dri2.c | 8 ++++++-- > >>>> 3 files changed, 18 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/src/gallium/include/pipe/p_context.h > >>>> b/src/gallium/include/pipe/p_context.h > >>>> index aa18cbf..38d5ee6 100644 > >>>> --- a/src/gallium/include/pipe/p_context.h > >>>> +++ b/src/gallium/include/pipe/p_context.h > >>>> @@ -354,6 +354,14 @@ struct pipe_context { > >>>> unsigned dstx, unsigned dsty, > >>>> unsigned width, unsigned height); > >>>> > >>>> + /** > >>>> + * Expand a color resource in-place. > >>>> + * > >>>> + * \return TRUE if resource was expanded, FALSE otherwise > >>>> + */ > >>>> + boolean (*expand_resource)(struct pipe_context *pipe, > >>>> + struct pipe_resource *dst); > >>>> + > >>>> /** Flush draw commands > >>>> * > >>>> * \param flags bitfield of enum pipe_flush_flags values. > >>>> diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c > >>>> b/src/gallium/state_trackers/dri/common/dri_drawable.c > >>>> index 18d8d89..b67a497 100644 > >>>> --- a/src/gallium/state_trackers/dri/common/dri_drawable.c > >>>> +++ b/src/gallium/state_trackers/dri/common/dri_drawable.c > >>>> @@ -448,6 +448,10 @@ dri_flush(__DRIcontext *cPriv, > >>>> } > >>>> > >>>> /* FRONT_LEFT is resolved in drawable->flush_frontbuffer. */ > >>>> + } else if (ctx->st->pipe->expand_resource) { > >>>> + /* Expand fast-cleared framebuffer */ > >>>> + ctx->st->pipe->expand_resource(ctx->st->pipe, > >>>> + drawable->textures[ST_ATTACHMENT_BACK_LEFT]); > >>>> } > >>>> > >>>> dri_postprocessing(ctx, drawable, ST_ATTACHMENT_BACK_LEFT); > >>>> diff --git a/src/gallium/state_trackers/dri/drm/dri2.c > >>>> b/src/gallium/state_trackers/dri/drm/dri2.c > >>>> index 1dcc1f7..97784ec 100644 > >>>> --- a/src/gallium/state_trackers/dri/drm/dri2.c > >>>> +++ b/src/gallium/state_trackers/dri/drm/dri2.c > >>>> @@ -490,18 +490,22 @@ dri2_flush_frontbuffer(struct dri_context *ctx, > >>>> { > >>>> __DRIdrawable *dri_drawable = drawable->dPriv; > >>>> struct __DRIdri2LoaderExtensionRec *loader = > >>>> drawable->sPriv->dri2.loader; > >>>> + struct pipe_context *pipe = ctx->st->pipe; > >>>> > >>>> if (statt != ST_ATTACHMENT_FRONT_LEFT) > >>>> return; > >>>> > >>>> if (drawable->stvis.samples > 1) { > >>>> - struct pipe_context *pipe = ctx->st->pipe; > >>>> - > >>>> /* Resolve the front buffer. */ > >>>> dri_pipe_blit(ctx->st->pipe, > >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT], > >>>> drawable->msaa_textures[ST_ATTACHMENT_FRONT_LEFT]); > >>>> pipe->flush(pipe, NULL, 0); > >>>> + } else if (pipe->expand_resource && > >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT]) { > >>>> + /* Expand fast-cleared framebuffer */ > >>>> + if (pipe->expand_resource(pipe, > >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT])) { > >>>> + pipe->flush(pipe, NULL, 0); > >>>> + } > >>>> } > >>>> > >>>> if (loader->flushFrontBuffer) { > >>>> > >>> _______________________________________________ > >>> mesa-dev mailing list > >>> mesa-dev@lists.freedesktop.org > >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
