From 9cd785e1c4d6bd7eb0380791dee525fef8a396b2 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 6 Sep 2019 17:12:06 +0000 Subject: [PATCH] Bug 1578045 - Correctly return zero vertices if clipping plane 0 or 2 clip away the entire polygon. r=kip This fixes a bug that was introduced three years ago in bug 1268854. What happened was that the final pass over the polygon assumed that the current polygon was living in plane[0]. But due to the double buffering, the "current" polygon alternates between plane[0] and plane[1]. And bug 1268854 had introduced an early exit so that we could hit the final pass at a time where the current, now empty, polygon was in plane[1]. So we would incorrectly treat all 32 points in plane[0] as part of the final polygon. This bug was responsible for intermittently unreasonable numbers in CompositorOGL's fill rate / overdraw overlay, and, since changeset cc84a0e9d5ddde198422f4f11ab6bf85f631d5f0, also caused CompositorOGL to execute unnecessary draw calls. Differential Revision: https://phabricator.services.mozilla.com/D44312 --HG-- extra : moz-landing-system : lando --- gfx/2d/Matrix.h | 42 +++++++++++++++-------- gfx/tests/gtest/TestMatrix.cpp | 61 ++++++++++++++++++++++++++++++++++ gfx/tests/gtest/moz.build | 1 + 3 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 gfx/tests/gtest/TestMatrix.cpp diff --git a/gfx/2d/Matrix.h b/gfx/2d/Matrix.h index 4d82409664a5..a80461384461 100644 --- a/gfx/2d/Matrix.h +++ b/gfx/2d/Matrix.h @@ -823,7 +823,7 @@ class Matrix4x4Typed { * The resulting vertices are populated in aVerts. aVerts must be * pre-allocated to hold at least kTransformAndClipRectMaxVerts Points. * The vertex count is returned by TransformAndClipRect. It is possible to - * emit fewer that 3 vertices, indicating that aRect will not be visible + * emit fewer than 3 vertices, indicating that aRect will not be visible * within aClip. */ template @@ -833,7 +833,8 @@ class Matrix4x4Typed { // Initialize a double-buffered array of points in homogenous space with // the input rectangle, aRect. Point4DTyped points[2][kTransformAndClipRectMaxVerts]; - Point4DTyped* dstPoint = points[0]; + Point4DTyped* dstPointStart = points[0]; + Point4DTyped* dstPoint = dstPointStart; *dstPoint++ = TransformPoint( Point4DTyped(aRect.X(), aRect.Y(), 0, 1)); @@ -855,16 +856,26 @@ class Matrix4x4Typed { Point4DTyped(0.0, -1.0, 0.0, aClip.YMost()); // Iterate through each clipping plane and clip the polygon. - // In each pass, we double buffer, alternating between points[0] and - // points[1]. + // For each clipping plane, we intersect the plane with all polygon edges. + // Each pass can increase or decrease the number of points that make up the + // current clipped polygon. We double buffer that set of points, alternating + // between points[0] and points[1]. for (int plane = 0; plane < 4; plane++) { planeNormals[plane].Normalize(); - Point4DTyped* srcPoint = points[plane & 1]; + Point4DTyped* srcPoint = dstPointStart; Point4DTyped* srcPointEnd = dstPoint; - dstPoint = points[~plane & 1]; - Point4DTyped* dstPointStart = dstPoint; + dstPointStart = points[~plane & 1]; + dstPoint = dstPointStart; + // Iterate over the polygon edges. In each iteration the current edge is + // the edge from prevPoint to srcPoint. If the two end points lie on + // different sides of the plane, we have an intersection. Otherwise, the + // edge is either completely "inside" the half-space created by the + // clipping plane, and we add srcPoint, or it is completely "outside", and + // we discard srcPoint. + // We may create duplicated points in the polygon. We keep those around + // until all clipping is done and then filter out duplicates at the end. Point4DTyped* prevPoint = srcPointEnd - 1; F prevDot = planeNormals[plane].DotProduct(*prevPoint); while (srcPoint < srcPointEnd && @@ -889,14 +900,16 @@ class Matrix4x4Typed { } if (dstPoint == dstPointStart) { + // No polygon points were produced, so the polygon has been + // completely clipped away by the current clipping plane. Exit. break; } } - size_t dstPointCount = 0; - size_t srcPointCount = dstPoint - points[0]; - for (Point4DTyped* srcPoint = points[0]; - srcPoint < points[0] + srcPointCount; srcPoint++) { + Point4DTyped* srcPoint = dstPointStart; + Point4DTyped* srcPointEnd = dstPoint; + size_t vertCount = 0; + while (srcPoint < srcPointEnd) { PointTyped p; if (srcPoint->w == 0.0) { // If a point lies on the intersection of the clipping planes at @@ -906,12 +919,13 @@ class Matrix4x4Typed { p = srcPoint->As2DPoint(); } // Emit only unique points - if (dstPointCount == 0 || p != aVerts[dstPointCount - 1]) { - aVerts[dstPointCount++] = p; + if (vertCount == 0 || p != aVerts[vertCount - 1]) { + aVerts[vertCount++] = p; } + srcPoint++; } - return dstPointCount; + return vertCount; } static const int kTransformAndClipRectMaxVerts = 32; diff --git a/gfx/tests/gtest/TestMatrix.cpp b/gfx/tests/gtest/TestMatrix.cpp new file mode 100644 index 000000000000..bc2f9e63cd2e --- /dev/null +++ b/gfx/tests/gtest/TestMatrix.cpp @@ -0,0 +1,61 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 "gtest/gtest.h" +#include "mozilla/gfx/Matrix.h" + +using namespace mozilla; +using namespace mozilla::gfx; + +static Rect NudgedToInt(const Rect& aRect) { + Rect r(aRect); + r.NudgeToIntegers(); + return r; +} + +TEST(Matrix, TransformAndClipRect) +{ + Rect c(100, 100, 100, 100); + Matrix4x4 m; + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 50, 20, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(250, 50, 20, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(250, 250, 20, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 250, 20, 20), c).IsEmpty()); + + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 50, 100, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(150, 50, 100, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 250, 100, 20), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(150, 250, 100, 20), c).IsEmpty()); + + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 50, 20, 100), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(50, 150, 20, 100), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(250, 50, 20, 100), c).IsEmpty()); + EXPECT_TRUE(m.TransformAndClipBounds(Rect(250, 150, 20, 100), c).IsEmpty()); + + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 50, 100, 100), c)) + .IsEqualInterior(Rect(100, 100, 50, 50))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(150, 50, 100, 100), c)) + .IsEqualInterior(Rect(150, 100, 50, 50))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(150, 150, 100, 100), c)) + .IsEqualInterior(Rect(150, 150, 50, 50))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 150, 100, 100), c)) + .IsEqualInterior(Rect(100, 150, 50, 50))); + + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(110, 110, 80, 80), c)) + .IsEqualInterior(Rect(110, 110, 80, 80))); + + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 50, 200, 200), c)) + .IsEqualInterior(Rect(100, 100, 100, 100))); + + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 50, 200, 100), c)) + .IsEqualInterior(Rect(100, 100, 100, 50))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 150, 200, 100), c)) + .IsEqualInterior(Rect(100, 150, 100, 50))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(50, 50, 100, 200), c)) + .IsEqualInterior(Rect(100, 100, 50, 100))); + EXPECT_TRUE(NudgedToInt(m.TransformAndClipBounds(Rect(150, 50, 100, 200), c)) + .IsEqualInterior(Rect(150, 100, 50, 100))); +} diff --git a/gfx/tests/gtest/moz.build b/gfx/tests/gtest/moz.build index 33352a0016be..927e247a3a9a 100644 --- a/gfx/tests/gtest/moz.build +++ b/gfx/tests/gtest/moz.build @@ -16,6 +16,7 @@ UNIFIED_SOURCES += [ 'TestGfxWidgets.cpp', 'TestJobScheduler.cpp', 'TestLayers.cpp', + 'TestMatrix.cpp', 'TestMoz2D.cpp', 'TestPolygon.cpp', 'TestQcms.cpp',