[Mesa-dev] [PATCH 1/3] mesa: implement GL_ARB_texture_buffer_range

[Christoph Bumiller]

On 16.12.2012 18:10, Brian Paul wrote:
> On Sun, Dec 16, 2012 at 9:50 AM, Christoph Bumiller
> <e0425955 at student.tuwien.ac.at> wrote:
>> ---
>>  src/mapi/glapi/gen/ARB_texture_buffer_range.xml |   22 ++++++
>>  src/mapi/glapi/gen/Makefile.am                  |    1 +
>>  src/mapi/glapi/gen/gl_API.xml                   |    2 +
>>  src/mesa/main/context.c                         |    1 +
>>  src/mesa/main/extensions.c                      |    1 +
>>  src/mesa/main/get.c                             |    1 +
>>  src/mesa/main/get_hash_params.py                |    3 +
>>  src/mesa/main/mtypes.h                          |    6 ++
>>  src/mesa/main/teximage.c                        |   91 ++++++++++++++++++-----
>>  src/mesa/main/teximage.h                        |    4 +
>>  src/mesa/main/texparam.c                        |    8 ++
>>  11 files changed, 121 insertions(+), 19 deletions(-)
>>  create mode 100644 src/mapi/glapi/gen/ARB_texture_buffer_range.xml
>> +   bufObj = _mesa_lookup_bufferobj(ctx, buffer);
>> +   if (bufObj) {
>> +      if (offset < 0 ||
>> +          size <= 0 ||
>> +          (offset + size) > bufObj->Size) {
>> +         _mesa_error(ctx, GL_INVALID_VALUE, "glTexBufferRange");
>> +         return;
>> +      }
>> +      if (offset % ctx->Const.TextureBufferOffsetAlignment) {
>> +         _mesa_error(ctx, GL_INVALID_VALUE, "glTexBufferRange(offset)");
> Maybe say something like "invalid offset alignment" to give more info.
>
> Looks OK to me otherwise.
>
> Reviewed-by: Brian Paul <brianp at vmware.com>

Thanks for the review.
However, I just noticed another issue:

The TBO piglit test expects that the size of the texture be updated when
BufferData is called after TexBuffer.
With this patchset, gl_texture_object.BufferSize would not be updated on
BufferData, so a potential change in size (like, if the data store had
not even been established before) would never be propagated.

I'm not really sure how the spec intends this to work.
For one, glTexBufferRange should fail with INVALID_VALUE if the supplied
size is < the size of the buffer object. It also says that TexBuffer
behaves just like TexBufferRange with size = size of the buffer object.
So, calling TexBuffer without ever having allocated storage really
should fail according to this ...

A quick workaround would be to just set BufferSize == -1 in case of
TexBuffer (non-Range) and have that signal the drivers to just use
whatever the current size of the buffer is.

The only remaining issue is what to do if the new buffer size makes the
previously established offset and/or size from TexBufferRange invalid ...