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

[Ilia Mirkin]

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 :)

  -ilia