[Mesa-dev] [PATCH] llvmpipe: initialize llvmpipe->dirty with LP_NEW_SCISSOR

[Brian Paul]

On 08/30/2017 02:43 AM, Roland Scheidegger wrote:
> Am 30.08.2017 um 04:49 schrieb Brian Paul:
>> On 08/29/2017 06:17 PM, Roland Scheidegger wrote:
>>> Looks good to me, thanks.
>>>
>>> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>>>
>>> Albeit I'm not quite sure why it is never set here?
>>
>> This particular test sets a 0 x 0 scissor region.  All the
>> pipe_scissor_state members are zero and when the state tracker does a
>> memcmp() with its copy it sees no difference and skips the call to
>> pipe->set_scissor_state().
> Ahh ok. It is pretty much the same as with the null fb not getting
> initialized then.
> Albeit technically, I think state trackers should ensure that they
> always initialize the state, this burden shouldn't be on the drivers.
> (With the all null fb, this didn't help, because both cso and llvmpipe
> threw out an initial null fb as no change on their own.)

That's a good point.  We've never clearly defined how much 
initialization a driver should be responsible for, vs. the state tracker.

I think we've implicitly been expecting the driver to do everything it 
needs to function, without relying on the state tracker for everything. 
  For example, if we want to render some non-textured triangles and we 
never call the driver's pipe_context::set_sampler_views() or 
bind_sampler_states() functions, we'd expect rendering to work.  And as 
in this case, if set_scissor_state() is never called, I think we'd 
expect rendering to work (though, we are enabling the scissor flag).

-Brian

>
> Roland
>
>
>>
>> -Brian
>>
>>
>>> This bug looks similar in nature to the uninitialized fb (see
>>> 8bfe451ed30918244618608871423b2a72cf9767) and I thought that was
>>> impossible to hit with gl state tracker...
>>>
>>> Roland
>>>
>>> Am 28.08.2017 um 22:20 schrieb Brian Paul:
>>>> If llvmpipe_set_scissor_states() is never called, we still need to be
>>>> sure
>>>> that derived scissor/clip state is updated.  As of commit
>>>> 743ad599a97d09b1
>>>> that function might not be called.
>>>>
>>>> Fixes regressed Piglit gl-1.0-scissor-offscreen -fbo -auto test.
>>>>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101709
>>>> ---
>>>>    src/gallium/drivers/llvmpipe/lp_context.c | 6 ++++++
>>>>    1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/src/gallium/drivers/llvmpipe/lp_context.c
>>>> b/src/gallium/drivers/llvmpipe/lp_context.c
>>>> index 599a9f1..613d60f 100644
>>>> --- a/src/gallium/drivers/llvmpipe/lp_context.c
>>>> +++ b/src/gallium/drivers/llvmpipe/lp_context.c
>>>> @@ -227,6 +227,12 @@ llvmpipe_create_context(struct pipe_screen
>>>> *screen, void *priv,
>>>>
>>>>       lp_reset_counters();
>>>>
>>>> +   /* If llvmpipe_set_scissor_states() is never called, we still
>>>> need to
>>>> +    * make sure that derived scissor state is computed.
>>>> +    * See https://bugs.freedesktop.org/show_bug.cgi?id=101709
>>>> +    */
>>>> +   llvmpipe->dirty |= LP_NEW_SCISSOR;
>>>> +
>>>>       return &llvmpipe->pipe;
>>>>
>>>>     fail:
>>>>
>>>
>>
>