On Thu, 2010-03-11 at 06:21 -0800, michal wrote:
> José Fonseca wrote on 2010-03-11 14:40:
> > 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.
> >
> >
> How about all component sizes and order of components must match, and
> the only difference can be in value type, that is one can convert
> between SNORM, UNORM, SRGB, FLOAT, etc., as long as the base format is
> the same (e.g. PIPE_FORMAT_R8G8B8A8)?
Yes, looking again, this does seem consistent with DX10's
DXGI_FORMAT_xxxxxx_TYPELESS formats.
What about GL's PBOs? I don't know much about them, but I heard that is
possible to use surfaces with a different format from what was original
created. Do you or anybody know how pipe sampler view could be used in
that case?
> > + 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.
> >
> >
> Had this object been created by pipe_screen, it would have a reference
> to a screen that created it. Sampler view is a first "resource" type
> that is created by pipe_context.
>
> We want to migrate other types to
> pipe_context as well. I suppose once pipe_texture has a reference to
> pipe_context, we could remove it from here, but in the meantime we will
> need it in e.g. pipe_sampler_view_reference().
If it makes life easier then sounds good.
> > + 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.
> >
> Makes sense, I will change it.
>
> > + 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.
> >
> >
> Will fix it, thanks.
Thanks!
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
