Commit b574643e by Geoff Lang Committed by Commit Bot

D3D11: Skip blits if there is no intersection of dest areas

Blit11 would clip the destination rectangle with the destination size but ignore the result. gl::ClipRectangle returns false when the rectangles do not intersect at all, indicating the blit can be skipped. This could lead to an out-of-bounds write to the GPU memory for the destination texture. Mark ClipRectangle as nodiscard to prevent future issues. Bug: chromium:1199402 Change-Id: I260e82d0917b8aa7e7887f2c9f7ed4b1a03ba785 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2836786Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org>
parent 3691797a
...@@ -90,7 +90,9 @@ enum class ClipSpaceOrigin ...@@ -90,7 +90,9 @@ enum class ClipSpaceOrigin
}; };
// Calculate the intersection of two rectangles. Returns false if the intersection is empty. // Calculate the intersection of two rectangles. Returns false if the intersection is empty.
bool ClipRectangle(const Rectangle &source, const Rectangle &clip, Rectangle *intersection); ANGLE_NO_DISCARD bool ClipRectangle(const Rectangle &source,
const Rectangle &clip,
Rectangle *intersection);
// Calculate the smallest rectangle that covers both rectangles. This rectangle may cover areas // Calculate the smallest rectangle that covers both rectangles. This rectangle may cover areas
// not covered by the two rectangles, for example in this situation: // not covered by the two rectangles, for example in this situation:
// //
......
...@@ -141,7 +141,10 @@ void StretchedBlitNearest(const gl::Box &sourceArea, ...@@ -141,7 +141,10 @@ void StretchedBlitNearest(const gl::Box &sourceArea,
uint8_t *destData) uint8_t *destData)
{ {
gl::Rectangle clippedDestArea(destArea.x, destArea.y, destArea.width, destArea.height); gl::Rectangle clippedDestArea(destArea.x, destArea.y, destArea.width, destArea.height);
gl::ClipRectangle(clippedDestArea, clipRect, &clippedDestArea); if (!gl::ClipRectangle(clippedDestArea, clipRect, &clippedDestArea))
{
return;
}
// Determine if entire rows can be copied at once instead of each individual pixel. There // Determine if entire rows can be copied at once instead of each individual pixel. There
// must be no out of bounds lookups, whole rows copies, and no scale. // must be no out of bounds lookups, whole rows copies, and no scale.
......
...@@ -1120,7 +1120,10 @@ angle::Result FramebufferGL::clipSrcRegion(const gl::Context *context, ...@@ -1120,7 +1120,10 @@ angle::Result FramebufferGL::clipSrcRegion(const gl::Context *context,
// If pixels lying outside the read framebuffer, adjust src region // If pixels lying outside the read framebuffer, adjust src region
// and dst region to appropriate in-bounds regions respectively. // and dst region to appropriate in-bounds regions respectively.
gl::Rectangle realSourceRegion; gl::Rectangle realSourceRegion;
ClipRectangle(bounds.sourceRegion, bounds.sourceBounds, &realSourceRegion); if (!ClipRectangle(bounds.sourceRegion, bounds.sourceBounds, &realSourceRegion))
{
return angle::Result::Stop;
}
GLuint xOffset = realSourceRegion.x - bounds.sourceRegion.x; GLuint xOffset = realSourceRegion.x - bounds.sourceRegion.x;
GLuint yOffset = realSourceRegion.y - bounds.sourceRegion.y; GLuint yOffset = realSourceRegion.y - bounds.sourceRegion.y;
......
...@@ -1625,7 +1625,10 @@ void ContextMtl::updateScissor(const gl::State &glState) ...@@ -1625,7 +1625,10 @@ void ContextMtl::updateScissor(const gl::State &glState)
// Clip the render area to the viewport. // Clip the render area to the viewport.
gl::Rectangle viewportClippedRenderArea; gl::Rectangle viewportClippedRenderArea;
gl::ClipRectangle(renderArea, glState.getViewport(), &viewportClippedRenderArea); if (!gl::ClipRectangle(renderArea, glState.getViewport(), &viewportClippedRenderArea))
{
viewportClippedRenderArea = gl::Rectangle();
}
gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false); gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false);
if (framebufferMtl->flipY()) if (framebufferMtl->flipY())
......
...@@ -3061,8 +3061,11 @@ void ContextVk::updateScissor(const gl::State &glState) ...@@ -3061,8 +3061,11 @@ void ContextVk::updateScissor(const gl::State &glState)
// Clip the render area to the viewport. // Clip the render area to the viewport.
gl::Rectangle viewportClippedRenderArea; gl::Rectangle viewportClippedRenderArea;
gl::ClipRectangle(renderArea, getCorrectedViewport(glState.getViewport()), if (!gl::ClipRectangle(renderArea, getCorrectedViewport(glState.getViewport()),
&viewportClippedRenderArea); &viewportClippedRenderArea))
{
viewportClippedRenderArea = gl::Rectangle();
}
gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false); gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false);
gl::Rectangle rotatedScissoredArea; gl::Rectangle rotatedScissoredArea;
......
...@@ -2524,6 +2524,30 @@ TEST_P(BlitFramebufferTest, BlitFramebufferSizeOverflow2) ...@@ -2524,6 +2524,30 @@ TEST_P(BlitFramebufferTest, BlitFramebufferSizeOverflow2)
EXPECT_GL_ERROR(GL_INVALID_VALUE); EXPECT_GL_ERROR(GL_INVALID_VALUE);
} }
// Test an edge case in D3D11 stencil blitting on the CPU that does not properly clip the
// destination regions
TEST_P(BlitFramebufferTest, BlitFramebufferStencilClipNoIntersection)
{
GLFramebuffer framebuffers[2];
glBindFramebuffer(GL_READ_FRAMEBUFFER, framebuffers[0]);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffers[1]);
GLRenderbuffer renderbuffers[2];
glBindRenderbuffer(GL_RENDERBUFFER, renderbuffers[0]);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, 4, 4);
glFramebufferRenderbuffer(GL_READ_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
renderbuffers[0]);
glBindRenderbuffer(GL_RENDERBUFFER, renderbuffers[1]);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, 4, 4);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
renderbuffers[1]);
glBlitFramebuffer(0, 0, 4, 4, 1 << 24, 1 << 24, 1 << 25, 1 << 25, GL_STENCIL_BUFFER_BIT,
GL_NEAREST);
EXPECT_GL_NO_ERROR();
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these // Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against. // tests should be run against.
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(BlitFramebufferANGLETest); GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(BlitFramebufferANGLETest);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment