On 16 jan 2010, at 09.47, Chia-I Wu wrote:
> 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?

Thanks! Go ahead and start a repo, I'll create a repo at fdo/ 
~wallbraker/st_api.git if I want to push something out.

>
>>> 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?

Both yes and no. The Linux GL ABI specifies exactly what libGL can  
contain (glx implementation + gl dispatch table) the rest needs to be  
inside the DRI driver (mesa and st). There is no such standard for  
libEGL.so/libOpenVG.so so we currently have the state trackers in  
those and I'm fine with leaving it that way for the time being. The  
disadvantage is that you need to upgrade libOpenVG along side you  
driver if you want it to work (New dri drivers work perfectly fine  
with old libGL's how ever). One way to solve that would be to put the  
all the state trackers in the driver binaries, but that gives very  
large binaries. So something needs to be sorted out, but its beyond  
the scope of this discussion.

>
> 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.

That be really great if you could do that, at least for st/mesa, there  
isn't much need for that on for example OpenVG.

>>> +    * 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.

Yeah you are correct.

>
> 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.

Sounds good, is this for keeping the contents of the backbuffer alive?  
There are several points when the specs allow the contents to be come  
undefined using that could allow us to save on memory.

>>> +   /**
>>> +    * 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.

Right...

>>> +
>>> +   /**
>>> +    * 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.

Thanks.

Cheers Jakob.

------------------------------------------------------------------------------
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