On 07/22/2011 01:32 PM, Eric Anholt wrote:
On Thu, 23 Jun 2011 19:08:51 -0600, Brian Paul<bri...@vmware.com> wrote:
I'd like to overhaul the part of Mesa related to texture memory
reading/writing.
OK, I'm taking a look at map-texture-image-v4. I like what I'm seeing
overall, I just want to be sure that this isn't something that gets
squash-merged. There's going to be breakage, and I want to be able to
bisect into it.
In the metaops code, please use glBufferData instead of
glBufferSubData. If you BufferSubData, I have to block on the GPU if it
was using that buffer already.
It looks like we'd have to change that in several other places too.
Can we do that change later?
In the comments for void (*MapTextureImage), please note what the units
of rowStride are. I see that's present in swrast later, but I think the
mtypes.h and dd.h files are used for reference a lot (I do, certainly).
Will do. The parameter comments in s_texture.c are out of date too.
c029312ad62039904818a8b1765c6bcdf50044df is huge, and it doesn't even
build. Ouch. I think there's some room for splitting some of this up
so that we can get a nice series.
Where's the build breakage? I don't remember that.
This was originally a long series of sometimes ugly WIP patches. At
one point I had a git mishap and trashed some of the intermediate
patches. I agree that splitting up this commit would be good, but it
would be a lot of work that I don't really have time for.
It would be great if you could do a full piglit run with the branch
and check for i965/i915 regressions. I'm not aware of any with swrast
or gallium. I'd help diagnose any regressions.
I'd be happy to incorporate any fixes into a new map-texture-image-v5
branch before merging to master.
2025416f5090ee776a5c3291201f003e7c090718 -- if we're going to have
asserts that the array got initialized in the right order, I think I'd
rather see the array initialized by hand in an init function so that we
don't even need them (assuming tha the reason for this is that you're
still supporting compilers that don't do array[] = { [ELEMENT] = value,
}).
I could do that. But I think there's a few other places where we do
this. Inserting a new MESA_FORMAT_x in the middle of the enum list
would definitely cause breakage.
Thanks for reviewing.
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
