On Fri, 2009-11-27 at 10:10 -0800, michal wrote:
> Hello,
> 
> Please review the patch below. It extends the gallium interface to allow 
> setting vertex texture sampler states.
> 
> This is an optional feature -- drivers not wishing to implement it 
> return 0 for PIPE_CAP_MAX_VERTEX_TEXTURES capability query. Drivers may 
> also choose to support it, but always fallback to software 
> implementation from the draw auxiliary module.
> 

Michal,

couple of comments inline.


> diff --git a/src/gallium/include/pipe/p_context.h 
> b/src/gallium/include/pipe/p_context.h
> index 5569001..70f9c8b 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -124,6 +124,9 @@ struct pipe_context {
>     void * (*create_sampler_state)(struct pipe_context *,
>                                    const struct pipe_sampler_state *);
>     void   (*bind_sampler_states)(struct pipe_context *, unsigned num, 
> void **);
> +   void   (*bind_vertex_sampler_states)(struct pipe_context *,
> +                                        unsigned num_samplers,
> +                                        void **samplers);
>     void   (*delete_sampler_state)(struct pipe_context *, void *);
>  
>     void * (*create_rasterizer_state)(struct pipe_context *,
> @@ -184,6 +187,10 @@ struct pipe_context {
>     void (*set_vertex_elements)( struct pipe_context *,
>                                  unsigned num_elements,
>                                  const struct pipe_vertex_element * );
> +
> +   void (*set_vertex_sampler_textures)(struct pipe_context *,
> +                                       unsigned num_textures,
> +                                       struct pipe_texture **);
>     /*...@}*/


If we're adding these functions, can the old ones be renamed to
fragment_sampler_states/textures for clarity?

> 
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index fd14dc8..eac6904 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -390,6 +390,7 @@ enum pipe_transfer_usage {
>  #define PIPE_CAP_BLEND_EQUATION_SEPARATE 28
>  #define PIPE_CAP_SM3                     29  /*< Shader Model 3 
> supported */
>  #define PIPE_CAP_MAX_PREDICATE_REGISTERS 30
> +#define PIPE_CAP_MAX_VERTEX_TEXTURES     31
>  
> 
>  /**
> diff --git a/src/gallium/include/pipe/p_state.h 
> b/src/gallium/include/pipe/p_state.h
> index 287b424..ce22f89 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -60,6 +60,7 @@ extern "C" {
>  #define PIPE_MAX_COLOR_BUFS        8
>  #define PIPE_MAX_CONSTANT         32
>  #define PIPE_MAX_SAMPLERS         16
> +#define PIPE_MAX_VERTEX_SAMPLERS   4
>  #define PIPE_MAX_SHADER_INPUTS    16
>  #define PIPE_MAX_SHADER_OUTPUTS   16
>  #define PIPE_MAX_TEXTURE_LEVELS   16


Why is the MAX here smaller than for fragment samplers?  Doesn't GL
require them to be the same, because GL effectively binds the same set
of sampler states in both cases?  

Can you take a closer look at what the GL state tracker would have to do
to expose this functionality and make sure it's valid?

Keith


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to