On Mon, Feb 11, 2019 at 5:30 AM Juan A. Suarez Romero <jasua...@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 > > Fixes: 465e5a86 ("anv: Clamp scissors to the framebuffer boundary") > CC: Jason Ekstrand <ja...@jlekstrand.net> > --- > src/intel/vulkan/gen7_cmd_buffer.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/src/intel/vulkan/gen7_cmd_buffer.c > b/src/intel/vulkan/gen7_cmd_buffer.c > index 352892aee33..1a8507fc94c 100644 > --- a/src/intel/vulkan/gen7_cmd_buffer.c > +++ b/src/intel/vulkan/gen7_cmd_buffer.c > @@ -70,12 +70,28 @@ gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer > *cmd_buffer) > }; > > const int max = 0xffff; > + > + uint32_t height = s->offset.y + s->extent.height - 1; > + uint32_t width = s->offset.x + s->extent.width - 1; > + > + /* Do this math using int64_t so overflow gets clamped correctly. */ > + if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) { > + 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); > Sorry for continuing to comment.... but should we clamp x and y to the render area as well? > + } else if (fb) { > + 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) > + .ScissorRectangleYMax = height, > + .ScissorRectangleXMax = width > }; > > if (s->extent.width <= 0 || s->extent.height <= 0) { > -- > 2.20.1 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev