[Mesa-dev] gallium st incorrect drawable <-> context bindings.

[Jose Fonseca]

----- Original Message -----
> On Tue, May 24, 2011 at 11:53 AM, Thomas Hellstrom
> <thellstrom at vmware.com> wrote:
> > Hi,
> >
> > I'm not sure when this use of drawable <-> context bindings
> > started, but the
> > below commit uses an invalid assumption to fix a problem the root
> > cause of
> > which is also an invalid assumption.
> >
> > It's incorrect to associate a drawable with a context.
> > A drawable may have multiple contexts bound to it, and attempts to
> > associate
> > a drawable with a single context will fail miserably in a
> > multithreading
> > environment.
> >
> > This means that the fix below should be reverted, and
> > drawable->driContextPriv should never have been used in the first
> > place. It
> > should nave no users whatsoever except old dri1 code using it as a
> > possible
> > swapbuffer context, but even that usage is undesired.
> >
> > Furthermore, the st interface that uses a specific context to
> > notify about
> > drawable invalidation is also incorrect. There should be no context
> > associated with that.
> 
> You are right, we need a list of contexts, and notify all of the
> contexts.

I think that a context lists is ugly (we really shouldn't reach out to any context other than the current context from the current thread) and inefficient (you'll need locking to traverse the context list).  Furthermore there is no need -- an atomic sequence number on st_framebuffer_iface gets the job done without all the complication.

After all, all we need to do is let context know that something happened -- we can't force them to take any action from the current thread.

Jose



diff --git a/src/gallium/include/state_tracker/st_api.h b/src/gallium/include/state_tracker/st_api.h
index 1c2148b..3ef1f45 100644
--- a/src/gallium/include/state_tracker/st_api.h
+++ b/src/gallium/include/state_tracker/st_api.h
@@ -267,6 +267,21 @@ struct st_framebuffer_iface
                           enum st_attachment_type statt);
 
    /**
+    * Get the framebuffer age.
+    *
+    * The rendering contexts call this function to determine whether they
+    * should update the textures they got from
+    * st_framebuffer_iface::validate.  It should do so at the latest time possible.
+    * Possible right before sending triangles to the pipe context.
+    *
+    * The implementor of this function needs to also ensure
+    * thread safety as this call might be done from multiple threads. This
+    * can however be implemented with an atomic counter.
+    */
+   uint32_t (*get_age)(struct st_context_iface *stctxi,
+                       struct st_framebuffer_iface *stfbi);
+
+   /**
     * The state tracker asks for the textures it needs.
     *
     * It should try to only ask for attachments that it currently renders
@@ -308,25 +323,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,