[Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer

[Juan A. Suarez Romero]

On Fri, 2019-02-01 at 15:33 -0600, Jason Ekstrand wrote:
> On Fri, Feb 1, 2019 at 10:14 AM Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > This can happen when we record a VkCmdDraw in a secondary buffer that
> > was created inheriting from the primary buffer, but with the framebuffer
> > set to NULL in the VkCommandBufferInheritanceInfo.
> > 
> > CC: Jason Ekstrand <jason at jlekstrand.net>
> > ---
> >  src/intel/vulkan/gen7_cmd_buffer.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/gen7_cmd_buffer.c b/src/intel/vulkan/gen7_cmd_buffer.c
> > index 352892aee33..fe1a47f6ce6 100644
> > --- a/src/intel/vulkan/gen7_cmd_buffer.c
> > +++ b/src/intel/vulkan/gen7_cmd_buffer.c
> > @@ -70,12 +70,21 @@ gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer *cmd_buffer)
> >        };
> > 
> >        const int max = 0xffff;
> > +
> > +      uint32_t height = 0;
> > +      uint32_t width = 0;
> > +
> > +      if (fb) {
> > +        height = fb->height;
> > +        width = fb->width;
> > +      }
> > +
> >        struct GEN7_SCISSOR_RECT scissor = {
> >           /* Do this math using int64_t so overflow gets clamped correctly. */
> >           .ScissorRectangleYMin = clamp_int64(s->offset.y, 0, max),
> >           .ScissorRectangleXMin = clamp_int64(s->offset.x, 0, max),
> > -         .ScissorRectangleYMax = clamp_int64((uint64_t) s->offset.y + s->extent.height - 1, 0, fb->height - 1),
> > -         .ScissorRectangleXMax = clamp_int64((uint64_t) s->offset.x + s->extent.width - 1, 0, fb->width - 1)
> > +         .ScissorRectangleYMax = clamp_int64((uint64_t) s->offset.y + s->extent.height - 1, 0, height - 1),
> > +         .ScissorRectangleXMax = clamp_int64((uint64_t) s->offset.x + s->extent.width - 1, 0, width - 1)
> 
> If fb == NULL, won't width/height be 0 and this be -1 and we end up clamping to -1?  I think we want to rather make the clamp conditional on having the framebuffer.
> 

Right. And as ScissorRectangle(X/Y)Max is an uint value, its value would be
MAX_UINT.

I think as we do not know the framebuffer size (which will be know later, IIUC),
we are emitting the scissor for the full rectangle.




> --Jason
>  
> >        };
> > 
> >        if (s->extent.width <= 0 || s->extent.height <= 0) {