Correctly return zero vertices if clipping plane 0 or 2 clip away the

entire polygon.

This fixes a bug that was introduced three years ago in BZ 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].
The bug had also 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.

This fixes a regression caused by the fix for CVE-2016-5252.

3 files changed, 91 insertions, 15 deletions

diff --git a/gfx/2d/Matrix.h b/gfx/2d/Matrix.h
index 22a01ca10..d6835c8e6 100644
--- a/gfx/2d/Matrix.h
+++ b/gfx/2d/Matrix.h
@@ -723,7 +723,7 @@ public:
    * 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<class F>
@@ -734,7 +734,8 @@ public:
     // Initialize a double-buffered array of points in homogenous space with
     // the input rectangle, aRect.
     Point4DTyped<UnknownUnits, F> points[2][kTransformAndClipRectMaxVerts];
-    Point4DTyped<UnknownUnits, F>* dstPoint = points[0];
+    Point4DTyped<UnknownUnits, F>* dstPointStart = points[0];
+    Point4DTyped<UnknownUnits, F>* dstPoint = dstPointStart;
 
     *dstPoint++ = TransformPoint(Point4DTyped<UnknownUnits, F>(aRect.x, aRect.y, 0, 1));
     *dstPoint++ = TransformPoint(Point4DTyped<UnknownUnits, F>(aRect.XMost(), aRect.y, 0, 1));
@@ -750,16 +751,26 @@ public:
     planeNormals[3] = Point4DTyped<UnknownUnits, F>(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<UnknownUnits, F>* srcPoint = points[plane & 1];
+      Point4DTyped<UnknownUnits, F>* srcPoint = dstPointStart;
       Point4DTyped<UnknownUnits, F>* srcPointEnd = dstPoint;
 
-      dstPoint = points[~plane & 1];
-      Point4DTyped<UnknownUnits, F>* 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<UnknownUnits, F>* prevPoint = srcPointEnd - 1;
       F prevDot = planeNormals[plane].DotProduct(*prevPoint);
       while (srcPoint < srcPointEnd && ((dstPoint - dstPointStart) < kTransformAndClipRectMaxVerts)) {
@@ -783,14 +794,16 @@ public:
       }
 
       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<UnknownUnits, F>* srcPoint = points[0]; srcPoint < points[0] + srcPointCount; srcPoint++) {
-
+    Point4DTyped<UnknownUnits, F>* srcPoint = dstPointStart;
+    Point4DTyped<UnknownUnits, F>* srcPointEnd = dstPoint;
+    size_t vertCount = 0;
+    while (srcPoint < srcPointEnd) {
       PointTyped<TargetUnits, F> p;
       if (srcPoint->w == 0.0) {
         // If a point lies on the intersection of the clipping planes at
@@ -800,12 +813,13 @@ public:
         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 000000000..bc2f9e63c
--- /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 23b019d1b..ea18c1e3b 100644
--- a/gfx/tests/gtest/moz.build
+++ b/gfx/tests/gtest/moz.build
@@ -17,6 +17,7 @@ UNIFIED_SOURCES += [
     'TestGfxWidgets.cpp',
     'TestJobScheduler.cpp',
     'TestLayers.cpp',
+    'TestMatrix.cpp',
     'TestMoz2D.cpp',
     'TestPolygon.cpp',
     'TestQcms.cpp',