Commit 303ddb5c by Sean Risser

Make ClipDstRect/ClipSrcRect cease on non-finite

Now that the GLES unit tests can cause the float calculations in these functions to result in inf and NaN, some platforms are throwing errors when those floats are converted to integers. The Clip*Rect functions no longer continue processing if any operation results in a non-finite value, and tell their callers that they have failed to clip properly. Make Context.cpp set the error state to GL_INVALID_OPERATION when it fails to blit. Bug chromium:979986 Change-Id: I8e11e77b1e25eab37dee97aa616a908737b16036 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35269 Presubmit-Ready: Sean Risser <srisser@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Tested-by: 's avatarSean Risser <srisser@google.com>
parent dadeb009
...@@ -4146,7 +4146,11 @@ void Context::blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1 ...@@ -4146,7 +4146,11 @@ void Context::blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1
if(mState.scissorTestEnabled) // Only write to parts of the destination framebuffer which pass the scissor test if(mState.scissorTestEnabled) // Only write to parts of the destination framebuffer which pass the scissor test
{ {
sw::Rect scissorRect(mState.scissorX, mState.scissorY, mState.scissorX + mState.scissorWidth, mState.scissorY + mState.scissorHeight); sw::Rect scissorRect(mState.scissorX, mState.scissorY, mState.scissorX + mState.scissorWidth, mState.scissorY + mState.scissorHeight);
Device::ClipDstRect(sourceScissoredRect, destScissoredRect, scissorRect, flipX, flipY); if (!Device::ClipDstRect(sourceScissoredRect, destScissoredRect, scissorRect, flipX, flipY))
{
// Failed to clip, blitting can't happen.
return error(GL_INVALID_OPERATION);
}
} }
sw::SliceRectF sourceTrimmedRect = sourceScissoredRect; sw::SliceRectF sourceTrimmedRect = sourceScissoredRect;
...@@ -4155,10 +4159,18 @@ void Context::blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1 ...@@ -4155,10 +4159,18 @@ void Context::blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1
// The source & destination rectangles also may need to be trimmed if // The source & destination rectangles also may need to be trimmed if
// they fall out of the bounds of the actual draw and read surfaces. // they fall out of the bounds of the actual draw and read surfaces.
sw::Rect sourceTrimRect(0, 0, readBufferWidth, readBufferHeight); sw::Rect sourceTrimRect(0, 0, readBufferWidth, readBufferHeight);
Device::ClipSrcRect(sourceTrimmedRect, destTrimmedRect, sourceTrimRect, flipX, flipY); if (!Device::ClipSrcRect(sourceTrimmedRect, destTrimmedRect, sourceTrimRect, flipX, flipY))
{
// Failed to clip, blitting can't happen.
return error(GL_INVALID_OPERATION);
}
sw::Rect destTrimRect(0, 0, drawBufferWidth, drawBufferHeight); sw::Rect destTrimRect(0, 0, drawBufferWidth, drawBufferHeight);
Device::ClipDstRect(sourceTrimmedRect, destTrimmedRect, destTrimRect, flipX, flipY); if (!Device::ClipDstRect(sourceTrimmedRect, destTrimmedRect, destTrimRect, flipX, flipY))
{
// Failed to clip, blitting can't happen.
return error(GL_INVALID_OPERATION);
}
bool partialBufferCopy = false; bool partialBufferCopy = false;
......
...@@ -453,8 +453,8 @@ namespace es2 ...@@ -453,8 +453,8 @@ namespace es2
int dHeight = dest->getHeight(); int dHeight = dest->getHeight();
if(sourceRect && destRect && if(sourceRect && destRect &&
(sourceRect->width() == 0.0f || std::isnan(sourceRect->width()) || (sourceRect->width() == 0.0f || !std::isfinite(sourceRect->width()) ||
sourceRect->height() == 0.0f || std::isnan(sourceRect->height()) || sourceRect->height() == 0.0f || !std::isfinite(sourceRect->height()) ||
destRect->width() == 0.0f || destRect->height() == 0.0f)) destRect->width() == 0.0f || destRect->height() == 0.0f))
{ {
return true; // No work to do. return true; // No work to do.
...@@ -530,14 +530,20 @@ namespace es2 ...@@ -530,14 +530,20 @@ namespace es2
} }
sw::Rect srcClipRect(0, 0, sWidth, sHeight); sw::Rect srcClipRect(0, 0, sWidth, sHeight);
ClipSrcRect(sRect, dRect, srcClipRect, flipX, flipY); if (!ClipSrcRect(sRect, dRect, srcClipRect, flipX, flipY))
{
return true;
}
sw::Rect dstClipRect(0, 0, dWidth, dHeight); sw::Rect dstClipRect(0, 0, dWidth, dHeight);
ClipDstRect(sRect, dRect, dstClipRect, flipX, flipY); if (!ClipDstRect(sRect, dRect, dstClipRect, flipX, flipY))
{
return true;
}
if((sRect.width() == 0) || (sRect.height() == 0) || if((sRect.width() == 0) || (sRect.height() == 0) ||
(dRect.width() == 0) || (dRect.height() == 0) || (dRect.width() == 0) || (dRect.height() == 0) ||
(std::isnan(sRect.width()) || std::isnan(sRect.height()))) !std::isfinite(sRect.width()) || !std::isfinite(sRect.height()))
{ {
return true; // no work to do return true; // no work to do
} }
...@@ -885,7 +891,8 @@ namespace es2 ...@@ -885,7 +891,8 @@ namespace es2
return false; return false;
} }
if (std::isnan(rect->x0) || std::isnan(rect->x1) || std::isnan(rect->y0) || std::isnan(rect->y1)) if (!std::isfinite(rect->x0) || !std::isfinite(rect->x1) ||
!std::isfinite(rect->y0) || !std::isfinite(rect->y1))
{ {
return false; return false;
} }
...@@ -893,11 +900,15 @@ namespace es2 ...@@ -893,11 +900,15 @@ namespace es2
return true; return true;
} }
void Device::ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY) bool Device::ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
{ {
if(dstRect.x0 < clipRect.x0) if(dstRect.x0 < clipRect.x0)
{ {
float offset = (static_cast<float>(clipRect.x0 - dstRect.x0) / static_cast<float>(dstRect.width())) * srcRect.width(); float offset = (static_cast<float>(clipRect.x0 - dstRect.x0) / static_cast<float>(dstRect.width())) * srcRect.width();
if (!std::isfinite(offset))
{
return false;
}
if(flipX) if(flipX)
{ {
srcRect.x1 -= offset; srcRect.x1 -= offset;
...@@ -911,6 +922,10 @@ namespace es2 ...@@ -911,6 +922,10 @@ namespace es2
if(dstRect.x1 > clipRect.x1) if(dstRect.x1 > clipRect.x1)
{ {
float offset = (static_cast<float>(dstRect.x1 - clipRect.x1) / static_cast<float>(dstRect.width())) * srcRect.width(); float offset = (static_cast<float>(dstRect.x1 - clipRect.x1) / static_cast<float>(dstRect.width())) * srcRect.width();
if (!std::isfinite(offset))
{
return false;
}
if(flipX) if(flipX)
{ {
srcRect.x0 += offset; srcRect.x0 += offset;
...@@ -924,6 +939,10 @@ namespace es2 ...@@ -924,6 +939,10 @@ namespace es2
if(dstRect.y0 < clipRect.y0) if(dstRect.y0 < clipRect.y0)
{ {
float offset = (static_cast<float>(clipRect.y0 - dstRect.y0) / static_cast<float>(dstRect.height())) * srcRect.height(); float offset = (static_cast<float>(clipRect.y0 - dstRect.y0) / static_cast<float>(dstRect.height())) * srcRect.height();
if (!std::isfinite(offset))
{
return false;
}
if(flipY) if(flipY)
{ {
srcRect.y1 -= offset; srcRect.y1 -= offset;
...@@ -937,6 +956,10 @@ namespace es2 ...@@ -937,6 +956,10 @@ namespace es2
if(dstRect.y1 > clipRect.y1) if(dstRect.y1 > clipRect.y1)
{ {
float offset = (static_cast<float>(dstRect.y1 - clipRect.y1) / static_cast<float>(dstRect.height())) * srcRect.height(); float offset = (static_cast<float>(dstRect.y1 - clipRect.y1) / static_cast<float>(dstRect.height())) * srcRect.height();
if (!std::isfinite(offset))
{
return false;
}
if(flipY) if(flipY)
{ {
srcRect.y0 += offset; srcRect.y0 += offset;
...@@ -947,14 +970,19 @@ namespace es2 ...@@ -947,14 +970,19 @@ namespace es2
} }
dstRect.y1 = clipRect.y1; dstRect.y1 = clipRect.y1;
} }
return true;
} }
void Device::ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY) bool Device::ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
{ {
if(srcRect.x0 < static_cast<float>(clipRect.x0)) if(srcRect.x0 < static_cast<float>(clipRect.x0))
{ {
float ratio = static_cast<float>(dstRect.width()) / srcRect.width(); float ratio = static_cast<float>(dstRect.width()) / srcRect.width();
float offsetf = roundf((static_cast<float>(clipRect.x0) - srcRect.x0) * ratio); float offsetf = roundf((static_cast<float>(clipRect.x0) - srcRect.x0) * ratio);
if (!std::isfinite(offsetf) || !std::isfinite(ratio))
{
return false;
}
int offset = static_cast<int>(offsetf); int offset = static_cast<int>(offsetf);
if(flipX) if(flipX)
{ {
...@@ -970,6 +998,10 @@ namespace es2 ...@@ -970,6 +998,10 @@ namespace es2
{ {
float ratio = static_cast<float>(dstRect.width()) / srcRect.width(); float ratio = static_cast<float>(dstRect.width()) / srcRect.width();
float offsetf = roundf((srcRect.x1 - static_cast<float>(clipRect.x1)) * ratio); float offsetf = roundf((srcRect.x1 - static_cast<float>(clipRect.x1)) * ratio);
if (!std::isfinite(offsetf) || !std::isfinite(ratio))
{
return false;
}
int offset = static_cast<int>(offsetf); int offset = static_cast<int>(offsetf);
if(flipX) if(flipX)
{ {
...@@ -985,6 +1017,10 @@ namespace es2 ...@@ -985,6 +1017,10 @@ namespace es2
{ {
float ratio = static_cast<float>(dstRect.height()) / srcRect.height(); float ratio = static_cast<float>(dstRect.height()) / srcRect.height();
float offsetf = roundf((static_cast<float>(clipRect.y0) - srcRect.y0) * ratio); float offsetf = roundf((static_cast<float>(clipRect.y0) - srcRect.y0) * ratio);
if (!std::isfinite(offsetf) || !std::isfinite(ratio))
{
return false;
}
int offset = static_cast<int>(offsetf); int offset = static_cast<int>(offsetf);
if(flipY) if(flipY)
{ {
...@@ -1000,6 +1036,10 @@ namespace es2 ...@@ -1000,6 +1036,10 @@ namespace es2
{ {
float ratio = static_cast<float>(dstRect.height()) / srcRect.height(); float ratio = static_cast<float>(dstRect.height()) / srcRect.height();
float offsetf = roundf((srcRect.y1 - static_cast<float>(clipRect.y1)) * ratio); float offsetf = roundf((srcRect.y1 - static_cast<float>(clipRect.y1)) * ratio);
if (!std::isfinite(offsetf) || !std::isfinite(ratio))
{
return false;
}
int offset = static_cast<int>(offsetf); int offset = static_cast<int>(offsetf);
if(flipY) if(flipY)
{ {
...@@ -1011,6 +1051,7 @@ namespace es2 ...@@ -1011,6 +1051,7 @@ namespace es2
} }
srcRect.y1 -= offsetf / ratio; srcRect.y1 -= offsetf / ratio;
} }
return true;
} }
void Device::finish() void Device::finish()
......
...@@ -75,8 +75,8 @@ namespace es2 ...@@ -75,8 +75,8 @@ namespace es2
bool stretchCube(sw::Surface *sourceSurface, sw::Surface *destSurface); bool stretchCube(sw::Surface *sourceSurface, sw::Surface *destSurface);
void finish(); void finish();
static void ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false); static bool ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
static void ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false); static bool ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
private: private:
sw::Context *const context; sw::Context *const context;
......
...@@ -1760,9 +1760,6 @@ TEST_F(SwiftShaderTest, BlitTest) ...@@ -1760,9 +1760,6 @@ TEST_F(SwiftShaderTest, BlitTest)
EXPECT_EQ(red[0][1], black[0][1]); EXPECT_EQ(red[0][1], black[0][1]);
// Check that glBlitFramebuffer doesn't crash with ugly input. // Check that glBlitFramebuffer doesn't crash with ugly input.
glBlitFramebuffer(-2, -2, 127, 2147483470, 10, 10, 200, 200, GL_COLOR_BUFFER_BIT, GL_NEAREST);
EXPECT_NO_GL_ERROR();
const int big = (int) 2e9; const int big = (int) 2e9;
const int small = 200; const int small = 200;
const int neg_small = -small; const int neg_small = -small;
...@@ -1774,6 +1771,7 @@ TEST_F(SwiftShaderTest, BlitTest) ...@@ -1774,6 +1771,7 @@ TEST_F(SwiftShaderTest, BlitTest)
{1, 1, 1, 1, 1, 1, 1, 1}, {1, 1, 1, 1, 1, 1, 1, 1},
{-1, -1, 1, 1, -1, -1, 1, 1}, {-1, -1, 1, 1, -1, -1, 1, 1},
{0, 0, 127, (int) 2e9, 10, 10, 200, 200}, {0, 0, 127, (int) 2e9, 10, 10, 200, 200},
{-2, -2, 127, 2147483470, 10, 10, 200, 200},
{big, small, small, big, big, big, small, small}, {big, small, small, big, big, big, small, small},
{neg_small, small, neg_small, neg_small, neg_small, big, small}, {neg_small, small, neg_small, neg_small, neg_small, big, small},
{big, big-1, big-2, big-3, big-4, big-5, big-6, big-7}, {big, big-1, big-2, big-3, big-4, big-5, big-6, big-7},
...@@ -1787,9 +1785,12 @@ TEST_F(SwiftShaderTest, BlitTest) ...@@ -1787,9 +1785,12 @@ TEST_F(SwiftShaderTest, BlitTest)
data[i][0], data[i][1], data[i][2], data[i][3], data[i][0], data[i][1], data[i][2], data[i][3],
data[i][4], data[i][5], data[i][6], data[i][7], data[i][4], data[i][5], data[i][6], data[i][7],
GL_COLOR_BUFFER_BIT, GL_NEAREST); GL_COLOR_BUFFER_BIT, GL_NEAREST);
EXPECT_NO_GL_ERROR(); // Ignore error state, just make sure that we don't crash on these inputs.
} }
// Clear the error state before uninitializing test.
glGetError();
glDeleteFramebuffers(2, fbos); glDeleteFramebuffers(2, fbos);
glDeleteTextures(2, textures); glDeleteTextures(2, textures);
Uninitialize(); Uninitialize();
......
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