[Mesa-dev] [PATCH] mesa/main: add ARB_clear_texture entrypoints

[Ilia Mirkin]

On Mon, Mar 10, 2014 at 5:44 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 03/04/2014 10:31 AM, Ilia Mirkin wrote:
>> On Tue, Mar 4, 2014 at 2:14 AM, Ian Romanick <idr at freedesktop.org> wrote:
>>> On 03/02/2014 12:09 AM, Ilia Mirkin wrote:
>>>> Thought I'd give this a shot. Am I on the right track here? Is the dd API
>>>> reasonable? I haven't written a piglit test for all the "generic" error
>>>> cases
>>>> yet, but I'll get to that in a bit. I'm a little confused between all the
>>>> different formats on a teximage... BaseFormat appears to be some
>>>> generalization of the format, but what's the difference between
>>>> InternalFormat
>>>> and TexFormat? Just the type, but otherwise equivalent? Or are they
>>>> representing different things?
>>>
>>>
>>> BaseFormat is the generic format.  So, GL_RGB or GL_RED are the base formats
>>> that go with GL_RGB8 or GL_RED16.  internalFormat is usually the GL format
>>> (e.g., GL_RGBA32F), and TexFormat is usually the Mesa format (e.g.,
>>> MESA_FORMAT_RGBA_FLOAT32).
>>
>> So TexFormat == InternalFormat, right? (Just different types)
>
> Maybe.  Applictions can still pass generic formats for internalFormat,
> and the driver can pick a different format.  The use could ask for
> GL_RGBA12, but the driver only supports 8-bit or 16-bit components.  In
> that case, internalFormat is still GL_RGBA12, but TexFormat would be
> MESA_FORMAT_RGBA_UNORM16.

Ah, that makes sense. Thanks for clearing it up!

>>>> +
>>>> +   GLenum (*QuerySupportForClearTex)(struct gl_context *ctx,
>>>> +                                     GLenum internalFormat);
>>>
>>>
>>> I think I'd rather add a generic query function. GL_ARB_query_internalformt2
>>> add a large number of target/internalFormat/pname queries.  I'm having
>>> difficulty deciding whether it's better to just pipe this interface directly
>>> into the driver or use something slightly more abstract.  Maybe add a
>>> function like
>>>
>>>     GLenum (*QueryFormatSupport)(struct gl_context *ctx,
>>>                                  GLenum internalFormat,
>>>                                  GLenum pname);
>>
>> Others have also pointed out to me that query2 isn't supported, so I'm
>> just going to drop it for now.
>
> There is an aspect of the spec that isn't clear to me... the query
> allows you to say that some formats are not supported.  You can imagine
> an implementation that uses something like meta to bind the texture to
> an FBO and glClearBuffer on it.  Some formats are not color-renderable
> (e.g., RGB16, shared exponent float, etc.), and these formats won't work
> with that implementation.
>
> What is supposed to happen in glClearTexImage for these unsupported
> format?  Generate an error?  Silently do nothing?
>
> Are you allowed to not support some formats if you don't support the
> query, or do you have to make everything "just work"?

Yes, the spec is unclear. I spoke about this briefly with Chris Forbes
on IRC. His take was that the query did not give you license to avoid
difficult/annoying cases. You'd be allowed to return NONE if there's
no way to get a TexImage with that internal format, but otherwise
you're stuck clearing it one way or another (Chris, please correct me
if I misunderstood). If I had to guess, I'd say that's why the spec
explicitly prohibits clearing compressed textures (vs just letting a
driver return NONE to the supported query if it didn't feel like
dealing with them). But of course, you're a lot closer to the
spec-writers than I am :)

>>>> +static void
>>>> +clear_tex_sub_image(struct gl_context *ctx, struct gl_texture_image
>>>> *texImage,
>>>> +                    GLint xoffset, GLint yoffset, GLint zoffset,
>>>> +                    GLsizei width, GLsizei height, GLsizei depth,
>>>> +                    GLenum format, GLenum type, const void *data)
>>>> +{
>>>> +   if (!texImage)
>>>> +      return;
>>>> +
>>>> +   if (_mesa_is_compressed_format(ctx, texImage->InternalFormat)) {
>>>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>>>> +                  "glClearTexSubImage(compressed format)");
>>>
>>>
>>> This will cause the wrong function name to get logged for glClearTexImage.
>>> Similar code in other parts of Mesa will pass a 'const char *caller' to the
>>> utility function.
>>
>> OK, didn't think it was important, but it's also really easy to fix,
>> so I'll do that. (When I first wrote the code, it was not going to be
>> easy to fix, but I since moved all the code into the helper as you see
>> it here.)
>
> It's not major, but it helps when you're trying to debug applications
> using MESA_DEBUG to log the error messages.

Ah, I see. Well, it's fixed up in the series I sent out a few days ago.

>>>> +   /* XXX figure out which dimensions the texImage has, and pass in 0's
>>>> for
>>>> +    * the border.
>>>> +    */
>>>> +   clear_tex_sub_image(ctx, texImage,
>>>> +                       -texImage->Border, -texImage->Border,
>>>> -texImage->Border,
>>>> +                       texImage->Width, texImage->Height,
>>>> texImage->Depth,
>>>> +                       format, type, data);
>>>
>>>
>>> I would just pass 0s.  Mesa doesn't support textures with borders.
>>
>> Should texImage->Border be removed? I figured it was easy to support
>> borders here, and the driver can deal with any issues. [I also haven't
>> the slightest was texture borders are, beyond the obvious -- borders
>> on textures. Why do they exist?]
>
> I think texImage->Border is there so that apps can query back what they
> requested... I'm not sure.  Brian may have any opion.

Well, you guys know what you're doing, I'm just a beginner here. Just
let me know how you want me to handle this, and I'll just do that.

Cheers,

  -ilia

>
> Texture borders are a 1-pixel border outside the actual texture.  The
> primary use was for stitching together larger textures with seamless
> filtering across edges.  This was mostly interesting when hardware only
> supported 256x256 textures.  There were similar uses for seamless
> filtering across cubemap faces.  Large textures, seamless cubemap
> filtering, and anisotropic filtering (that requires more than one extra
> pixel) have really made texture borders useless.