[Mesa-dev] [PATCH] st/mesa: Finalize texture on render-to-texture.

[Brian Paul]

On 09/22/2011 09:03 AM, Michel Dänzer wrote:
> On Mit, 2011-09-21 at 10:31 -0600, Brian Paul wrote:
>> On 09/21/2011 03:36 AM, Michel Dänzer wrote:
>>> On Die, 2011-09-20 at 12:55 -0600, Brian Paul wrote:
>>>> On 09/20/2011 10:40 AM, Michel Dänzer wrote:
>>>>> On Don, 2011-09-15 at 13:01 -0600, Brian Paul wrote:
>>>>>> On 09/15/2011 10:08 AM, Michel Dänzer wrote:
>>>>>>> On Don, 2011-09-15 at 09:01 -0600, Brian Paul wrote:
>>>>>>>> On 09/15/2011 04:55 AM, Michel Dänzer wrote:
>>>>>>>>> On Mit, 2011-09-14 at 11:44 -0600, Brian Paul wrote:
>>>>>>>>>> On 09/14/2011 11:16 AM, Michel Dänzer wrote:
>>>>>>>>>>> From: Michel Dänzer<michel.daenzer at amd.com>
>>>>>>>>>>>
>>>>>>>>>>> This makes sure that stObj->pt exists and is up to date.
>>>>>>>>>>>
>>>>>>>>>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=39193 .
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Michel Dänzer<michel.daenzer at amd.com>
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>> Reviewed-by: Brian Paul<brianp at vmware.com>
>>>>>>>>>
>>>>>>>>> Thanks, Brian.
>>>>>>>>>
>>>>>>>>>> I assume there's no piglit regressions.
>>>>>>>>>
>>>>>>>>> There are no regressions per se with quick-driver.tests, but there's a
>>>>>>>>> change for the fbo-incomplete-texture-03 test: Without this patch, it
>>>>>>>>> fails with "FBO clear didn't work". With this patch, it passes without
>>>>>>>>> --enable-debug or with GALLIUM_ABORT_ON_ASSERT=0, but there are some
>>>>>>>>> assertion failures:
>>>>>>>>>
>>>>>>>>> state_tracker/st_texture.c:369:st_texture_image_copy: Assertion `u_minify(src->width0, srcLevel) == width' failed.
>>>>>>>>> state_tracker/st_texture.c:370:st_texture_image_copy: Assertion `u_minify(src->height0, srcLevel) == height' failed.
>>>>>>>>>
>>>>>>>>> Not sure if this is a new problem introduced by this patch, or an old
>>>>>>>>> one uncovered by it, but I'm leaning towards the latter.
>>>>>>>>
>>>>>>>> Yeah, I'm looking into it.  I have a patch that fixes the assertion
>>>>>>>> but it needs testing...
>>>>>>>
>>>>>>> I'd be happy to give that a spin, but I guess it's okay to push this fix
>>>>>>> in the meantime?
>>>>>>
>>>>>> I guess I'd like to fix up the assertion too or we'll probably get
>>>>>> some new bug reports.
>>>>>>
>>>>>> Here's my patch that seems to fix the issue:
>>>>>>
>>>>>> diff --git a/src/mesa/state_tracker/st_cb_texture.c
>>>>>> b/src/mesa/state_tracker/st_cb_texture.c
>>>>>> index eab02fb..538a7a3 100644
>>>>>> --- a/src/mesa/state_tracker/st_cb_texture.c
>>>>>> +++ b/src/mesa/state_tracker/st_cb_texture.c
>>>>>> @@ -1679,7 +1679,7 @@ copy_image_data_to_texture(struct st_context *st,
>>>>>>            assert(dstImage->Depth == stImage->base.Depth);
>>>>>>         }
>>>>>>
>>>>>> -   if (stImage->pt) {
>>>>>> +   if (stImage->pt&&    stObj->base._Complete) {
>>>>>>            /* Copy potentially with the blitter:
>>>>>>             */
>>>>>>            st_texture_image_copy(st->pipe,
>>>>>>
>>>>>>
>>>>>> I did a quick run of piglit texture tests and didn't see any
>>>>>> regressions but I'd like to test more.
>>>>>>
>>>>>> The idea with this change is that if the texture is not complete
>>>>>> (mismatched image sizes) we can't draw with it anyway so skip copying
>>>>>> the texture data into the texture buffer.
>>>>>>
>>>>>> Maybe you can run some tests with this too...
>>>>>
>>>>> This change seems to make all fbo-generatemipmap-formats tests fail.
>>>>
>>>> Hmmm, I must have screwed up when I was testing.
>>>>
>>>> Here's a different patch.  It just replaces the assertions with a
>>>> conditional.  Both fbo-generatemipmap-formats and
>>>> fbo-incomplete-texture-03 seem ok.
>>>
>>> Yeah, this looks good.
>>
>> OK, feel free to incorporate that change into your patch and commit
>> it.  Maybe mention in the commit that it also fixes the
>> fbo-incomplete-texture-03 test.
>
> I figured it's better to keep the changes separate, so any potential
> regressions can be isolated to one or the other. Hope that's okay.

Sure.

-Brian