[Mesa-dev] RFC: ctx->Driver.Map/UnmapTextureImage() hooks

[Brian Paul]

On 07/22/2011 01:32 PM, Eric Anholt wrote:
> On Thu, 23 Jun 2011 19:08:51 -0600, Brian Paul<brianp at 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