st/mesa: add a winsys buffers list in st_context

Commit a5e733c6b52e93de3000647d075f5ca2f55fcb71 fixes the dangling
framebuffer object by unreferencing the window system draw/read buffers
when context is released. However this can prematurely destroy the
resources associated with these window system buffers. The problem is
reproducible with Turbine Demo running with VMware driver. In this case,
the depth buffer content was lost when the context is rebound to a
drawable.

To prevent premature destroy of the resources associated with
window system buffers, this patch maintains a list of these buffers in
the context, making sure the reference counts of these buffers will not
reach zero until the associated framebuffer interface objects no
longer exist. This also helps to avoid unnecessary destruction and
re-construction of the resources associated with the framebuffer.

Fixes VMware bug 1909807.

Reviewed-by: Brian Paul <brianp@vmware.com>

diff --git a/src/gallium/include/state_tracker/st_api.h b/src/gallium/include/state_tracker/st_api.h
index d641092aa234617a1d01c395a6d91328503d76a9..3fd5f011f7db82d6f9fa1d2b17f209648282815c 100644 (file)
--- a/src/gallium/include/state_tracker/st_api.h
+++ b/src/gallium/include/state_tracker/st_api.h
@@ -310,6 +310,11 @@ struct st_framebuffer_iface
     */
    int32_t stamp;
 
+   /**
+    * Identifier that uniquely identifies the framebuffer interface object.
+    */
+   uint32_t ID;
+
    /**
     * Available for the state tracker manager to use.
     */

diff --git a/src/gallium/state_trackers/dri/dri_drawable.c b/src/gallium/state_trackers/dri/dri_drawable.c
index 3c2e3075249e07f0590153a7072a234451ec9641..0cfdc305583957ad52351f0efc6af55f29964e2a 100644 (file)
--- a/src/gallium/state_trackers/dri/dri_drawable.c
+++ b/src/gallium/state_trackers/dri/dri_drawable.c
@@ -38,6 +38,8 @@
 #include "util/u_memory.h"
 #include "util/u_inlines.h"
 
+static uint32_t drifb_ID = 0;
+
 static void
 swap_fences_unref(struct dri_drawable *draw);
 
@@ -155,6 +157,7 @@ dri_create_buffer(__DRIscreen * sPriv,
 
    dPriv->driverPrivate = (void *)drawable;
    p_atomic_set(&drawable->base.stamp, 1);
+   drawable->base.ID = p_atomic_inc_return(&drifb_ID);
 
    return GL_TRUE;
 fail:
@@ -177,6 +180,7 @@ dri_destroy_buffer(__DRIdrawable * dPriv)
 
    swap_fences_unref(drawable);
 
+   drawable->base.ID = 0;
    FREE(drawable);
 }
 

diff --git a/src/gallium/state_trackers/wgl/stw_st.c b/src/gallium/state_trackers/wgl/stw_st.c
index 7806a2a10e37319284b69251134ed02c8243fb0e..c2844b04c4ee59fdd69327b8ebb1a52d7cc97613 100644 (file)
--- a/src/gallium/state_trackers/wgl/stw_st.c
+++ b/src/gallium/state_trackers/wgl/stw_st.c
@@ -46,7 +46,7 @@ struct stw_st_framebuffer {
    unsigned texture_mask;
 };
 
-
+static uint32_t stwfb_ID = 0;
 
 /**
  * Is the given mutex held by the calling thread?
@@ -234,6 +234,7 @@ stw_st_create_framebuffer(struct stw_framebuffer *fb)
 
    stwfb->fb = fb;
    stwfb->stvis = fb->pfi->stvis;
+   stwfb->base.ID = p_atomic_inc_return(&stwfb_ID);
 
    stwfb->base.visual = &stwfb->stvis;
    p_atomic_set(&stwfb->base.stamp, 1);
@@ -255,6 +256,7 @@ stw_st_destroy_framebuffer_locked(struct st_framebuffer_iface *stfb)
    for (i = 0; i < ST_ATTACHMENT_COUNT; i++)
       pipe_resource_reference(&stwfb->textures[i], NULL);
 
+   stwfb->base.ID = 0;
    FREE(stwfb);
 }
 

diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
index f5351398474d892484224fe9c0d8bc91c2f8877e..0b353551da3ee4a0c792b7f2024dbbee46b79f2e 100644 (file)
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -38,6 +38,7 @@
 #include "program/prog_cache.h"
 #include "vbo/vbo.h"
 #include "glapi/glapi.h"
+#include "st_manager.h"
 #include "st_context.h"
 #include "st_debug.h"
 #include "st_cb_bitmap.h"
@@ -571,6 +572,9 @@ struct st_context *st_create_context(gl_api api, struct pipe_context *pipe,
       _mesa_destroy_context(ctx);
    }
 
+   /* Initialize context's winsys buffers list */
+   LIST_INITHEAD(&st->winsys_buffers);
+
    return st;
 }
 
@@ -591,6 +595,17 @@ destroy_tex_sampler_cb(GLuint id, void *data, void *userData)
 void st_destroy_context( struct st_context *st )
 {
    struct gl_context *ctx = st->ctx;
+   struct st_framebuffer *stfb, *next;
+
+   GET_CURRENT_CONTEXT(curctx);
+   if (curctx == NULL) {
+
+      /* No current context, but we need one to release
+       * renderbuffer surface when we release framebuffer.
+       * So temporarily bind the context.
+       */
+      _mesa_make_current(ctx, NULL, NULL);
+   }
 
    /* This must be called first so that glthread has a chance to finish */
    _mesa_glthread_destroy(ctx);
@@ -604,6 +619,11 @@ void st_destroy_context( struct st_context *st )
    st_reference_prog(st, &st->tep, NULL);
    st_reference_compprog(st, &st->cp, NULL);
 
+   /* release framebuffer in the winsys buffers list */
+   LIST_FOR_EACH_ENTRY_SAFE_REV(stfb, next, &st->winsys_buffers, head) {
+      st_framebuffer_reference(&stfb, NULL);
+   }
+
    pipe_sampler_view_reference(&st->pixel_xfer.pixelmap_sampler_view, NULL);
    pipe_resource_reference(&st->pixel_xfer.pixelmap_texture, NULL);
 

diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h
index af9149ebe41baa07faa3ae7ad0b35ae3a0ec48ae..43c8b0853bfd878e59b2c5c70663c391efed11e3 100644 (file)
--- a/src/mesa/state_tracker/st_context.h
+++ b/src/mesa/state_tracker/st_context.h
@@ -33,6 +33,7 @@
 #include "main/fbobject.h"
 #include "state_tracker/st_atom.h"
 #include "util/u_inlines.h"
+#include "util/list.h"
 
 
 #ifdef __cplusplus
@@ -277,6 +278,9 @@ struct st_context
     */
    struct st_bound_handles bound_texture_handles[PIPE_SHADER_TYPES];
    struct st_bound_handles bound_image_handles[PIPE_SHADER_TYPES];
+
+   /* Winsys buffers */
+   struct list_head winsys_buffers;
 };
 
 
@@ -301,6 +305,10 @@ struct st_framebuffer
    unsigned num_statts;
    int32_t stamp;
    int32_t iface_stamp;
+   uint32_t iface_ID;
+
+   /* list of framebuffer objects */
+   struct list_head head;
 };
 
 

diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
index 085f54efaa6756b99f365c6a3b499e87332324fb..de16a3a2cf000b6442e2a36e3983a9841758cd5b 100644 (file)
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -57,6 +57,7 @@
 #include "util/u_inlines.h"
 #include "util/u_atomic.h"
 #include "util/u_surface.h"
+#include "util/list.h"
 
 
 /**
@@ -459,6 +460,7 @@ st_framebuffer_create(struct st_context *st,
    _mesa_initialize_window_framebuffer(&stfb->Base, &mode);
 
    stfb->iface = stfbi;
+   stfb->iface_ID = stfbi->ID;
    stfb->iface_stamp = p_atomic_read(&stfbi->stamp) - 1;
 
    /* add the color buffer */
@@ -480,7 +482,7 @@ st_framebuffer_create(struct st_context *st,
 /**
  * Reference a framebuffer.
  */
-static void
+void
 st_framebuffer_reference(struct st_framebuffer **ptr,
                          struct st_framebuffer *stfb)
 {
@@ -488,6 +490,29 @@ st_framebuffer_reference(struct st_framebuffer **ptr,
    _mesa_reference_framebuffer((struct gl_framebuffer **) ptr, fb);
 }
 
+/**
+ * Purge the winsys buffers list to remove any references to
+ * non-existing framebuffer interface objects.
+ */
+static void
+st_framebuffers_purge(struct st_context *st)
+{
+   struct st_framebuffer *stfb, *next;
+
+   LIST_FOR_EACH_ENTRY_SAFE_REV(stfb, next, &st->winsys_buffers, head) {
+      /**
+       * If the corresponding framebuffer interface object no longer exists,
+       * remove the framebuffer object from the context's winsys buffers list,
+       * and unreference the framebuffer object, so its resources can be
+       * deleted.
+       */
+      if (stfb->iface->ID != stfb->iface_ID) {
+         LIST_DEL(&stfb->head);
+         st_framebuffer_reference(&stfb, NULL);
+      }
+   }
+}
+
 static void
 st_context_flush(struct st_context_iface *stctxi, unsigned flags,
                  struct pipe_fence_handle **fence)
@@ -761,17 +786,26 @@ st_framebuffer_reuse_or_create(struct st_context *st,
                                struct gl_framebuffer *fb,
                                struct st_framebuffer_iface *stfbi)
 {
-   struct st_framebuffer *cur = st_ws_framebuffer(fb), *stfb = NULL;
+   struct st_framebuffer *cur = NULL, *stfb = NULL;
 
-   /* dummy framebuffers cant be used as st_framebuffer */
-   if (cur && &cur->Base != _mesa_get_incomplete_framebuffer() &&
-       cur->iface == stfbi) {
-      /* reuse the current stfb */
-      st_framebuffer_reference(&stfb, cur);
+   /* Check if there is already a framebuffer object for the specified
+    * framebuffer interface in this context. If there is one, use it.
+    */
+   LIST_FOR_EACH_ENTRY(cur, &st->winsys_buffers, head) {
+      if (cur->iface_ID == stfbi->ID) {
+         st_framebuffer_reference(&stfb, cur);
+         break;
+      }
    }
-   else {
-      /* create a new one */
-      stfb = st_framebuffer_create(st, stfbi);
+
+   /* If there is not already a framebuffer object, create one */
+   if (stfb == NULL) {
+      cur = st_framebuffer_create(st, stfbi);
+
+      /* add to the context's winsys buffers list */
+      LIST_ADD(&cur->head, &st->winsys_buffers);
+
+      st_framebuffer_reference(&stfb, cur);
    }
 
    return stfb;
@@ -822,6 +856,11 @@ st_api_make_current(struct st_api *stapi, struct st_context_iface *stctxi,
 
       st_framebuffer_reference(&stdraw, NULL);
       st_framebuffer_reference(&stread, NULL);
+
+      /* Purge the context's winsys_buffers list in case any
+       * of the referenced drawables no longer exist.
+       */
+      st_framebuffers_purge(st);
    }
    else {
       ret = _mesa_make_current(NULL, NULL, NULL);

diff --git a/src/mesa/state_tracker/st_manager.h b/src/mesa/state_tracker/st_manager.h
index 65874b0040124dd719a883c634f25b2c7784dbde..68adf2fa1a49d3354dd49ca0af0fc858e1c956cb 100644 (file)
--- a/src/mesa/state_tracker/st_manager.h
+++ b/src/mesa/state_tracker/st_manager.h
@@ -33,6 +33,7 @@
 #include "pipe/p_compiler.h"
 
 struct st_context;
+struct st_framebuffer;
 
 void
 st_manager_flush_frontbuffer(struct st_context *st);
@@ -44,4 +45,7 @@ boolean
 st_manager_add_color_renderbuffer(struct st_context *st, struct gl_framebuffer *fb,
                                   gl_buffer_index idx);
 
+void
+st_framebuffer_reference(struct st_framebuffer **ptr,
+                         struct st_framebuffer *stfb);
 #endif /* ST_MANAGER_H */