From c77f66ee40ac110b2cd02b81f4521029d4d707f9 Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Fri, 18 Jul 2014 14:25:34 -0400 Subject: [PATCH] Bug 1023473. Pad out tile contents by one pixel. r=BenWa This fixes reading unitialized data during texture filtering. It's complicated because we can have regions of initialized data and padding out a region is much harder than a rectangle. We currently only take this path for tiled things because of the artifacts that show up during the sampling that helps in overscroll. It's also not a great long term solution because it only works for software backends. --- gfx/layers/client/TiledContentClient.cpp | 84 +++++++++++ gfx/src/nsRegion.cpp | 164 ++++++++++++++++++++ gfx/src/nsRegion.h | 29 ++++ gfx/tests/gtest/TestRegion.cpp | 182 +++++++++++++++++++++++ 4 files changed, 459 insertions(+) diff --git a/gfx/layers/client/TiledContentClient.cpp b/gfx/layers/client/TiledContentClient.cpp index 047b403836d5..9d0130fca0a3 100644 --- a/gfx/layers/client/TiledContentClient.cpp +++ b/gfx/layers/client/TiledContentClient.cpp @@ -5,6 +5,7 @@ #include "mozilla/layers/TiledContentClient.h" #include // for ceil, ceilf, floor +#include #include "ClientTiledThebesLayer.h" // for ClientTiledThebesLayer #include "GeckoProfiler.h" // for PROFILER_LABEL #include "ClientLayerManager.h" // for ClientLayerManager @@ -16,6 +17,7 @@ #include "mozilla/MathAlgorithms.h" // for Abs #include "mozilla/gfx/Point.h" // for IntSize #include "mozilla/gfx/Rect.h" // for Rect +#include "mozilla/gfx/Tools.h" // for BytesPerPixel #include "mozilla/layers/CompositableForwarder.h" #include "mozilla/layers/ShadowLayers.h" // for ShadowLayerForwarder #include "TextureClientPool.h" @@ -813,6 +815,68 @@ ClientTiledLayerBuffer::PaintThebes(const nsIntRegion& aNewValidRegion, mSinglePaintDrawTarget = nullptr; } +void PadDrawTargetOutFromRegion(RefPtr drawTarget, nsIntRegion ®ion) +{ + struct LockedBits { + uint8_t *data; + IntSize size; + int32_t stride; + SurfaceFormat format; + static int clamp(int x, int min, int max) + { + if (x < min) + x = min; + if (x > max) + x = max; + return x; + } + + static void visitor(void *closure, VisitSide side, int x1, int y1, int x2, int y2) { + LockedBits *lb = static_cast(closure); + uint8_t *bitmap = lb->data; + const int bpp = gfx::BytesPerPixel(lb->format); + const int stride = lb->stride; + const int width = lb->size.width; + const int height = lb->size.height; + + if (side == VisitSide::TOP) { + if (y1 > 0) { + x1 = clamp(x1, 0, width - 1); + x2 = clamp(x2, 0, width - 1); + memcpy(&bitmap[x1*bpp + (y1-1) * stride], &bitmap[x1*bpp + y1 * stride], (x2 - x1) * bpp); + } + } else if (side == VisitSide::BOTTOM) { + if (y1 < height) { + x1 = clamp(x1, 0, width - 1); + x2 = clamp(x2, 0, width - 1); + memcpy(&bitmap[x1*bpp + y1 * stride], &bitmap[x1*bpp + (y1-1) * stride], (x2 - x1) * bpp); + } + } else if (side == VisitSide::LEFT) { + if (x1 > 0) { + while (y1 != y2) { + memcpy(&bitmap[(x1-1)*bpp + y1 * stride], &bitmap[x1*bpp + y1*stride], bpp); + y1++; + } + } + } else if (side == VisitSide::RIGHT) { + if (x1 < width) { + while (y1 != y2) { + memcpy(&bitmap[x1*bpp + y1 * stride], &bitmap[(x1-1)*bpp + y1*stride], bpp); + y1++; + } + } + } + + } + } lb; + + if (drawTarget->LockBits(&lb.data, &lb.size, &lb.stride, &lb.format)) { + // we can only pad software targets so if we can't lock the bits don't pad + region.VisitEdges(lb.visitor, &lb); + drawTarget->ReleaseBits(lb.data); + } +} + TileClient ClientTiledLayerBuffer::ValidateTile(TileClient aTile, const nsIntPoint& aTileOrigin, @@ -890,6 +954,26 @@ ClientTiledLayerBuffer::ValidateTile(TileClient aTile, aTile.mInvalidFront.Or(aTile.mInvalidFront, nsIntRect(copyTarget.x, copyTarget.y, copyRect.width, copyRect.height)); } + // only worry about padding when not doing low-res + // because it simplifies the math and the artifacts + // won't be noticable + if (mResolution == 1) { + nsIntRect unscaledTile = nsIntRect(aTileOrigin.x, + aTileOrigin.y, + GetTileSize().width, + GetTileSize().height); + + nsIntRegion tileValidRegion = GetValidRegion(); + tileValidRegion.Or(tileValidRegion, aDirtyRegion); + // We only need to pad out if the tile has area that's not valid + if (!tileValidRegion.Contains(unscaledTile)) { + tileValidRegion = tileValidRegion.Intersect(unscaledTile); + // translate the region into tile space and pad + tileValidRegion.MoveBy(-nsIntPoint(unscaledTile.x, unscaledTile.y)); + PadDrawTargetOutFromRegion(drawTarget, tileValidRegion); + } + } + // The new buffer is now validated, remove the dirty region from it. aTile.mInvalidBack.Sub(nsIntRect(0, 0, GetTileSize().width, GetTileSize().height), offsetScaledDirtyRegion); diff --git a/gfx/src/nsRegion.cpp b/gfx/src/nsRegion.cpp index 0b8d755ca44c..52c48622c6d1 100644 --- a/gfx/src/nsRegion.cpp +++ b/gfx/src/nsRegion.cpp @@ -370,6 +370,170 @@ void nsRegion::SimplifyOutwardByArea(uint32_t aThreshold) } +typedef void (*visit_fn)(void *closure, VisitSide side, int x1, int y1, int x2, int y2); + +static void VisitNextEdgeBetweenRect(visit_fn visit, void *closure, VisitSide side, + pixman_box32_t *&r1, pixman_box32_t *&r2, const int y, int &x1) +{ + // check for overlap + if (r1->x2 >= r2->x1) { + MOZ_ASSERT(r2->x1 >= x1); + visit(closure, side, x1, y, r2->x1, y); + + // find the rect that ends first or always drop the one that comes first? + if (r1->x2 < r2->x2) { + x1 = r1->x2; + r1++; + } else { + x1 = r2->x2; + r2++; + } + } else { + MOZ_ASSERT(r1->x2 < r2->x2); + // we handle the corners by just extending the top and bottom edges + visit(closure, side, x1, y, r1->x2+1, y); + r1++; + x1 = r2->x1 - 1; + } +} + +//XXX: if we need to this can compute the end of the row +static void +VisitSides(visit_fn visit, void *closure, pixman_box32_t *r, pixman_box32_t *r_end) +{ + // XXX: we can drop LEFT/RIGHT and just use the orientation + // of the line if it makes sense + while (r != r_end) { + visit(closure, VisitSide::LEFT, r->x1, r->y1, r->x1, r->y2); + visit(closure, VisitSide::RIGHT, r->x2, r->y1, r->x2, r->y2); + r++; + } +} + +static void +VisitAbove(visit_fn visit, void *closure, pixman_box32_t *r, pixman_box32_t *r_end) +{ + while (r != r_end) { + visit(closure, VisitSide::TOP, r->x1-1, r->y1, r->x2+1, r->y1); + r++; + } +} + +static void +VisitBelow(visit_fn visit, void *closure, pixman_box32_t *r, pixman_box32_t *r_end) +{ + while (r != r_end) { + visit(closure, VisitSide::BOTTOM, r->x1-1, r->y2, r->x2+1, r->y2); + r++; + } +} + +static pixman_box32_t * +VisitInbetween(visit_fn visit, void *closure, pixman_box32_t *r1, + pixman_box32_t *r1_end, + pixman_box32_t *r2, + pixman_box32_t *r2_end) +{ + const int y = r1->y2; + int x1; + + /* Find the left-most edge */ + if (r1->x1 < r2->x1) { + x1 = r1->x1 - 1; + } else { + x1 = r2->x1 - 1; + } + + while (r1 != r1_end && r2 != r2_end) { + MOZ_ASSERT((x1 >= (r1->x1 - 1)) || (x1 >= (r2->x1 - 1))); + if (r1->x1 < r2->x1) { + VisitNextEdgeBetweenRect(visit, closure, VisitSide::BOTTOM, r1, r2, y, x1); + } else { + VisitNextEdgeBetweenRect(visit, closure, VisitSide::TOP, r2, r1, y, x1); + } + } + + /* Finish up which ever row has remaining rects*/ + if (r1 != r1_end) { + // top row + do { + visit(closure, VisitSide::BOTTOM, x1, y, r1->x2 + 1, y); + r1++; + if (r1 == r1_end) + break; + x1 = r1->x1 - 1; + } while (true); + } else if (r2 != r2_end) { + // bottom row + do { + visit(closure, VisitSide::TOP, x1, y, r2->x2 + 1, y); + r2++; + if (r2 == r2_end) + break; + x1 = r2->x1 - 1; + } while (true); + } + + return 0; +} + +void nsRegion::VisitEdges (visit_fn visit, void *closure) +{ + pixman_box32_t *boxes; + int n; + boxes = pixman_region32_rectangles(&mImpl, &n); + + // if we have no rectangles then we're done + if (!n) + return; + + pixman_box32_t *end = boxes + n; + pixman_box32_t *topRectsEnd = boxes + 1; + pixman_box32_t *topRects = boxes; + + // find the end of the first span of rectangles + while (topRectsEnd < end && topRectsEnd->y1 == topRects->y1) { + topRectsEnd++; + } + + // In order to properly handle convex corners we always visit the sides first + // that way when we visit the corners we can pad using the value from the sides + VisitSides(visit, closure, topRects, topRectsEnd); + + VisitAbove(visit, closure, topRects, topRectsEnd); + + pixman_box32_t *bottomRects = topRects; + pixman_box32_t *bottomRectsEnd = topRectsEnd; + if (topRectsEnd != end) { + do { + // find the next row of rects + bottomRects = topRectsEnd; + bottomRectsEnd = topRectsEnd + 1; + while (bottomRectsEnd < end && bottomRectsEnd->y1 == bottomRects->y1) { + bottomRectsEnd++; + } + + VisitSides(visit, closure, bottomRects, bottomRectsEnd); + + if (topRects->y2 == bottomRects->y1) { + VisitInbetween(visit, closure, topRects, topRectsEnd, + bottomRects, bottomRectsEnd); + } else { + VisitBelow(visit, closure, topRects, topRectsEnd); + VisitAbove(visit, closure, bottomRects, bottomRectsEnd); + } + + topRects = bottomRects; + topRectsEnd = bottomRectsEnd; + } while (bottomRectsEnd != end); + } + + // the bottom of the region doesn't touch anything else so we + // can always visit it at the end + VisitBelow(visit, closure, bottomRects, bottomRectsEnd); +} + + void nsRegion::SimplifyInward (uint32_t aMaxRects) { NS_ASSERTION(aMaxRects >= 1, "Invalid max rect count"); diff --git a/gfx/src/nsRegion.h b/gfx/src/nsRegion.h index 455b07378dda..2ac782d7bb93 100644 --- a/gfx/src/nsRegion.h +++ b/gfx/src/nsRegion.h @@ -17,6 +17,7 @@ #include "nsMargin.h" // for nsIntMargin #include "nsStringGlue.h" // for nsCString #include "xpcom-config.h" // for CPP_THROW_NEW +#include "mozilla/TypedEnum.h" // for the VisitEdges typed enum class nsIntRegion; class gfx3DMatrix; @@ -37,6 +38,13 @@ class gfx3DMatrix; * projects including Qt, Gtk, Wine. It should perform reasonably well. */ +MOZ_BEGIN_ENUM_CLASS(VisitSide) + TOP, + BOTTOM, + LEFT, + RIGHT +MOZ_END_ENUM_CLASS(VisitSide) + class nsRegionRectIterator; class nsRegion @@ -264,6 +272,21 @@ public: */ void SimplifyInward (uint32_t aMaxRects); + /** + * VisitEdges is a weird kind of function that we use for padding + * out surfaces to prevent texture filtering artifacts. + * It calls the visitFn callback for each of the exterior edges of + * the regions. The top and bottom edges will be expanded 1 pixel + * to the left and right if there's an outside corner. The order + * the edges are visited is not guaranteed. + * + * visitFn has a side parameter that can be TOP,BOTTOM,LEFT,RIGHT + * and specifies which kind of edge is being visited. x1, y1, x2, y2 + * are the coordinates of the line. (x1 == x2) || (y1 == y2) + */ + typedef void (*visitFn)(void *closure, VisitSide side, int x1, int y1, int x2, int y2); + void VisitEdges(visitFn, void *closure); + nsCString ToString() const; private: pixman_region32_t mImpl; @@ -591,6 +614,12 @@ public: mImpl.SimplifyInward (aMaxRects); } + typedef void (*visitFn)(void *closure, VisitSide side, int x1, int y1, int x2, int y2); + void VisitEdges (visitFn visit, void *closure) + { + mImpl.VisitEdges (visit, closure); + } + nsCString ToString() const { return mImpl.ToString(); } private: diff --git a/gfx/tests/gtest/TestRegion.cpp b/gfx/tests/gtest/TestRegion.cpp index 444d4ca52d9f..488d98ed0d61 100644 --- a/gfx/tests/gtest/TestRegion.cpp +++ b/gfx/tests/gtest/TestRegion.cpp @@ -3,9 +3,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include + #include "gtest/gtest.h" #include "nsRegion.h" +using namespace std; + class TestLargestRegion { public: static void TestSingleRect(nsRect r) { @@ -172,6 +176,7 @@ TEST(Gfx, RegionScaleToInside) { } + TEST(Gfx, RegionSimplify) { { // ensure simplify works on a single rect nsRegion r(nsRect(0,100,200,100)); @@ -281,5 +286,182 @@ TEST(Gfx, RegionSimplify) { nsRegion r; r.SimplifyOutwardByArea(100); } +} + +#define DILATE_VALUE 0x88 +#define REGION_VALUE 0xff + +struct RegionBitmap { + RegionBitmap(unsigned char *bitmap, int width, int height) : bitmap(bitmap), width(width), height(height) {} + + void clear() { + for (int y = 0; y < height; y++) { + for (int x = 0; x < width; x++) { + bitmap[x + y * width] = 0; + } + } + } + + void set(nsRegion ®ion) { + clear(); + nsRegionRectIterator iter(region); + for (const nsRect* r = iter.Next(); r; r = iter.Next()) { + for (int y = r->y; y < r->YMost(); y++) { + for (int x = r->x; x < r->XMost(); x++) { + bitmap[x + y * width] = REGION_VALUE; + } + } + } + } + + void dilate() { + for (int y = 0; y < height; y++) { + for (int x = 0; x < width; x++) { + if (bitmap[x + y * width] == REGION_VALUE) { + for (int yn = max(y - 1, 0); yn <= min(y + 1, height - 1); yn++) { + for (int xn = max(x - 1, 0); xn <= min(x + 1, width - 1); xn++) { + if (bitmap[xn + yn * width] == 0) + bitmap[xn + yn * width] = DILATE_VALUE; + } + } + } + } + } + } + void compare(RegionBitmap &reference) { + for (int y = 0; y < height; y++) { + for (int x = 0; x < width; x++) { + EXPECT_EQ(bitmap[x + y * width], reference.bitmap[x + y * width]); + } + } + } + + unsigned char *bitmap; + int width; + int height; +}; + +void VisitEdge(void *closure, VisitSide side, int x1, int y1, int x2, int y2) +{ + RegionBitmap *visitor = static_cast(closure); + unsigned char *bitmap = visitor->bitmap; + const int width = visitor->width; + + if (side == VisitSide::TOP) { + while (x1 != x2) { + bitmap[x1 + (y1 - 1) * width] = DILATE_VALUE; + x1++; + } + } else if (side == VisitSide::BOTTOM) { + while (x1 != x2) { + bitmap[x1 + y1 * width] = DILATE_VALUE; + x1++; + } + } else if (side == VisitSide::LEFT) { + while (y1 != y2) { + bitmap[x1 - 1 + y1 *width] = DILATE_VALUE; + y1++; + } + } else if (side == VisitSide::RIGHT) { + while (y1 != y2) { + bitmap[x1 + y1 * width] = DILATE_VALUE; + y1++; + } + } +} + +void TestVisit(nsRegion &r) +{ + unsigned char reference[600 * 600]; + unsigned char result[600 * 600]; + RegionBitmap ref(reference, 600, 600); + RegionBitmap res(result, 600, 600); + + ref.set(r); + ref.dilate(); + + res.set(r); + r.VisitEdges(VisitEdge, &res); + res.compare(ref); +} + +TEST(Gfx, RegionVisitEdges) { + { // visit edges + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(20, 120, 200, 100)); + TestVisit(r); + } + + { // two rects side by side - 1 pixel inbetween + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(121, 20, 100, 100)); + TestVisit(r); + } + + { // two rects side by side - 2 pixels inbetween + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(122, 20, 100, 100)); + TestVisit(r); + } + + { + // only corner of the rects are touching + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(120, 120, 100, 100)); + + TestVisit(r); + } + + { + // corners are 1 pixel away + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(121, 120, 100, 100)); + + TestVisit(r); + } + + { + // vertically separated + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(120, 125, 100, 100)); + + TestVisit(r); + } + + { + // not touching + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(130, 120, 100, 100)); + r.Or(r, nsRect(240, 20, 100, 100)); + + TestVisit(r); + } + + { // rect with a hole in it + nsRegion r(nsRect(20, 20, 100, 100)); + r.Sub(r, nsRect(40, 40, 10, 10)); + + TestVisit(r); + } + { + // left overs + nsRegion r(nsRect(20, 20, 10, 10)); + r.Or(r, nsRect(50, 20, 10, 10)); + r.Or(r, nsRect(90, 20, 10, 10)); + r.Or(r, nsRect(24, 30, 10, 10)); + r.Or(r, nsRect(20, 40, 15, 10)); + r.Or(r, nsRect(50, 40, 15, 10)); + r.Or(r, nsRect(90, 40, 15, 10)); + + TestVisit(r); + } + + { + // vertically separated + nsRegion r(nsRect(20, 20, 100, 100)); + r.Or(r, nsRect(120, 125, 100, 100)); + + TestVisit(r); + } }