[Mesa-dev] RFC: array textures in gallium and assorted cleanups

[Keith Whitwell]

On Thu, 2010-06-10 at 07:08 -0700, Roland Scheidegger wrote:
> On 10.06.2010 11:30, Keith Whitwell wrote:
> > On Thu, 2010-06-03 at 13:26 -0700, Roland Scheidegger wrote:
> >> Hi,
> >>
> >> I've created a new branch gallium-array-textures which has some more
> >> interface changes, this time to support array textures basically.
> >> Nothing has been adapted to these changes yet (I'll do that it should be
> >> mostly trivial as long as array textures aren't actually supported by
> >> the driver or even mesa state tracker), but now would be a good time if
> >> you have some comments for the proposed interface changes.
> >>
> >> Roland
> > 
> > Roland,
> > 
> > This looks great!
> > 
> > Couple of comments -- you're now using the term "layer" in various
> > places, but there is no strong definition of what that means - except in
> > the patch comment, which isn't useful once the patch is committed.  Can
> > you define this term somewhere in the documentation?
> Ok will do.
> 
> > 
> > Also, there are a couple of things that look like typos in the interface
> > change diff, but I'm sure you'll find those the first time you try to
> > compile this.  eg:
> > 
> >     void (*resource_copy_region)(struct pipe_context *pipe,
> >                                  struct pipe_resource *dst,
> > -                                struct pipe_subresource subdst,
> > +                                unsigned level,
> >                                  unsigned dstx, unsigned dsty, unsigned dstz,
> >                                  struct pipe_resource *src,
> > -                                struct pipe_subresource subsrc,
> > -                                unsigned srcx, unsigned srcy, unsigned srcz,
> > -                                unsigned width, unsigned height);
> > +                                unsigned level,
> > +                                const struct pipe_box *);
> >  
> > It seems like you end up with two parameters named "level" ??
> Yes, I had already fixed this locally.
> create_surface also had a bug (still got passed pipe_screen instead of
> pipe_context since it moved to context), as well as I need to store the
> context itself in pipe_surface (much like pipe_sampler_view does).
> That actually was a bit non-trivial since some state trackers don't
> really have a context handy when they called the former
> get_tex_surface() (glx, wgl and so on statetrackers not the rendering
> ones). Some of them did, though, already have their own context (for
> resource_copy_region, for instance) so I'm about to do this in a similar
> fashion.
> Actually, I was wondering if surface_destroy() should also get passed in
> a context - seems strange since it already stores the context, but this
> is exactly what sampler_view_destroy() does, which I'd like to see as a
> very analogous function.

Yes, it should take a context, mainly for consistency.  It helps when
wrapping/unwrapping these functions to have a consistent interface.  

Keith