Hi
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.
>
> 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.
> +
> +/* 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.
> +
> +#define ST_MODULE_OPENGL "st_module_OpenGL"
> +#define ST_MODULE_OPENGL_ES1 "st_module_OpenGL_ES1"
> +#define ST_MODULE_OPENGL_ES2 "st_module_OpenGL_ES2"
> +#define ST_MODULE_OPENVG "st_module_OpenVG"
> +
> +/**
> + * Attachments of an sm surface.
> + *
> + * Intentionally include only color buffers to see if they suffice.
> + */
> +enum sm_attachment {
> + SM_ATTACHMENT_FRONT_LEFT,
> + SM_ATTACHMENT_BACK_LEFT,
> + SM_ATTACHMENT_FRONT_RIGHT,
> + SM_ATTACHMENT_BACK_RIGHT,
> +
> + NUM_SM_ATTACHMENTS
> +};
> +
> +/**
> + * Which buffer should be or is being rendered. Redundant?
> + */
> +enum sm_render_buffer {
> + SM_RENDER_BUFFER_FRONT,
> + SM_RENDER_BUFFER_BACK,
> +};
> +
> +/**
> + * The resources of the state tracker manager.
> + */
> +enum sm_resource_type {
> + SM_RESOURCE_EGL_DISPLAY, /* return struct pipe_screen */
> + SM_RESOURCE_EGL_CONTEXT, /* return struct pipe_context */
> + SM_RESOURCE_EGL_SURFACE, /* return struct sm_surface */
> + SM_RESOURCE_EGL_IMAGE, /* return struct pipe_texture */
> +};
> +
> +/**
> + * The resources of a state tracker.
> + */
> +enum st_resource_type {
> + /* all of them return struct pipe_texture */
> + ST_RESOURCE_OPENGL_TEXTURE_2D,
> + ST_RESOURCE_OPENGL_TEXTURE_3D,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_POSITIVE_X,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_NEGATIVE_X,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_POSITIVE_Y,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_NEGATIVE_Y,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_POSITIVE_Z,
> + ST_RESOURCE_OPENGL_TEXTURE_CUBE_MAP_NEGATIVE_Z,
> + ST_RESOURCE_OPENGL_RENDERBUFFER,
> + ST_RESOURCE_OPENVG_PARENT_IMAGE,
> +};
> +
> +/**
> + * The config of a surface.
> + */
> +struct sm_surface_config {
> + /**
> + * The format of the color buffers.
> + */
> + enum pipe_format color_format;
> +
> + /**
> + * The desired format of the depth/stencil buffer.
> + */
> + enum pipe_format depth_stencil_format;
> +
> + /**
> + * The desired render buffer.
> + */
> + enum sm_render_buffer render_buffer;
> +
> + uint paddings[32];
> +};
> +
> +/**
> + * An sm surface is owned by the state tracker manager. It
> provides buffers
> + * that are potentially visible to users.
> + */
> +struct sm_surface {
> + struct sm_surface_config config;
> +
> + /**
> + * The buffers of a surface may change at any time. A state
> tracker should
> + * validate the surface before using it. The returned buffers
> are owned by
> + * the caller.
> + *
> + * 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.
> + */
> + void (*validate)(struct sm_surface *smsurf, enum sm_attachment
> *smatts,
> + uint num_smatts, struct pipe_texture **textures);
> +
> + /**
> + * Changes to front buffers might not be immediately visible.
> Call this
> + * function to make sure they are. It might cause the
> update_surfaces of
> + * the bound context to be called.
> + */
> + void (*show)(struct sm_surface *smsurf);
> +
> + /**
> + * Private data for the state tracker manager.
> + */
> + void *sm_data;
> +};
> +
> +/**
> + * Describe an sm resource.
> + */
> +struct sm_resource {
> + enum sm_resource_type type;
> +
> + union {
> + void *p;
> + uint paddings[2];
> + } data;
> +};
> +
> +/**
> + * The API provided by the state tracker manager.
> + */
> +struct sm_api {
> + int major, minor;
> +
> + /**
> + * Lock and unlock an sm resource.
> + */
> + void *(*lock_resource)(const struct sm_api *smapi,
> + const struct sm_resource *res);
> + void (*unlock_resource)(const struct sm_api *smapi,
> + const struct sm_resource *res);
> +};
> +
> +/**
> + * The config of a context.
> + */
> +struct st_context_config {
> + const void *mode; /* __GLcontextModes, to be removed soon */
> +
> + int samples;
> + boolean have_accum;
> + boolean have_depth_stencil;
> + boolean double_buffer;
> + boolean stereo;
> +
> + uint paddings[32];
> +};
> +
> +/**
> + * An st context is owned by a state tracker.
> + */
> +struct st_context {
> + struct st_context_config config;
> +
> + /**
> + * Destroy the context and the pipe context.
> + */
> + void (*destroy)(struct st_context *stctx);
> +
> + /**
> + * Bind the context to sm surfaces.
> + */
> + boolean (*bind_surfaces)(struct st_context *stctx,
> + struct sm_surface *smdraw,
> + struct sm_surface *smread);
> +
> + /**
> + * 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 if validate as
you suggest is a lightweight operation this function is probably not
needed.
> +
> + /**
> + * 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.
> +
> + /**
> + * Flush and/or finish any queued command. If fence is not
> NULL, it must be
> + * waited.
> + */
> + void (*flush)(struct st_context *stctx, uint flags,
> + struct pipe_fence_handle **fence);
> + void (*wait)(struct st_context *stctx, struct pipe_fence_handle
> *fence);
> +
> + /**
> + * Replace the texture image of a texture object at the
> specified level.
> + * This function is optional.
> + */
> + boolean (*teximage)(struct st_context *stctx, int target_dim,
> int level,
> + enum pipe_format internal_format,
> + struct pipe_texture *ptex, boolean mipmap);
> +};
> +
> +/**
> + * Describe an st resource.
> + */
> +struct st_resource {
> + enum st_resource_type type;
> +
> + union {
> + uint u;
> + long l;
> + uint paddings[2];
> + } data;
> +};
> +
> +/**
> + * The API provided by a state tracker.
> + */
> +struct st_api {
> + const struct sm_api *smapi;
> +
> + /**
> + * Destroy the st API.
> + */
> + void (*destroy)(struct st_api *stapi);
> +
> + /**
> + * Lock and unlock an st resource.
> + */
> + void *(*lock_resource)(const struct st_api *stapi,
> + const struct st_resource *res);
> + void (*unlock_resource)(const struct st_api *stapi,
> + const struct st_resource *res);
> +
> + boolean (*is_config_supported)(const struct st_api *stapi,
> + const struct st_context_config
> *conf);
> +
> + /**
> + * Create a st context.
> + */
> + struct st_context *(*create_context)(const struct st_api *stapi,
> + struct pipe_context *pipe,
> + const struct
> st_context_config *conf,
> + struct st_context
> *shared_stctx);
> +
> + /**
> + * Make the given context the current context of the state
> tracker.
> + */
> + boolean (*set_current_context)(const struct st_api *stapi,
> + struct st_context *stctx);
> + struct st_context *(*get_current_context)(const struct st_api
> *stapi);
> +
> + /**
> + * Return the address of the named procedure.
> + */
> + void *(*get_proc_address)(const struct st_api *stapi, const char
> *proc);
> +};
> +
> +typedef struct st_api *(*st_create_api_proc)(const struct sm_api
> *smapi);
> +
> +struct st_module {
> + int major, minor;
> + st_create_api_proc create;
> +};
> +
> +extern PUBLIC const struct st_module st_module_OpenGL;
> +extern PUBLIC const struct st_module st_module_OpenGL_ES1;
> +extern PUBLIC const struct st_module st_module_OpenGL_ES2;
> +extern PUBLIC const struct st_module st_module_OpenVG;
> +
> +#endif /* _ST_API_H_ */
> --
> 1.6.5
------------------------------------------------------------------------------
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
