Fix clearing of dirty textures. When clear operations fall back to the slow path (i.e. neither fastClear nor blitReactor is used), we were copying a rectangle the size of the destination image. It should only sample within the 1x1 source pixel instead. Bug chromium:852641, chromium:851707 Change-Id: I9f247483f6167f92be8308b8470c021f5641b657 Reviewed-on: https://swiftshader-review.googlesource.com/19448 Reviewed-by: Alexis Hétu <sugoi@google.com> Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Renderer/Blitter.cpp b/src/Renderer/Blitter.cpp index f68c7c8..4245cd4 100644 --- a/src/Renderer/Blitter.cpp +++ b/src/Renderer/Blitter.cpp
@@ -39,7 +39,7 @@ } sw::Surface *color = sw::Surface::create(1, 1, 1, format, pixel, sw::Surface::bytes(format), sw::Surface::bytes(format)); - SliceRectF sRect((float)dRect.x0, (float)dRect.y0, (float)dRect.x1, (float)dRect.y1, 0); + SliceRectF sRect(0.5f, 0.5f, 0.5f, 0.5f, 0); // Sample from the middle. blit(color, sRect, dest, dRect, {rgbaMask}); delete color; }
diff --git a/src/Renderer/Surface.cpp b/src/Renderer/Surface.cpp index 83412a3..5fc506d 100644 --- a/src/Renderer/Surface.cpp +++ b/src/Renderer/Surface.cpp
@@ -44,6 +44,8 @@ void Surface::Buffer::write(int x, int y, int z, const Color<float> &color) { + ASSERT(x < width && y < height && z < depth); + byte *element = (byte*)buffer + (x + border) * bytes + (y + border) * pitchB + z * samples * sliceB; for(int i = 0; i < samples; i++) @@ -55,6 +57,8 @@ void Surface::Buffer::write(int x, int y, const Color<float> &color) { + ASSERT(x < width && y < height); + byte *element = (byte*)buffer + (x + border) * bytes + (y + border) * pitchB; for(int i = 0; i < samples; i++) @@ -396,6 +400,8 @@ Color<float> Surface::Buffer::read(int x, int y, int z) const { + ASSERT(x < width && y < height && z < depth); + void *element = (unsigned char*)buffer + (x + border) * bytes + (y + border) * pitchB + z * samples * sliceB; return read(element); @@ -403,6 +409,8 @@ Color<float> Surface::Buffer::read(int x, int y) const { + ASSERT(x < width && y < height); + void *element = (unsigned char*)buffer + (x + border) * bytes + (y + border) * pitchB; return read(element);
diff --git a/tests/unittests/unittests.cpp b/tests/unittests/unittests.cpp index 6bef144..40f1c59 100644 --- a/tests/unittests/unittests.cpp +++ b/tests/unittests/unittests.cpp
@@ -49,10 +49,22 @@ #endif } - void compareColor(unsigned char referenceColor[4]) + void expectFramebufferColor(const unsigned char referenceColor[4], GLint x = 0, GLint y = 0) { unsigned char color[4] = { 0 }; - glReadPixels(0, 0, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &color); + glReadPixels(x, y, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &color); + EXPECT_GLENUM_EQ(GL_NONE, glGetError()); + EXPECT_EQ(color[0], referenceColor[0]); + EXPECT_EQ(color[1], referenceColor[1]); + EXPECT_EQ(color[2], referenceColor[2]); + EXPECT_EQ(color[3], referenceColor[3]); + } + + void expectFramebufferColor(const float referenceColor[4], GLint x = 0, GLint y = 0) + { + float color[4] = { 0 }; + glReadPixels(x, y, 1, 1, GL_RGBA, GL_FLOAT, &color); + EXPECT_GLENUM_EQ(GL_NONE, glGetError()); EXPECT_EQ(color[0], referenceColor[0]); EXPECT_EQ(color[1], referenceColor[1]); EXPECT_EQ(color[2], referenceColor[2]); @@ -383,7 +395,7 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -448,7 +460,7 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -505,13 +517,44 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); Uninitialize(); } +// Tests clearing of a texture with 'dirty' content. +TEST_F(SwiftShaderTest, ClearDirtyTexture) +{ + Initialize(3, false); + + GLuint tex = 1; + glBindTexture(GL_TEXTURE_2D, tex); + glTexImage2D(GL_TEXTURE_2D, 0, GL_R11F_G11F_B10F, 256, 256, 0, GL_RGB, GL_UNSIGNED_INT_10F_11F_11F_REV, nullptr); + EXPECT_GLENUM_EQ(GL_NONE, glGetError()); + + GLuint fbo = 1; + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0); + EXPECT_GLENUM_EQ(GL_NONE, glGetError()); + EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER)); + + float dirty_color[3] = { 128 / 255.0f, 64 / 255.0f, 192 / 255.0f }; + GLint dirty_x = 8; + GLint dirty_y = 12; + glTexSubImage2D(GL_TEXTURE_2D, 0, dirty_x, dirty_y, 1, 1, GL_RGB, GL_FLOAT, dirty_color); + + const float clear_color[4] = { 1.0f, 32.0f, 0.5f, 1.0f }; + glClearColor(clear_color[0], clear_color[1], clear_color[2], 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + EXPECT_GLENUM_EQ(GL_NONE, glGetError()); + + expectFramebufferColor(clear_color, dirty_x, dirty_y); + + Uninitialize(); +} + // Tests construction of a structure containing a single matrix TEST_F(SwiftShaderTest, MatrixInStruct) { @@ -593,7 +636,7 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -647,7 +690,7 @@ deleteProgram(ph); unsigned char grey[4] = { 128, 128, 128, 128 }; - compareColor(grey); + expectFramebufferColor(grey); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -910,7 +953,7 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -964,7 +1007,7 @@ deleteProgram(ph); - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); @@ -992,7 +1035,7 @@ glClear(GL_COLOR_BUFFER_BIT); unsigned char green[4] = { 0, 255, 0, 255 }; - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); Uninitialize(); @@ -1046,7 +1089,7 @@ glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_RECTANGLE_ARB, tex, 0); unsigned char green[4] = { 0, 255, 0, 255 }; - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); Uninitialize(); @@ -1080,7 +1123,7 @@ glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_RECTANGLE_ARB, tex, 0); unsigned char green[4] = { 0, 255, 0, 255 }; - compareColor(green); + expectFramebufferColor(green); EXPECT_GLENUM_EQ(GL_NONE, glGetError()); Uninitialize();