Keith Whitwell wrote:
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.
Yes. The other reason is you have to be careful with objects that
save context pointers when those objects might be shared among
multiple contexts.
If object A is created by context C1 and shared with context C2 and C1
gets destroyed, we're in trouble if we use A's stale context pointer.
It's safer to use the context pointer that's passed to the function.
I fixed a bug along those lines a couple months ago. See
st_DeleteTextureObject().
-Brian
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
