[Mesa-dev] [PATCH 1/2] intel: Prevent huge lastlevel values from being computed.

[Brian Paul]

On 09/04/2012 04:39 PM, Ian Romanick wrote:
> On 09/04/2012 03:32 PM, Stéphane Marchesin wrote:
>> On Tue, Sep 4, 2012 at 3:27 PM, Ian Romanick <idr at freedesktop.org>
>> wrote:
>>> On 09/04/2012 03:19 PM, Stéphane Marchesin wrote:
>>>>
>>>> On Tue, Sep 4, 2012 at 2:59 PM, Eric Anholt <eric at anholt.net> wrote:
>>>>>
>>>>> Stéphane Marchesin <marcheu at chromium.org> writes:
>>>>>
>>>>>> The current computation for the lastlevel is based on the level
>>>>>> size and
>>>>>> can
>>>>>> lead to writing past the end of the texture array. Instead we
>>>>>> clamp it
>>>>>> by
>>>>>> MAX_TEXTURE_LEVELS.
>>>>>> ---
>>>>>> src/mesa/drivers/dri/intel/intel_tex_image.c | 5 +++++
>>>>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c
>>>>>> b/src/mesa/drivers/dri/intel/intel_tex_image.c
>>>>>> index fe9040c..7ef258b 100644
>>>>>> --- a/src/mesa/drivers/dri/intel/intel_tex_image.c
>>>>>> +++ b/src/mesa/drivers/dri/intel/intel_tex_image.c
>>>>>> @@ -88,6 +88,11 @@ intel_miptree_create_for_teximage(struct
>>>>>> intel_context *intel,
>>>>>> lastLevel = firstLevel;
>>>>>> } else {
>>>>>> lastLevel = firstLevel + _mesa_logbase2(MAX2(MAX2(width,
>>>>>> height), depth));
>>>>>> + /* We tried to guess the last level based on the texture size,
>>>>>> make
>>>>>> + * sure we don't go past MAX_TEXTURE_LEVELS since it's hardcoded
>>>>>> + * in many places.
>>>>>> + */
>>>>>> + lastLevel = MIN2(lastLevel, MAX_TEXTURE_LEVELS - 1);
>>>>>> }
>>>>>> }
>>>>>
>>>>>
>>>>> I'm confused. MAX_TEXTURE_LEVELS should set such that it covers
>>>>> something from the texture size limits down to 1x1. Does it not?
>>>>
>>>>
>>>> Let's say the app calls glTexImage for level 6, and width = 2048.
>>>> That
>>>
>>>
>>> Is that valid? Since the base level of this texture would be huge,
>>> it seems
>>> like glTexImage should reject it. I'm not sure there's any language
>>> in the
>>> spec either way, but that's my gut feeling.
>>
>> I didn't see anything in the spec. I also wonder how that interacts
>> with something like a subsequent glCopyTexImage from that level of an
>> incomplete texture to somewhere else.
>>
>> I'd be fine with rejecting it, if that's the right thing to do.
>
> I found it! There's similar language for 3D and cube textures also.
> Page 149 (page 164 of the PDF) of the OpenGL 3.3 core profile spec says:
>
> "In a similar fashion, the maximum allowable width of a texel
> array for a one- or two-dimensional, one- or two-dimensional
> array, two-dimensional multisample, or two-dimensional
> multisample array texture, and the maximum allowable height
> of a two-dimensional, two-dimensional array, two-dimensional
> multisample, or two-dimensional multisample array texture,
> must be at least 2k−lod + 2bt for image arrays of level 0
> through k, where k is the log base 2 of MAX_TEXTURE_SIZE."

This is in the 2.1 spec too.

I just whipped up a piglit test to check this.  NVIDIA's driver raises 
an error in this case, but Mesa doesn't.  I'll post the test to the 
piglist list.

Adding the check to Mesa should be simple.

-Brian