<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>On Mon, 2019-02-04 at 10:01 -0600, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 4, 2019 at 3:02 AM Juan A. Suarez Romero <<a href="mailto:jasuarez@igalia.com">jasuarez@igalia.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">On Fri, 2019-02-01 at 15:33 -0600, Jason Ekstrand wrote:<br>
> On Fri, Feb 1, 2019 at 10:14 AM Juan A. Suarez Romero <<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>> wrote:<br>
> > This can happen when we record a VkCmdDraw in a secondary buffer that<br>
> > was created inheriting from the primary buffer, but with the framebuffer<br>
> > set to NULL in the VkCommandBufferInheritanceInfo.<br>
> > <br>
> > CC: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
> > ---<br>
> >  src/intel/vulkan/gen7_cmd_buffer.c | 13 +++++++++++--<br>
> >  1 file changed, 11 insertions(+), 2 deletions(-)<br>
> > <br>
> > diff --git a/src/intel/vulkan/gen7_cmd_buffer.c b/src/intel/vulkan/gen7_cmd_buffer.c<br>
> > index 352892aee33..fe1a47f6ce6 100644<br>
> > --- a/src/intel/vulkan/gen7_cmd_buffer.c<br>
> > +++ b/src/intel/vulkan/gen7_cmd_buffer.c<br>
> > @@ -70,12 +70,21 @@ gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer *cmd_buffer)<br>
> >        };<br>
> > <br>
> >        const int max = 0xffff;<br>
> > +<br>
> > +      uint32_t height = 0;<br>
> > +      uint32_t width = 0;<br>
> > +<br>
> > +      if (fb) {<br>
> > +        height = fb->height;<br>
> > +        width = fb->width;<br>
> > +      }<br>
> > +<br>
> >        struct GEN7_SCISSOR_RECT scissor = {<br>
> >           /* Do this math using int64_t so overflow gets clamped correctly. */<br>
> >           .ScissorRectangleYMin = clamp_int64(s->offset.y, 0, max),<br>
> >           .ScissorRectangleXMin = clamp_int64(s->offset.x, 0, max),<br>
> > -         .ScissorRectangleYMax = clamp_int64((uint64_t) s->offset.y + s->extent.height - 1, 0, fb->height - 1),<br>
> > -         .ScissorRectangleXMax = clamp_int64((uint64_t) s->offset.x + s->extent.width - 1, 0, fb->width - 1)<br>
> > +         .ScissorRectangleYMax = clamp_int64((uint64_t) s->offset.y + s->extent.height - 1, 0, height - 1),<br>
> > +         .ScissorRectangleXMax = clamp_int64((uint64_t) s->offset.x + s->extent.width - 1, 0, width - 1)<br>
> <br>
> 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.<br>
> <br>
<br>
Right. And as ScissorRectangle(X/Y)Max is an uint value, its value would be<br>
MAX_UINT.<br></blockquote><div><br></div><div>Which is fine if they specified a scissor that is >= the framebuffer size.  However, if they are actually trying to scissor something, this makes their scissor disappear.  That's bad. :-)<br></div><div><br></div></div></div></blockquote><div>I see :)</div><div><br></div><div>So we are setting the clamping to ensure that the rendering happens inside the render area, which must be contained withing the framebuffer area.</div><div><br></div><div>This is something that actually the application should ensure, but we do it ourselves because it takes little effort to do it.</div><div><br></div><div>But if we do not have a framebuffer, we can't do it, so in this case I think the best compromise is go ahead with the scissor, and let the application to ensure later that the framebuffer is big enough for the rendering area.</div><div><br></div><div>I'm sending a V2 patch.</div><div><br></div><div>  J.A.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>--Jason<br></div><div> </div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
I think as we do not know the framebuffer size (which will be know later, IIUC),<br>
we are emitting the scissor for the full rectangle.<br>
<br>
<br>
<br>
<br>
> --Jason<br>
>  <br>
> >        };<br>
> > <br>
> >        if (s->extent.width <= 0 || s->extent.height <= 0) {<br>
<br>
</blockquote></div></div>
</blockquote></body></html>