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

[Stéphane Marchesin]

On Tue, Sep 4, 2012 at 4:01 PM, Brian Paul <brianp at vmware.com> wrote:
> 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.
>

Sounds good, I'll do that instead.

Stéphane