Am 30.08.2017 um 16:46 schrieb 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 <srol...@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).
Albeit I think this isn't quite the same thing: null sampler views are "natural" defaults - the driver can't reasonably do anything else, plus, typically if you render non-textured triangles, these views do not actually matter no matter what the driver would assume. But if you enable scissor, then scissor rectangle surely matters. And it's really not obvious 0-sized scissor is the "natural" default neither (GL's default of it would be drawable width/height for width and height). That said, the driver definitely also looks at fault here: its idea of a default scissor also really was an empty scissor, it just neglected to calculate the derived values... So the fix is correct, never setting a scissor in the state tracker is imho iffy however, iff the scissor rectangle is needed (that is, scissor test enabled). I think that's just asking for trouble. Roland > -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: >>>>> >>>> >>> >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
