<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>On Tue, 2019-02-12 at 11:31 -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 Tue, Feb 12, 2019 at 10:48 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">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>
Vulkan 1.1.81 spec says that "the application must ensure (using scissor<br>
if neccesary) that all rendering is contained in the render area [...]<br>
[which] must be contained within the framebuffer dimesions".<br>
<br>
While this should be done by the application, commit 465e5a86 added the<br>
clamp to the framebuffer size, in case of application does not do it.<br>
But this requires to know the framebuffer dimensions.<br>
<br>
If we do not have a framebuffer at that moment, the best compromise we<br>
can do is to just apply the scissor as it is, and let the application to<br>
ensure the rendering is contained in the render area.<br>
<br>
v2: do not clamp to framebuffer if there isn't a framebuffer<br>
<br>
v3 (Jason):<br>
- clamp earlier in the conditional<br>
- clamp to render area if command buffer is primary<br>
<br>
v4: clamp also x and y to render area (Jason)<br>
<br>
Fixes: 465e5a86 ("anv: Clamp scissors to the framebuffer boundary")<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 | 32 +++++++++++++++++++++++++-----<br>
 1 file changed, 27 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/intel/vulkan/gen7_cmd_buffer.c b/src/intel/vulkan/gen7_cmd_buffer.c<br>
index 352892aee33..2924c6031fd 100644<br>
--- a/src/intel/vulkan/gen7_cmd_buffer.c<br>
+++ b/src/intel/vulkan/gen7_cmd_buffer.c<br>
@@ -70,12 +70,34 @@ gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer *cmd_buffer)<br>
       };<br>
<br>
       const int max = 0xffff;<br>
+<br>
+      uint32_t y = s->offset.y;<br>
+      uint32_t x = s->offset.x;<br>
+      uint32_t height = s->offset.y + s->extent.height - 1;<br>
+      uint32_t width = s->offset.x + s->extent.width - 1;<br></blockquote><div><br></div><div>These should be x_max and y_max not width and height.  With that changed,</div></div></div></blockquote><div><br></div><div>Right. I'll change also "x" and "y" by "x_min" and "y_min".</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>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></div><div><br></div><div>Sorry we're going to v5...</div><div><br></div></div></div></blockquote><div> </div><div>Not problem!</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">
+<br>
+      /* Do this math using int64_t so overflow gets clamped correctly. */<br>
+      if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) {<br>
+         y = clamp_int64((uint64_t) y, cmd_buffer->state.render_area.offset.y, max);<br>
+         x = clamp_int64((uint64_t) x, cmd_buffer->state.render_area.offset.x, max);<br>
+         height = clamp_int64((uint64_t) height, 0,<br>
+                              cmd_buffer->state.render_area.offset.y +<br>
+                              cmd_buffer->state.render_area.extent.height - 1);<br>
+         width = clamp_int64((uint64_t) width, 0,<br>
+                             cmd_buffer->state.render_area.offset.x +<br>
+                             cmd_buffer->state.render_area.extent.width - 1);<br>
+      } else if (fb) {<br>
+         y = clamp_int64((uint64_t) y, 0, max);<br>
+         x = clamp_int64((uint64_t) x, 0, max);<br>
+         height = clamp_int64((uint64_t) height, 0, fb->height - 1);<br>
+         width = clamp_int64((uint64_t) width, 0, fb->width - 1);<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>
+         .ScissorRectangleYMin = y,<br>
+         .ScissorRectangleXMin = x,<br>
+         .ScissorRectangleYMax = height,<br>
+         .ScissorRectangleXMax = width<br>
       };<br>
<br>
       if (s->extent.width <= 0 || s->extent.height <= 0) {<br>
</blockquote></div></div></blockquote></body></html>