These changes prepare BSPTree, BSPTree tests, and Polygon3D for integration with the layers code. I apologize for the large changeset, these classes are tightly-coupled, so splitting the patch is very difficult. Here is a list of changes: TestBSPTree.cpp --------------- - Fix inverted logic in FuzzyCompare and rename it to FuzzyEquals. - Add tests to verify that polygon/point comparison works properly. - Fix tests that broke from surface normal change. - Refactor tests to use the new LayerPolygon struct. BSPTree.cpp / BSPTree.h ----------------------- - Add new LayerPolygon struct to represent layers with arbitrary, convex geometry. - Refactor the code to use LayerPolygon instead of Polygon3D. Polygon.h --------- - Add TransformToLayerSpace() and TransformToScreenSpace() functions to transform polygon vertices. - Change how the polygon surface normal is calculated: Now the initial surface normal is defined during the construction. The surface normal is transformed along with the polygon (by using inverse transpose of the transform). - Remove CalculateNormal() as obsolete. - Add EnsurePlanarPolygon() for debug builds.

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code https://reviewboard.mozilla.org/r/78030/#review77414 ::: gfx/2d/Polygon.h:91 (Diff revision 3) > - for (size_t i = 2; i < mPoints.Length() - 1; ++i) { > - mNormal += (mPoints[i] - mPoints[0]).CrossProduct(mPoints[i + 1] - mPoints[0]); > - } > } > > - mNormal.Normalize(); > + normal.Normalize(); If the length of the normal vector here is 0, the Normalize() function may divide by zero. A safety check that rejects this polygon rather than crashing could be useful here. Perhaps check that at least one member is < -epsilon or > epsilon ? ::: gfx/layers/BSPTree.h:38 (Diff revision 3) > + > +// Represents a node in a BSP tree. The node contains at least one layer with > +// associated geometry that is used as a splitting plane, and at most two child > +// nodes that represent the splitting planes that further subdivide the space. > struct BSPTreeNode { > - explicit BSPTreeNode(gfx::Polygon3D && aPolygon) > + explicit BSPTreeNode(LayerPolygon && layer) nit: Remove space between double-address operator and LayerPolygon

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code https://reviewboard.mozilla.org/r/78030/#review77414 This is looking good, thanks! r=me with those two fixes.

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code r=me with the two changes in MozReview.

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code https://reviewboard.mozilla.org/r/78028/#review78264

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code https://reviewboard.mozilla.org/r/78028/#review78266

Comment on attachment 8790054 [details] Bug 1301818 - Prepare BSPTree for integration with the layers code https://reviewboard.mozilla.org/r/78030/#review78268 Ship it!

Pushed by kgilbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/974defd493a3 Prepare BSPTree for integration with the layers code r=kip