On Sat, Jan 16, 2010 at 05:49:44AM +0000, Jakob Bornecrantz wrote:
> Here comes a quick review, lot of the things in here are out of date
> due to discussions over email so I tried to only comment on bits.
> A general note is that you seem to have removed a lot of comments.
> Could you please base the a newer version on the st_api.h I sent
> out.
I am willing to rebase on your version.  I think we should maintain
st_api.h and a DESIGN document on a git repo on
fdo/~<username>/st_api.git.  It will give us something we can base our
discussion on.  What do you think?  Do you want to do it or should I?

> >diff --git a/src/gallium/include/state_tracker/st_api.h b/src/
> >gallium/include/state_tracker/st_api.h
> >new file mode 100644
> >index 0000000..408bedb
> >--- /dev/null
> >+++ b/src/gallium/include/state_tracker/st_api.h
> >@@ -0,0 +1,278 @@
> >+#ifndef _ST_API_H_
> >+#define _ST_API_H_
> >+#include "pipe/p_compiler.h"
> >+#include "pipe/p_screen.h"
> >+#include "pipe/p_context.h"
> >+#include "pipe/p_state.h"
> These shouldn't be needed, just forward declare the needed structs.
Ok.
> >+
> >+/* XXX this is WIP! */
> >+#define SM_VERSION_MAJOR 0
> >+#define SM_VERSION_MINOR 0
> >+#define ST_VERSION_MAJOR 0
> >+#define ST_VERSION_MINOR 0
> This is an internal Gallium API. As long as the Gallium API isn't
> stable there is no need to make this API stable so for now please
> remove any versioning from the API.
I think the state trackers (libGL.so, libOpenVG.so, ...) are distributed
separately from state tracker managers (libEGL.so).  Shouldn't we need
them?

By the way, a state tracker is able to implement both st_public.h and
st_api.h at the same time.  It gives a mild migration path.
> >+    * Note that if this funcion is called multiple times with
> >different sets of
> >+    * attachments, those not listed in the latest call might be
> >destroyed.
> >+    * This behavior might change in the future.
> The pipe textures are referenced counted and as long as the
> rendering API has references to them they will stay around and it
> must be possible to render to them. However they might be the wrong
> buffers to render to. That is invalidated.
The comment describes the behavior of DRI2GetBuffers.  Even the texture
are reference counted by the state tracker, becase DRI2CopyRegion is
called when flushing front buffer or swapping buffers, it causes
BadDrawable X errors if the state tracker is not cautious.

But, a big BUT, I want to change the semantics of "validate" and remove
the comment.  That is, one can ask front & back buffers, and then ask
only the front buffer.  The back buffer won't not be destroyed.
> >+   /**
> >+    * Ask the st context to re-validate the binding surfaces.
> >+    */
> >+   void (*update_surfaces)(struct st_context *stctx);
> Can you please use the name as given my version, it really is a
> notify event since on certain platforms this call might originate
> from another thread then the one that the context is current in and
> as such needs to be thread safe. And by having it just as a
> notification we don't need any locking in the rendering api. However
Ok.
> if validate as you suggest is a lightweight operation this function
> is probably not needed.
validate will be cheap on X only after there is a notification mechanism
allowing the state tracker manager to cache the textures and avoid the
round trip.
> >+
> >+   /**
> >+    * Return the render buffer of the context.
> >+    */
> >+   enum sm_render_buffer (*get_render_buffer)(struct st_context
> >*stctx);
> For now lets wait with introducing this function. There must be ways
> around it.
Yeah, I will leave it out.

-- 
Regards,
olv

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to