On Thu, 2010-03-11 at 03:16 -0800, michal wrote:
> Hi,
>
> I would like to merge the branch in subject this week. This feature
> branch allows state trackers to bind sampler views instead of textures
> to shader stages.
>
> A sampler view object holds a reference to a texture and also overrides
> internal texture format (resource casting) and specifies RGBA swizzle
> (needed for GL_EXT_texture_swizzle extension).
>
> Yesterday I had to merge master into gallium-sampler-view -- the nv50
> and r300 drivers had lots of conflicts. Could the maintainers of said
> drivers had a look at them and see if they are still functional, please?
>
> Thanks.
Michal,
Looks good overall. Minor comments below. Sorry if I rehash things that
have been already discussed. Feel free to ignore them.
diff --git a/src/gallium/include/pipe/p_state.h
b/src/gallium/include/pipe/p_state.h
index 3a97d88..3c7c0a5 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -300,6 +300,24 @@ struct pipe_surface
/**
+ * A view into a texture that can be bound to a shader stage.
+ */
+struct pipe_sampler_view
+{
+ struct pipe_reference reference;
+ enum pipe_format format; /**< typed PIPE_FORMAT_x */
I think it is worth to document that not all formats are valid here.
pipe_sampler_view::format and pipe_texture::format must be compatible.
The semantics of compatibility are a bit confusing though. Even DX10's.
At very least format's util_format_block must match.
RGB <=> SRGB variations should also be accepted. And sizzwle variations.
The difficulty is whether formats like A4R4G4B4 <=> A1G5R5B5 or
R8G8B8A8 <=> R32 should be accepted. I think hardware should have no
troubles with typecasting. So I'm inclined towards make this acceptible.
+ struct pipe_texture *texture; /**< texture into which this is a view */
+ struct pipe_context *context; /**< context this view belongs to */
Is this for debugging? No other state object has a pointer to context so far.
+ unsigned first_level:8; /**< first mipmap level */
+ unsigned num_levels:8; /**< number of mipamp levels */
I don't care much, but we've been using last_level instead of num_levels
elsewhere. Is there a particular reason to use num_levels here?
s/mipamp/mipmap/ in comment.
+ unsigned swizzle_r:3; /**< PIPE_SWIZZLE_x for red component */
+ unsigned swizzle_g:3; /**< PIPE_SWIZZLE_x for green component */
+ unsigned swizzle_b:3; /**< PIPE_SWIZZLE_x for blue component */
+ unsigned swizzle_a:3; /**< PIPE_SWIZZLE_x for alpha component */
+};
+
+
+/**
* Transfer object. For data transfer to/from a texture.
*/
struct pipe_transfer
diff --git a/src/gallium/drivers/llvmpipe/lp_state_sampler.c
b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
index b30a075..2df86a0 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_sampler.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
[...]
> +struct pipe_sampler_view *
> +llvmpipe_create_sampler_view(struct pipe_context *pipe,
> + struct pipe_texture *texture,
> + const struct pipe_sampler_view *templ)
> +{
> + struct pipe_sampler_view *view = CALLOC_STRUCT(pipe_sampler_view);
Need to handle out of memory here.
> + *view = *templ;
> + view->reference.count = 1;
> + view->texture = NULL;
> + pipe_texture_reference(&view->texture, texture);
> + view->context = pipe;
> +
> + return view;
> +}
The rest looks great to me. It's a very useful gallium interface change.
I particularly like how you eased migration with
cso_set_sampler_textures().
BTW, I noticed you commented out pipe video code. Everybody agreed on
removing it from master and mature pipe video on a branch, but we never
got around to do it. I'll remove this code in the next few days.
Jose
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
