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

[Juan A. Suarez Romero]

On Tue, 2019-02-12 at 11:31 -0600, Jason Ekstrand wrote:
> On Tue, Feb 12, 2019 at 10:48 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.
> > 
> > 
> > 
> > Vulkan 1.1.81 spec says that "the application must ensure (using scissor
> > 
> > if neccesary) that all rendering is contained in the render area [...]
> > 
> > [which] must be contained within the framebuffer dimesions".
> > 
> > 
> > 
> > While this should be done by the application, commit 465e5a86 added the
> > 
> > clamp to the framebuffer size, in case of application does not do it.
> > 
> > But this requires to know the framebuffer dimensions.
> > 
> > 
> > 
> > If we do not have a framebuffer at that moment, the best compromise we
> > 
> > can do is to just apply the scissor as it is, and let the application to
> > 
> > ensure the rendering is contained in the render area.
> > 
> > 
> > 
> > v2: do not clamp to framebuffer if there isn't a framebuffer
> > 
> > 
> > 
> > v3 (Jason):
> > 
> > - clamp earlier in the conditional
> > 
> > - clamp to render area if command buffer is primary
> > 
> > 
> > 
> > v4: clamp also x and y to render area (Jason)
> > 
> > 
> > 
> > Fixes: 465e5a86 ("anv: Clamp scissors to the framebuffer boundary")
> > 
> > CC: Jason Ekstrand <jason at jlekstrand.net>
> > 
> > ---
> > 
> >  src/intel/vulkan/gen7_cmd_buffer.c | 32 +++++++++++++++++++++++++-----
> > 
> >  1 file changed, 27 insertions(+), 5 deletions(-)
> > 
> > 
> > 
> > diff --git a/src/intel/vulkan/gen7_cmd_buffer.c b/src/intel/vulkan/gen7_cmd_buffer.c
> > 
> > index 352892aee33..2924c6031fd 100644
> > 
> > --- a/src/intel/vulkan/gen7_cmd_buffer.c
> > 
> > +++ b/src/intel/vulkan/gen7_cmd_buffer.c
> > 
> > @@ -70,12 +70,34 @@ gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer *cmd_buffer)
> > 
> >        };
> > 
> > 
> > 
> >        const int max = 0xffff;
> > 
> > +
> > 
> > +      uint32_t y = s->offset.y;
> > 
> > +      uint32_t x = s->offset.x;
> > 
> > +      uint32_t height = s->offset.y + s->extent.height - 1;
> > 
> > +      uint32_t width = s->offset.x + s->extent.width - 1;
> 
> These should be x_max and y_max not width and height.  With that changed,

Right. I'll change also "x" and "y" by "x_min" and "y_min".
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> 
> Sorry we're going to v5...
> 
 Not problem!
	J.A.
> --Jason
>  
> > +
> > 
> > +      /* Do this math using int64_t so overflow gets clamped correctly. */
> > 
> > +      if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) {
> > 
> > +         y = clamp_int64((uint64_t) y, cmd_buffer->state.render_area.offset.y, max);
> > 
> > +         x = clamp_int64((uint64_t) x, cmd_buffer->state.render_area.offset.x, max);
> > 
> > +         height = clamp_int64((uint64_t) height, 0,
> > 
> > +                              cmd_buffer->state.render_area.offset.y +
> > 
> > +                              cmd_buffer->state.render_area.extent.height - 1);
> > 
> > +         width = clamp_int64((uint64_t) width, 0,
> > 
> > +                             cmd_buffer->state.render_area.offset.x +
> > 
> > +                             cmd_buffer->state.render_area.extent.width - 1);
> > 
> > +      } else if (fb) {
> > 
> > +         y = clamp_int64((uint64_t) y, 0, max);
> > 
> > +         x = clamp_int64((uint64_t) x, 0, max);
> > 
> > +         height = clamp_int64((uint64_t) height, 0, fb->height - 1);
> > 
> > +         width = clamp_int64((uint64_t) width, 0, fb->width - 1);
> > 
> > +      }
> > 
> > +
> > 
> >        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)
> > 
> > +         .ScissorRectangleYMin = y,
> > 
> > +         .ScissorRectangleXMin = x,
> > 
> > +         .ScissorRectangleYMax = height,
> > 
> > +         .ScissorRectangleXMax = width
> > 
> >        };
> > 
> > 
> > 
> >        if (s->extent.width <= 0 || s->extent.height <= 0) {
> > 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190212/8abd005a/attachment.html>