[Mesa-stable] [Mesa-dev] [PATCH] st/mesa: use first image's dimensions when finalizing texture

[Emil Velikov]

On 6 June 2016 at 23:35, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Jun 6, 2016 at 6:32 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> On 06.06.2016 23:58, Ilia Mirkin wrote:
>>>
>>> On Mon, Jun 6, 2016 at 5:37 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>>>
>>>> On 05.06.2016 08:24, Ilia Mirkin wrote:
>>>>>
>>>>>
>>>>> In the case where we can't guess the base level size, just use the first
>>>>> image's dims. The width0/height0/depth0 on stObj may not have been set
>>>>> at this point. Observed in a trace that set up levels 2..9 of a 2d
>>>>> texture,
>>>>> and set the base level to 2, with height 1. This made the guess logic
>>>>> always bail.
>>>>
>>>>
>>>>
>>>> So by coincidence, I've been digging around this code today as well,
>>>> though
>>>> I think the issues we were looking at are unrelated.
>>>>
>>>> I'm a bit confused. As far as I understand the code, the allocated pipe
>>>> texture always has levels corresponding to the GL texture (ignoring
>>>> texture
>>>> views, at least...). So in the described situation, BaseLevel == 2,
>>>> stObj->lastLevel == 9, and we allocate a 10-level texture object.
>>>>
>>>> The pipe texture dimensions should be the dimensions of a hypothetical
>>>> level
>>>> 0, but with your change they will end up being the dimension of the GL
>>>> level
>>>> 2, leading to a texture that is too small? How does this work?
>>>>
>>>> I agree though that the current code can't be correct either...
>>>
>>>
>>> Well, since it's *hypothetical*, I was kinda hoping everything would
>>> work out starting from firstImage and ignore the hypothetical higher
>>> levels. However perhaps that hope isn't rooted in reality, and is the
>>> reason why the test fails.
>>
>>
>> I mean, aren't the level sizes wrong?
>>
>> E.g. the program sets GL texture level 2 to 128x1, level 3 to 64x1, etc. Now
>> the pipe texture width0 should be 512 and the height0 should be 1, but will
>> end up being 128 and 1, respectively.
>
> I had hoped there'd be something that made firstImage be level 0. I
> guess I was wrong.
>
>>
>>>>
>>>> Perhaps guess_base_level_size shouldn't bail on dim == 1 in the first
>>>> place?
>>>
>>>
>>> Well, it'll bail for *some* cases, so we might as well make it work. I
>>> think this was to avoid the situation where you end up over-guessing
>>> for a narrow image, such as this one, which only ever has height 1,
>>> but width up to 512. If we had started guessing at the 1x1 level, we'd
>>> come up with 512x512.
>>
>>
>> Yeah, I guess there is an argument in favor of bailing in this case when
>> called from guess_and_alloc_texture.
>>
>> But when called from st_finalize_texture, I think it should just do
>>
>> width = width > 1 ? width << level : 1;
>>
>> etc. Does that fix the test?
>
> Unfortunately I don't really have time to do much more than email
> right now. Feel free to investigate :)
>
Gents, can anyone share the current status on this patch/issue ?

Thanks
Emil