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

[Ilia Mirkin]

On Wed, Jul 27, 2016 at 9:46 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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 ?

Nicolai followed up with a series of patches which fixed the
underlying issues (and then some).

  -ilia