Perform BSP polygon splitting and orientation selection in a single step.

cc/quads/draw_polygon.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_QUADS_DRAW_POLYGON_H_
 #define CC_QUADS_DRAW_POLYGON_H_
 
 #include <vector>
 
 #include "cc/base/math_util.h"
@@ skipping 22 matching lines @@
               const gfx::Transform& transform,
               int draw_order_index = 0);
 
   // Split takes this DrawPolygon and splits it into two pieces that are on
   // either side of |splitter|. Any edges of this polygon that cross the plane
   // of |splitter| will have an intersection point that is shared by both
   // polygons on either side.
   // Split will only return true if it determines that we got back 2
   // intersection points. Only when it returns true will front and back both be
   // valid new polygons that are on opposite sides of the splitting plane.
-  bool Split(const DrawPolygon& splitter,
-             std::unique_ptr<DrawPolygon>* front,
-             std::unique_ptr<DrawPolygon>* back);
+  bool SplitPolygon(std::unique_ptr<DrawPolygon> polygon,

[enne (OOO), 2016/06/08 22:02:51]

Style nit: if you have more than one out parameter, then make them all
parameters and not some as out parameters and one as a return value.

I see you're using it for EXPECT calls, but it looks like you could write a test
helper that says bool IsCoplanar() and hides the front and back parameters
inside it, and checks that front and back are not null when a split occurs.

[Tobias Sargeant, 2016/06/09 16:48:44]

Done.

+                    std::unique_ptr<DrawPolygon>* front,
+                    std::unique_ptr<DrawPolygon>* back);
+  bool SplitPolygon(const DrawPolygon& polygon,

[enne (OOO), 2016/06/08 22:02:51]

This is a bit awkward.  How many test locations use this? Can you just make the
test call WrapUnique or move this function into tests?

[Tobias Sargeant, 2016/06/09 16:48:44]

Moved this into the tests.

+                    std::unique_ptr<DrawPolygon>* front,
+                    std::unique_ptr<DrawPolygon>* back);
   float SignedPointDistance(const gfx::Point3F& point) const;
   // Checks polygon a against polygon b and returns which side it lies on, or
   // whether it crosses (necessitating a split in the BSP tree).
   static BspCompareResult SideCompare(const DrawPolygon& a,
                                       const DrawPolygon& b);
   void ToQuads2D(std::vector<gfx::QuadF>* quads) const;
   void TransformToScreenSpace(const gfx::Transform& transform);
   void TransformToLayerSpace(const gfx::Transform& inverse_transform);
 
   const std::vector<gfx::Point3F>& points() const { return points_; }
@@ skipping 22 matching lines @@
   // we need.
   // This DrawQuad is owned by the caller and its lifetime must be preserved
   // as long as this DrawPolygon is alive.
   const DrawQuad* original_ref_;
   bool is_split_;
 };
 
 }  // namespace cc
 
 #endif  // CC_QUADS_DRAW_POLYGON_H_