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

[Ilia Mirkin]

On Mon, Jun 6, 2016 at 12:52 PM, Brian Paul <brianp at vmware.com> wrote:
> On 06/06/2016 10:05 AM, Ilia Mirkin wrote:
>>
>> On Mon, Jun 6, 2016 at 11:37 AM, Brian Paul <brianp at vmware.com> wrote:
>>>
>>> On 06/05/2016 12:24 AM, 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.
>>>>
>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>>>> ---
>>>>    src/mesa/state_tracker/st_cb_texture.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_cb_texture.c
>>>> b/src/mesa/state_tracker/st_cb_texture.c
>>>> index d38f24c..1dd1ef6 100644
>>>> --- a/src/mesa/state_tracker/st_cb_texture.c
>>>> +++ b/src/mesa/state_tracker/st_cb_texture.c
>>>> @@ -2463,9 +2463,9 @@ st_finalize_texture(struct gl_context *ctx,
>>>>                                     firstImage->base.Depth2,
>>>>                                     firstImage->base.Level,
>>>>                                     &width, &height, &depth)) {
>>>> -         width = stObj->width0;
>>>> -         height = stObj->height0;
>>>> -         depth = stObj->depth0;
>>>> +         width = stObj->width0 = firstImage->base.Width2;
>>>> +         height = stObj->height0 = firstImage->base.Height2;
>>>> +         depth = stObj->depth0 = firstImage->base.Depth2;
>>>>          } else {
>>>>             /* The width/height/depth may have been previously reset in
>>>>              * guess_and_alloc_texture. */
>>>>
>>>
>>> Does this fix a crash or glitch or something else?
>>>
>>> The state tracker's texture code is pretty delicate so I'd like to fully
>>> understand the change.
>>
>>
>> Yes, this fixes a trace that was supplied by someone on IRC. The
>> situation was that the texture only ever had levels 2..9 (or 10?) set,
>> and its base level was set to 2. And each level was Nx1 (but
>> GL_TEXTURE_2D). So all the guessing logic always bailed, which means
>> that the stObj->width/height/depth were set to 0, which caused asserts
>> on the next line. (And I assume it wouldn't have rendered correctly
>> either, but it can be hard to tell in a large scene.)
>>
>>>
>>> Maybe we should have a piglit test for this?
>>
>>
>> I can try to whip something up, not sure when I'll get to it though.
>> Definitely not today, and probably not for a few at least.
>
>
> I whipped up a piglet test.  No assertion with your patch, but still
> incorrect rendering.  The textured quad is drawn black instead of gray.
> Works w/ NVIDIA driver.
>
> I'm attaching the patch if you want to use it to investigate further.  I
> have to get back to other things.

Thanks. My knowledge of pre-GL2 usage is ... lacking. I guess
glEnable(GL_TEXTURE_2D) is the same thing as glActiveTexture or
whatever? Anyways, I assume you know what you're doing.

Black usually means "incomplete texture", perhaps something decided
that it was incomplete down the line?

Anyways, like you, I have to do other things for a while. I do think
it's reasonable that if we failed to guess the size that we should
just use the first image's size rather than some possibly-old idea of
what the overall texture object size is.

  -ilia