[Mesa-dev] [PATCH 1/3] mesa: move gl_texture_image::Data, RowStride, ImageOffsets to swrast

[Brian Paul]

On 10/12/2011 06:21 PM, Eric Anholt wrote:
> On Wed, 12 Oct 2011 11:44:10 -0600, Brian Paul<brianp at vmware.com>  wrote:
>> On 10/12/2011 10:57 AM, Eric Anholt wrote:
>>> On Mon, 10 Oct 2011 20:27:44 -0600, Brian Paul<brian.e.paul at gmail.com>   wrote:
>>>> From: Brian Paul<brianp at vmware.com>
>>>>
>>>> Only swrast and the drivers that fall back to swrast need these fields now.
>>>> This removes the last of the fields related to software rendering from
>>>> gl_texture_image.
>>>
>>>> +   /* Allocate the swrast_texture_image::ImageOffsets array now */
>>>> +   switch (texobj->Target) {
>>>> +   case GL_TEXTURE_3D:
>>>> +   case GL_TEXTURE_2D_ARRAY:
>>>> +      slices = image->Depth;
>>>> +      break;
>>>> +   case GL_TEXTURE_1D_ARRAY:
>>>> +      slices = image->Height;
>>>> +      break;
>>>> +   default:
>>>> +      slices = 1;
>>>> +   }
>>>> +   assert(!intel_image->base.ImageOffsets);
>>>> +   intel_image->base.ImageOffsets = malloc(slices * sizeof(GLuint));
>>>
>>> This (and the corresponding code in the other drivers) should live in
>>> swrast with the IsPowerOfTwo and *Scale setup I think.
>>
>> I'd like to do that in a follow-on commit.
>>
>> There's actually an issue with the swrast FetchTexel code that should
>> probably be addressed first.
>>
>> The 2D FetchTexel function is used for both 2D and 1D_ARRAY targets,
>> but ImageOffsets[] is not used there.  The 1D array slice/coord is
>> passed as the second texcoord component so the texel address is
>> computed using the RowStride.  So while the 1D_ARRAY case in the code
>> above is what we need for glTex[Sub]Image, it's irrelevant and ignored
>> during swrast sampling of a 1D array texture.  It's probably always
>> been broken (I doubt i965 user has ever hit that sw fallback path).
>>
>> The simplest fix is to use the FetchTexel3D function when sampling
>> 1D_ARRAYs, pass the slice as the 3rd coordinate so we use
>> ImageOffsets[] and finally, have the swrast driver setup the
>> ImageOffsets[] array as we're doing in the intel driver.  Then we
>> could merge some code.
>>
>> I'd like to commit the patch series as-is and then refine things bit
>> by bit.  I tested these patches on i965 and swrast and there were no
>> piglit texture/fbo regressions.
>
> Works for me.
>
>> BTW, under what circumstances will the i965 driver revert to swrast
>> rendering nowadays?  The only FALLBACK() instance I see in the driver
>> (just grepping) is for selection/feedback mode.
>
> The FALLBACK() macros is sort of a red herring from i915.  The real
> fallback code is brw_fallback.c, which triggers for: rendermode,
> no_rast, and texture border.
>
> I want to just ditch texture borders like you do in gallium --
> successfully rendering them through swrast will be worse than not
> rendering them.

I don't think anyone has ever complained about the lack of texture 
borders in gallium.  There's only a handful of piglit/conform tests 
that use borders.  The state tracker code that "removes" borders by 
tweaking the unpack parameters could easily be lifted into core Mesa. 
  A ctx->Const.TextureBorder flag could be used to enable/disable it 
so drivers wouldn't have to do anything special.


> That leaves feedback and select.  For SELECT we just need to slightly
> generalize the meta code that exists for it already, I think.  For
> feedback, it's a bit trickier because that's supposed to happen after
> clipping.  I wonder if giving unclipped results generated by
> transform_feedback wouldn't be good enough, though?

Years ago, the one use of feedback mode that I remember seeing was for 
producing vector drawings.  The idea was to capture the transformed 
verts in a feedback buffer then use the coords to generate postscript 
code for printing, etc.  I'd expect that clipping is important for 
that.  But who knows if anyone's doing this anymore.


> no_rast is just used for testing swrast for me, since the real swrast
> driver gives bad alpha values and just fails all of piglit.
>
> We're not planning on actually supporting general rendering through
> swrast when we get GL3.0 going, currently -- where we end up rendering
> in swrast, we should fix that to go through hardware.
>

-Brian