Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
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
> 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
> >
> > ---
> >
> > 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 = 0x;
> >
> > +
> >
> > + 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
>
> 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) {
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
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
---
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 = 0x;
+
+ 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;
+
+ /* 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) {
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
On Tue, Feb 12, 2019 at 10:48 AM Juan A. Suarez Romero
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
> ---
> 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 = 0x;
> +
> + 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,
Reviewed-by: Jason Ekstrand
Sorry we're going to v5...
--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) {
> --
> 2.20.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
On Mon, 2019-02-04 at 10:01 -0600, Jason Ekstrand wrote:
> On Mon, Feb 4, 2019 at 3:02 AM Juan A. Suarez Romero
> wrote:
> > 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
> > > 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
> >
> > > > ---
> >
> > > > 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 = 0x;
> >
> > > > +
> >
> > > > + 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.
>
> 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. :-)
>
I see :)
So we are setting the clamping to ensure that the rendering happens inside the
render area, which must be contained withing the framebuffer area.
This is something that actually the application should ensure, but we do it
ourselves because it takes little effort to do it.
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.
I'm sending a V2 patch.
J.A.
> --Jason
>
> > 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) {
> >
> >
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
On Mon, Feb 4, 2019 at 3:02 AM Juan A. Suarez Romero
wrote:
> 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 <
> 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.
> > >
> > > CC: Jason Ekstrand
> > > ---
> > > 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 = 0x;
> > > +
> > > + 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.
>
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. :-)
--Jason
> 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) {
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
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
> 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
> > ---
> > 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 = 0x;
> > +
> > + 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) {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
On Fri, Feb 1, 2019 at 10:14 AM Juan A. Suarez Romero
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
> ---
> 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 = 0x;
> +
> + 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.
--Jason
>};
>
>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
[Mesa-dev] [PATCH] anv/cmd_buffer: check for NULL framebuffer
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
---
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 = 0x;
+
+ 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 (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
