From ac8fdbc1c723afb19eeaba5457ba78d0bf33b8d4 Mon Sep 17 00:00:00 2001 From: Thomas Hellstrom Date: Wed, 29 Jun 2011 09:00:23 +0200 Subject: [PATCH] st-api: Rework how drawables are invalidated v3. The api and the state tracker manager code as well as the state tracker code assumed that only a single context could be bound to a drawable. That is not a valid assumption, since multiple contexts can bind to the same drawable. Fix this by making it the state tracker's responsibility to update all contexts binding to a drawable Note that the state trackers themselves don't use atomic stamps on frame-buffers. Multiple context rendering to the same drawable should be protected by the application. Signed-off-by: Thomas Hellstrom --- src/gallium/include/state_tracker/st_api.h | 25 ++--- .../state_trackers/dri/common/dri_drawable.c | 1 + src/gallium/state_trackers/dri/drm/dri2.c | 4 +- src/gallium/state_trackers/dri/sw/drisw.c | 5 +- src/gallium/state_trackers/egl/common/egl_g3d.c | 13 +-- .../state_trackers/egl/common/egl_g3d_api.c | 7 -- src/gallium/state_trackers/egl/common/egl_g3d_st.c | 2 + src/gallium/state_trackers/vega/vg_context.h | 5 +- src/gallium/state_trackers/vega/vg_manager.c | 48 +++++----- src/gallium/state_trackers/wgl/stw_context.c | 6 +- src/gallium/state_trackers/wgl/stw_st.c | 2 + src/mesa/state_tracker/st_cb_viewport.c | 15 ++- src/mesa/state_tracker/st_context.h | 6 +- src/mesa/state_tracker/st_manager.c | 102 ++++++++++++--------- 14 files changed, 121 insertions(+), 120 deletions(-) diff --git a/src/gallium/include/state_tracker/st_api.h b/src/gallium/include/state_tracker/st_api.h index 04fc7c6..f7cc243 100644 --- a/src/gallium/include/state_tracker/st_api.h +++ b/src/gallium/include/state_tracker/st_api.h @@ -253,6 +253,12 @@ struct st_context_attribs struct st_framebuffer_iface { /** + * Atomic stamp which changes when framebuffers need to be updated. + */ + + int32_t stamp; + + /** * Available for the state tracker manager to use. */ void *st_manager_private; @@ -315,25 +321,6 @@ struct st_context_iface void (*destroy)(struct st_context_iface *stctxi); /** - * Invalidate the current textures that was taken from a framebuffer. - * - * The state tracker manager calls this function to let the rendering - * context know that it should update the textures it got from - * st_framebuffer_iface::validate. It should do so at the latest time possible. - * Possible right before sending triangles to the pipe context. - * - * For certain platforms this function might be called from a thread other - * than the thread that the context is currently bound in, and must - * therefore be thread safe. But it is the state tracker manager's - * responsibility to make sure that the framebuffer is bound to the context - * and the API context is current for the duration of this call. - * - * Thus reducing the sync primitive needed to a single atomic flag. - */ - void (*notify_invalid_framebuffer)(struct st_context_iface *stctxi, - struct st_framebuffer_iface *stfbi); - - /** * Flush all drawing from context to the pipe also flushes the pipe. */ void (*flush)(struct st_context_iface *stctxi, unsigned flags, diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c b/src/gallium/state_trackers/dri/common/dri_drawable.c index 28a33ac..7b8de31 100644 --- a/src/gallium/state_trackers/dri/common/dri_drawable.c +++ b/src/gallium/state_trackers/dri/common/dri_drawable.c @@ -136,6 +136,7 @@ dri_create_buffer(__DRIscreen * sPriv, drawable->sPriv = sPriv; drawable->dPriv = dPriv; dPriv->driverPrivate = (void *)drawable; + p_atomic_set(&drawable->base.stamp, 1); return GL_TRUE; fail: diff --git a/src/gallium/state_trackers/dri/drm/dri2.c b/src/gallium/state_trackers/dri/drm/dri2.c index 0296a78..fe4ddb3 100644 --- a/src/gallium/state_trackers/dri/drm/dri2.c +++ b/src/gallium/state_trackers/dri/drm/dri2.c @@ -52,13 +52,11 @@ static void dri2_invalidate_drawable(__DRIdrawable *dPriv) { struct dri_drawable *drawable = dri_drawable(dPriv); - struct dri_context *ctx = drawable->context; dri2InvalidateDrawable(dPriv); drawable->dPriv->lastStamp = *drawable->dPriv->pStamp; - if (ctx) - ctx->st->notify_invalid_framebuffer(ctx->st, &drawable->base); + p_atomic_inc(&drawable->base.stamp); } static const __DRI2flushExtension dri2FlushExtension = { diff --git a/src/gallium/state_trackers/dri/sw/drisw.c b/src/gallium/state_trackers/dri/sw/drisw.c index ac11f7c..a1879a8 100644 --- a/src/gallium/state_trackers/dri/sw/drisw.c +++ b/src/gallium/state_trackers/dri/sw/drisw.c @@ -103,14 +103,11 @@ drisw_present_texture(__DRIdrawable *dPriv, static INLINE void drisw_invalidate_drawable(__DRIdrawable *dPriv) { - struct dri_context *ctx = dri_get_current(dPriv->driScreenPriv); struct dri_drawable *drawable = dri_drawable(dPriv); drawable->texture_stamp = dPriv->lastStamp - 1; - /* check if swapping currently bound buffer */ - if (ctx && ctx->dPriv == dPriv) - ctx->st->notify_invalid_framebuffer(ctx->st, &drawable->base); + p_atomic_inc(&drawable->base.stamp); } static INLINE void diff --git a/src/gallium/state_trackers/egl/common/egl_g3d.c b/src/gallium/state_trackers/egl/common/egl_g3d.c index 7d1eafe..6649f02 100644 --- a/src/gallium/state_trackers/egl/common/egl_g3d.c +++ b/src/gallium/state_trackers/egl/common/egl_g3d.c @@ -31,6 +31,7 @@ #include "util/u_memory.h" #include "util/u_format.h" #include "util/u_string.h" +#include "util/u_atomic.h" #include "egl_g3d.h" #include "egl_g3d_api.h" @@ -45,15 +46,9 @@ egl_g3d_invalid_surface(struct native_display *ndpy, { /* XXX not thread safe? */ struct egl_g3d_surface *gsurf = egl_g3d_surface(nsurf->user_data); - struct egl_g3d_context *gctx; - - /* - * Some functions such as egl_g3d_copy_buffers create a temporary native - * surface. There is no gsurf associated with it. - */ - gctx = (gsurf) ? egl_g3d_context(gsurf->base.CurrentContext) : NULL; - if (gctx) - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, gsurf->stfbi); + + if (gsurf && gsurf->stfbi) + p_atomic_inc(&gsurf->stfbi->stamp); } static struct pipe_screen * diff --git a/src/gallium/state_trackers/egl/common/egl_g3d_api.c b/src/gallium/state_trackers/egl/common/egl_g3d_api.c index cd68e97..f897054 100644 --- a/src/gallium/state_trackers/egl/common/egl_g3d_api.c +++ b/src/gallium/state_trackers/egl/common/egl_g3d_api.c @@ -504,19 +504,12 @@ egl_g3d_make_current(_EGLDriver *drv, _EGLDisplay *dpy, (gdraw) ? gdraw->stfbi : NULL, (gread) ? gread->stfbi : NULL); if (ok) { if (gdraw) { - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, - gdraw->stfbi); - if (gdraw->base.Type == EGL_WINDOW_BIT) { gctx->base.WindowRenderBuffer = (gdraw->stvis.render_buffer == ST_ATTACHMENT_FRONT_LEFT) ? EGL_SINGLE_BUFFER : EGL_BACK_BUFFER; } } - if (gread && gread != gdraw) { - gctx->stctxi->notify_invalid_framebuffer(gctx->stctxi, - gread->stfbi); - } } } else if (old_gctx) { diff --git a/src/gallium/state_trackers/egl/common/egl_g3d_st.c b/src/gallium/state_trackers/egl/common/egl_g3d_st.c index 2b944b5..60c3e33 100644 --- a/src/gallium/state_trackers/egl/common/egl_g3d_st.c +++ b/src/gallium/state_trackers/egl/common/egl_g3d_st.c @@ -292,6 +292,8 @@ egl_g3d_create_st_framebuffer(_EGLSurface *surf) return NULL; stfbi->visual = &gsurf->stvis; + p_atomic_set(&stfbi->stamp, 1); + if (gsurf->base.Type != EGL_PBUFFER_BIT) { stfbi->flush_front = egl_g3d_st_framebuffer_flush_front; stfbi->validate = egl_g3d_st_framebuffer_validate; diff --git a/src/gallium/state_trackers/vega/vg_context.h b/src/gallium/state_trackers/vega/vg_context.h index 71491a5..d91ee97 100644 --- a/src/gallium/state_trackers/vega/vg_context.h +++ b/src/gallium/state_trackers/vega/vg_context.h @@ -65,6 +65,8 @@ struct st_framebuffer { enum st_attachment_type strb_att; void *privateData; + int32_t stamp; + int32_t iface_stamp; }; enum vg_object_type { @@ -105,7 +107,6 @@ struct vg_context VGErrorCode _error; struct st_framebuffer *draw_buffer; - int32_t draw_buffer_invalid; struct cso_hash *owned_objects[VG_OBJECT_LAST]; @@ -129,6 +130,8 @@ struct vg_context struct vg_paint *default_paint; struct blit_state *blit; + + int32_t draw_stamp; }; diff --git a/src/gallium/state_trackers/vega/vg_manager.c b/src/gallium/state_trackers/vega/vg_manager.c index eeea686..dec1581 100644 --- a/src/gallium/state_trackers/vega/vg_manager.c +++ b/src/gallium/state_trackers/vega/vg_manager.c @@ -106,35 +106,38 @@ vg_manager_validate_framebuffer(struct vg_context *ctx) { struct st_framebuffer *stfb = ctx->draw_buffer; struct pipe_resource *pt; + int32_t new_stamp; /* no binding surface */ if (!stfb) return; - if (!p_atomic_read(&ctx->draw_buffer_invalid)) - return; + new_stamp = p_atomic_read(&stfb->iface->stamp); + if (stfb->iface_stamp != new_stamp) { + do { + /* validate the fb */ + if (!stfb->iface->validate(stfb->iface, &stfb->strb_att, + 1, &pt) || !pt) + return; - /* validate the fb */ - if (!stfb->iface->validate(stfb->iface, &stfb->strb_att, 1, &pt) || !pt) - return; + stfb->iface_stamp = new_stamp; + new_stamp = p_atomic_read(&stfb->iface->stamp); - p_atomic_set(&ctx->draw_buffer_invalid, FALSE); + } while (stfb->iface_stamp != new_stamp); - if (vg_context_update_color_rb(ctx, pt) || - stfb->width != pt->width0 || - stfb->height != pt->height0) - ctx->state.dirty |= FRAMEBUFFER_DIRTY; + if (vg_context_update_color_rb(ctx, pt) || + stfb->width != pt->width0 || + stfb->height != pt->height0) + ++stfb->stamp; - stfb->width = pt->width0; - stfb->height = pt->height0; -} + stfb->width = pt->width0; + stfb->height = pt->height0; + } -static void -vg_context_notify_invalid_framebuffer(struct st_context_iface *stctxi, - struct st_framebuffer_iface *stfbi) -{ - struct vg_context *ctx = (struct vg_context *) stctxi; - p_atomic_set(&ctx->draw_buffer_invalid, TRUE); + if (ctx->draw_stamp != stfb->stamp) { + ctx->state.dirty |= FRAMEBUFFER_DIRTY; + ctx->draw_stamp = stfb->stamp; + } } static void @@ -187,8 +190,6 @@ vg_api_create_context(struct st_api *stapi, struct st_manager *smapi, ctx->iface.destroy = vg_context_destroy; - ctx->iface.notify_invalid_framebuffer = - vg_context_notify_invalid_framebuffer; ctx->iface.flush = vg_context_flush; ctx->iface.teximage = NULL; @@ -266,8 +267,6 @@ vg_context_bind_framebuffers(struct st_context_iface *stctxi, if (stdrawi != streadi) return FALSE; - p_atomic_set(&ctx->draw_buffer_invalid, TRUE); - strb_att = (stdrawi) ? choose_attachment(stdrawi) : ST_ATTACHMENT_INVALID; if (ctx->draw_buffer) { @@ -313,11 +312,14 @@ vg_context_bind_framebuffers(struct st_context_iface *stctxi, stfb->width = 0; stfb->height = 0; stfb->strb_att = strb_att; + stfb->stamp = 1; + stfb->iface_stamp = p_atomic_read(&stdrawi->stamp) - 1; ctx->draw_buffer = stfb; } ctx->draw_buffer->iface = stdrawi; + ctx->draw_stamp = ctx->draw_buffer->stamp - 1; return TRUE; } diff --git a/src/gallium/state_trackers/wgl/stw_context.c b/src/gallium/state_trackers/wgl/stw_context.c index 5608d4f..c2839fe 100644 --- a/src/gallium/state_trackers/wgl/stw_context.c +++ b/src/gallium/state_trackers/wgl/stw_context.c @@ -31,6 +31,7 @@ #include "pipe/p_context.h" #include "pipe/p_state.h" #include "util/u_memory.h" +#include "util/u_atomic.h" #include "state_tracker/st_api.h" #include "stw_icd.h" @@ -361,10 +362,7 @@ stw_flush_current_locked( struct stw_framebuffer *fb ) void stw_notify_current_locked( struct stw_framebuffer *fb ) { - struct stw_context *ctx = stw_current_context(); - - if (ctx && ctx->current_framebuffer == fb) - ctx->st->notify_invalid_framebuffer(ctx->st, fb->stfb); + p_atomic_inc(&fb->stfb->stamp); } /** diff --git a/src/gallium/state_trackers/wgl/stw_st.c b/src/gallium/state_trackers/wgl/stw_st.c index 9174533..28c93f4 100644 --- a/src/gallium/state_trackers/wgl/stw_st.c +++ b/src/gallium/state_trackers/wgl/stw_st.c @@ -27,6 +27,7 @@ #include "util/u_memory.h" #include "util/u_inlines.h" +#include "util/u_atomic.h" #include "state_tracker/st_gl_api.h" /* for st_gl_api_create */ #include "stw_st.h" @@ -196,6 +197,7 @@ stw_st_create_framebuffer(struct stw_framebuffer *fb) stwfb->stvis = fb->pfi->stvis; stwfb->base.visual = &stwfb->stvis; + p_atomic_set(&stwfb->base.stamp, 1); stwfb->base.flush_front = stw_st_framebuffer_flush_front; stwfb->base.validate = stw_st_framebuffer_validate; diff --git a/src/mesa/state_tracker/st_cb_viewport.c b/src/mesa/state_tracker/st_cb_viewport.c index 049755e..d4742eb 100644 --- a/src/mesa/state_tracker/st_cb_viewport.c +++ b/src/mesa/state_tracker/st_cb_viewport.c @@ -56,13 +56,20 @@ static void st_viewport(struct gl_context * ctx, GLint x, GLint y, if (!st->invalidate_on_gl_viewport) return; + /* + * Normally we'd want the state tracker manager to mark the drawables + * invalid only when needed. This will force the state tracker manager + * to revalidate the drawable, rather than just update the context with + * the latest cached drawable info. + */ + stdraw = st_ws_framebuffer(st->ctx->DrawBuffer); stread = st_ws_framebuffer(st->ctx->ReadBuffer); - if (stdraw) - p_atomic_set(&stdraw->revalidate, TRUE); - if (stread && stread != stdraw) - p_atomic_set(&stread->revalidate, TRUE); + if (stdraw && stdraw->iface) + stdraw->iface_stamp = p_atomic_read(&stdraw->iface->stamp) - 1; + if (stread && stread != stdraw && stread->iface) + stread->iface_stamp = p_atomic_read(&stread->iface->stamp) - 1; } void st_init_viewport_functions(struct dd_function_table *functions) diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h index ff20703..0a32202 100644 --- a/src/mesa/state_tracker/st_context.h +++ b/src/mesa/state_tracker/st_context.h @@ -204,6 +204,9 @@ struct st_context /* Active render condition. */ struct pipe_query *render_condition; unsigned condition_mode; + + int32_t draw_stamp; + int32_t read_stamp; }; @@ -227,7 +230,8 @@ struct st_framebuffer struct st_framebuffer_iface *iface; enum st_attachment_type statts[ST_ATTACHMENT_COUNT]; unsigned num_statts; - int32_t revalidate; + int32_t stamp; + int32_t iface_stamp; }; diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c index c95514c..a8c4b5c 100644 --- a/src/mesa/state_tracker/st_manager.c +++ b/src/mesa/state_tracker/st_manager.c @@ -139,23 +139,64 @@ buffer_index_to_attachment(gl_buffer_index index) } /** + * Make sure a context picks up the latest cached state of the + * drawables it binds to. + */ +static void +st_context_validate(struct st_context *st, + struct st_framebuffer *stdraw, + struct st_framebuffer *stread) +{ + if (stdraw && stdraw->stamp != st->draw_stamp) { + st->dirty.st |= ST_NEW_FRAMEBUFFER; + _mesa_resize_framebuffer(st->ctx, &stdraw->Base, + stdraw->Base.Width, + stdraw->Base.Height); + st->draw_stamp = stdraw->stamp; + } + + if (stread && stread->stamp != st->read_stamp) { + if (stread != stdraw) { + st->dirty.st |= ST_NEW_FRAMEBUFFER; + _mesa_resize_framebuffer(st->ctx, &stread->Base, + stread->Base.Width, + stread->Base.Height); + } + st->read_stamp = stread->stamp; + } +} + +/** * Validate a framebuffer to make sure up-to-date pipe_textures are used. + * The context we need to pass in is s dummy context needed only to be + * able to get a pipe context to create pipe surfaces, and to have a + * context to call _mesa_resize_framebuffer(): + * (That should probably be rethought, since those surfaces become + * drawable state, not context state, and can be freed by another pipe + * context). */ static void -st_framebuffer_validate(struct st_framebuffer *stfb, struct st_context *st) +st_framebuffer_validate(struct st_framebuffer *stfb, + struct st_context *st) { - struct pipe_context *pipe = st->pipe; struct pipe_resource *textures[ST_ATTACHMENT_COUNT]; uint width, height; unsigned i; boolean changed = FALSE; + int32_t new_stamp = p_atomic_read(&stfb->iface->stamp); - if (!p_atomic_read(&stfb->revalidate)) + if (stfb->iface_stamp == new_stamp) return; /* validate the fb */ - if (!stfb->iface->validate(stfb->iface, stfb->statts, stfb->num_statts, textures)) - return; + do { + if (!stfb->iface->validate(stfb->iface, stfb->statts, + stfb->num_statts, textures)) + return; + + stfb->iface_stamp = new_stamp; + new_stamp = p_atomic_read(&stfb->iface->stamp); + } while(stfb->iface_stamp != new_stamp); width = stfb->Base.Width; height = stfb->Base.Height; @@ -184,7 +225,7 @@ st_framebuffer_validate(struct st_framebuffer *stfb, struct st_context *st) memset(&surf_tmpl, 0, sizeof(surf_tmpl)); u_surface_default_template(&surf_tmpl, textures[i], PIPE_BIND_RENDER_TARGET); - ps = pipe->create_surface(pipe, textures[i], &surf_tmpl); + ps = st->pipe->create_surface(st->pipe, textures[i], &surf_tmpl); if (ps) { pipe_surface_reference(&strb->surface, ps); pipe_resource_reference(&strb->texture, ps->texture); @@ -204,14 +245,9 @@ st_framebuffer_validate(struct st_framebuffer *stfb, struct st_context *st) } if (changed) { - st->dirty.st |= ST_NEW_FRAMEBUFFER; + ++stfb->stamp; _mesa_resize_framebuffer(st->ctx, &stfb->Base, width, height); - - assert(stfb->Base.Width == width); - assert(stfb->Base.Height == height); } - - p_atomic_set(&stfb->revalidate, FALSE); } /** @@ -236,8 +272,7 @@ st_framebuffer_update_attachments(struct st_framebuffer *stfb) st_visual_have_buffers(stfb->iface->visual, 1 << statt)) stfb->statts[stfb->num_statts++] = statt; } - - p_atomic_set(&stfb->revalidate, TRUE); + stfb->stamp++; } /** @@ -443,6 +478,7 @@ st_framebuffer_create(struct st_framebuffer_iface *stfbi) &stfb->Base._ColorReadBufferIndex); stfb->iface = stfbi; + stfb->iface_stamp = p_atomic_read(&stfbi->stamp) - 1; /* add the color buffer */ idx = stfb->Base._ColorDrawBufferIndexes[0]; @@ -454,6 +490,7 @@ st_framebuffer_create(struct st_framebuffer_iface *stfbi) st_framebuffer_add_renderbuffer(stfb, BUFFER_DEPTH); st_framebuffer_add_renderbuffer(stfb, BUFFER_ACCUM); + stfb->stamp = 0; st_framebuffer_update_attachments(stfb); stfb->Base.Initialized = GL_TRUE; @@ -473,31 +510,6 @@ st_framebuffer_reference(struct st_framebuffer **ptr, } static void -st_context_notify_invalid_framebuffer(struct st_context_iface *stctxi, - struct st_framebuffer_iface *stfbi) -{ - struct st_context *st = (struct st_context *) stctxi; - struct st_framebuffer *stfb; - - /* either draw or read winsys fb */ - stfb = st_ws_framebuffer(st->ctx->WinSysDrawBuffer); - if (!stfb || stfb->iface != stfbi) - stfb = st_ws_framebuffer(st->ctx->WinSysReadBuffer); - - if (stfb && stfb->iface == stfbi) { - p_atomic_set(&stfb->revalidate, TRUE); - } - else { - /* This function is probably getting called when we've detected a - * change in a window's size but the currently bound context is - * not bound to that window. - * If the st_framebuffer_iface structure had a pointer to the - * corresponding st_framebuffer we'd be able to handle this. - */ - } -} - -static void st_context_flush(struct st_context_iface *stctxi, unsigned flags, struct pipe_fence_handle **fence) { @@ -696,8 +708,6 @@ st_api_create_context(struct st_api *stapi, struct st_manager *smapi, smapi->get_param(smapi, ST_MANAGER_BROKEN_INVALIDATE); st->iface.destroy = st_context_destroy; - st->iface.notify_invalid_framebuffer = - st_context_notify_invalid_framebuffer; st->iface.flush = st_context_flush; st->iface.teximage = st_context_teximage; st->iface.copy = st_context_copy; @@ -762,10 +772,6 @@ st_api_make_current(struct st_api *stapi, struct st_context_iface *stctxi, } if (stdraw && stread) { - if (stctxi != st_api_get_current(stapi)) { - p_atomic_set(&stdraw->revalidate, TRUE); - p_atomic_set(&stread->revalidate, TRUE); - } st_framebuffer_validate(stdraw, st); if (stread != stdraw) st_framebuffer_validate(stread, st); @@ -781,6 +787,10 @@ st_api_make_current(struct st_api *stapi, struct st_context_iface *stctxi, } ret = _mesa_make_current(st->ctx, &stdraw->Base, &stread->Base); + + st->draw_stamp = stdraw->stamp - 1; + st->read_stamp = stread->stamp - 1; + st_context_validate(st, stdraw, stread); } else { struct gl_framebuffer *incomplete = _mesa_get_incomplete_framebuffer(); @@ -872,6 +882,8 @@ st_manager_validate_framebuffers(struct st_context *st) st_framebuffer_validate(stdraw, st); if (stread && stread != stdraw) st_framebuffer_validate(stread, st); + + st_context_validate(st, stdraw, stread); } /** -- 2.7.4