Commit f2d4abb2 by Geoff Lang Committed by Commit Bot

Vulkan: Correct the viewport before intersecting it with the scissor.

Eliminate the potential for integer overflow when clippling a large viewport rectangle by first limiting it to the Vulkan viewport size limits. BUG=chromium:1078378 Change-Id: I2648c6136d2d27d67a3fc5dae2de821279d70d81 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2215308Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Geoff Lang <geofflang@chromium.org>
parent b131d25f
...@@ -2472,13 +2472,8 @@ void ContextVk::updateSampleMask(const gl::State &glState) ...@@ -2472,13 +2472,8 @@ void ContextVk::updateSampleMask(const gl::State &glState)
} }
} }
void ContextVk::updateViewport(FramebufferVk *framebufferVk, gl::Rectangle ContextVk::getCorrectedViewport(const gl::Rectangle &viewport) const
const gl::Rectangle &viewport,
float nearPlane,
float farPlane,
bool invertViewport)
{ {
VkViewport vkViewport;
const gl::Caps &caps = getCaps(); const gl::Caps &caps = getCaps();
const VkPhysicalDeviceLimits &limitsVk = mRenderer->getPhysicalDeviceProperties().limits; const VkPhysicalDeviceLimits &limitsVk = mRenderer->getPhysicalDeviceProperties().limits;
const int viewportBoundsRangeLow = static_cast<int>(limitsVk.viewportBoundsRange[0]); const int viewportBoundsRangeLow = static_cast<int>(limitsVk.viewportBoundsRange[0]);
...@@ -2494,10 +2489,11 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk, ...@@ -2494,10 +2489,11 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk,
// VkPhysicalDeviceLimits::maxViewportDimensions[1] // VkPhysicalDeviceLimits::maxViewportDimensions[1]
int correctedHeight = std::min<int>(viewport.height, caps.maxViewportHeight); int correctedHeight = std::min<int>(viewport.height, caps.maxViewportHeight);
correctedHeight = std::max<int>(correctedHeight, 0); correctedHeight = std::max<int>(correctedHeight, 0);
// x and y must each be between viewportBoundsRange[0] and viewportBoundsRange[1], inclusive // x and y must each be between viewportBoundsRange[0] and viewportBoundsRange[1], inclusive.
int correctedX = std::min<int>(viewport.x, viewportBoundsRangeHigh); // Viewport size cannot be 0 so ensure there is always size for a 1x1 viewport
int correctedX = std::min<int>(viewport.x, viewportBoundsRangeHigh - 1);
correctedX = std::max<int>(correctedX, viewportBoundsRangeLow); correctedX = std::max<int>(correctedX, viewportBoundsRangeLow);
int correctedY = std::min<int>(viewport.y, viewportBoundsRangeHigh); int correctedY = std::min<int>(viewport.y, viewportBoundsRangeHigh - 1);
correctedY = std::max<int>(correctedY, viewportBoundsRangeLow); correctedY = std::max<int>(correctedY, viewportBoundsRangeLow);
// x + width must be less than or equal to viewportBoundsRange[1] // x + width must be less than or equal to viewportBoundsRange[1]
if ((correctedX + correctedWidth) > viewportBoundsRangeHigh) if ((correctedX + correctedWidth) > viewportBoundsRangeHigh)
...@@ -2510,12 +2506,23 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk, ...@@ -2510,12 +2506,23 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk,
correctedHeight = viewportBoundsRangeHigh - correctedY; correctedHeight = viewportBoundsRangeHigh - correctedY;
} }
gl::Box fbDimensions = framebufferVk->getState().getDimensions(); return gl::Rectangle(correctedX, correctedY, correctedWidth, correctedHeight);
gl::Rectangle correctedRect = }
gl::Rectangle(correctedX, correctedY, correctedWidth, correctedHeight);
void ContextVk::updateViewport(FramebufferVk *framebufferVk,
const gl::Rectangle &viewport,
float nearPlane,
float farPlane,
bool invertViewport)
{
gl::Box fbDimensions = framebufferVk->getState().getDimensions();
gl::Rectangle correctedRect = getCorrectedViewport(viewport);
gl::Rectangle rotatedRect; gl::Rectangle rotatedRect;
RotateRectangle(getRotationDrawFramebuffer(), false, fbDimensions.width, fbDimensions.height, RotateRectangle(getRotationDrawFramebuffer(), false, fbDimensions.width, fbDimensions.height,
correctedRect, &rotatedRect); correctedRect, &rotatedRect);
VkViewport vkViewport;
gl_vk::GetViewport(rotatedRect, nearPlane, farPlane, invertViewport, gl_vk::GetViewport(rotatedRect, nearPlane, farPlane, invertViewport,
// If the surface is rotated 90/270 degrees, use the framebuffer's width // If the surface is rotated 90/270 degrees, use the framebuffer's width
// instead of the height for calculating the final viewport. // instead of the height for calculating the final viewport.
...@@ -2538,7 +2545,8 @@ angle::Result ContextVk::updateScissor(const gl::State &glState) ...@@ -2538,7 +2545,8 @@ angle::Result 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, glState.getViewport(), &viewportClippedRenderArea); gl::ClipRectangle(renderArea, getCorrectedViewport(glState.getViewport()),
&viewportClippedRenderArea);
gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false); gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false);
gl::Rectangle rotatedScissoredArea; gl::Rectangle rotatedScissoredArea;
......
...@@ -669,6 +669,7 @@ class ContextVk : public ContextImpl, public vk::Context ...@@ -669,6 +669,7 @@ class ContextVk : public ContextImpl, public vk::Context
uint32_t *numIndicesOut); uint32_t *numIndicesOut);
angle::Result setupDispatch(const gl::Context *context, vk::CommandBuffer **commandBufferOut); angle::Result setupDispatch(const gl::Context *context, vk::CommandBuffer **commandBufferOut);
gl::Rectangle getCorrectedViewport(const gl::Rectangle &viewport) const;
void updateViewport(FramebufferVk *framebufferVk, void updateViewport(FramebufferVk *framebufferVk,
const gl::Rectangle &viewport, const gl::Rectangle &viewport,
float nearPlane, float nearPlane,
......
...@@ -322,6 +322,66 @@ TEST_P(ViewportTest, DrawLineWithLargeViewport) ...@@ -322,6 +322,66 @@ TEST_P(ViewportTest, DrawLineWithLargeViewport)
} }
} }
// Test very large viewport sizes so sanitizers can verify there is no undefined behaviour
TEST_P(ViewportTest, Overflow)
{
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Green());
glUseProgram(program);
std::vector<Vector3> vertices = {{-1.0f, -1.0f, 0.0f}, {1.0f, 1.0f, 0.0f}};
const GLint positionLocation = glGetAttribLocation(program, essl1_shaders::PositionAttrib());
ASSERT_NE(-1, positionLocation);
GLBuffer vertexBuffer;
glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
glBufferData(GL_ARRAY_BUFFER, sizeof(vertices[0]) * vertices.size(), vertices.data(),
GL_STATIC_DRAW);
glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
glEnableVertexAttribArray(positionLocation);
constexpr int kMaxSize = std::numeric_limits<int>::max();
const int kTestViewportSizes[][4] = {
{
kMaxSize,
kMaxSize,
1,
1,
},
{
0,
0,
kMaxSize,
kMaxSize,
},
{
1,
1,
kMaxSize,
kMaxSize,
},
{
kMaxSize,
kMaxSize,
kMaxSize,
kMaxSize,
},
};
for (const int *viewportSize : kTestViewportSizes)
{
// Set the viewport.
glViewport(viewportSize[0], viewportSize[1], viewportSize[2], viewportSize[3]);
glClear(GL_COLOR_BUFFER_BIT);
glDrawArrays(GL_LINES, 0, static_cast<GLsizei>(vertices.size()));
glDisableVertexAttribArray(positionLocation);
ASSERT_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. D3D11 Feature Level 9 and D3D9 emulate large and negative viewports // tests should be run against. D3D11 Feature Level 9 and D3D9 emulate large and negative viewports
// in the vertex shader. We should test both of these as well as D3D11 Feature Level 10_0+. // in the vertex shader. We should test both of these as well as D3D11 Feature Level 10_0+.
......
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