Bug 1846079 - Ensure pruned point begins first sub-path if necessary. r=aosmond

The Canvas2D specification says that if a path has no active sub-paths, and a
primitive is drawn, that the first point of that primitive becomes the start of
the newly created sub-path that will be created for it.

So if we prune a point when a path has no active sub-paths, and then a new
primitive comes in that does not start with that same point, we risk not
installing the pruned point as the start of that new sub-path.

To solve this, we need to detect if a path has no active sub-paths while
we are building it. This adds PathBuilder::IsActive() to help with that.
Then before we go to add a primitive, we check if there is a pruned point
on a path that is not active yet, and if so, install the correct start
point with a MoveTo.

This also makes IsActive and IsEmpty required so to ensure all our path
implementations behave consistently rather than having any surprising
unimplemented behavior.

Differential Revision: https://phabricator.services.mozilla.com/D184891

dom/canvas/CanvasPath.h

@@ -88,6 +88,7 @@ class CanvasPath final : public nsWrapperCache {
 
   void EnsurePathBuilder() const;
   void EnsureCapped() const;
+  void EnsureActive() const;
 };
 
 }  // namespace dom

dom/canvas/CanvasRenderingContext2D.cpp

@@ -3385,6 +3385,8 @@ void CanvasRenderingContext2D::Arc(double aX, double aY, double aR,
 
   EnsureWritablePath();
 
+  EnsureActivePath();
+
   mPathBuilder->Arc(Point(aX, aY), aR, aStartAngle, aEndAngle, aAnticlockwise);
   mPathPruned = false;
 }
@@ -6369,6 +6371,16 @@ inline void CanvasPath::EnsureCapped() const {
   }
 }
 
+inline void CanvasPath::EnsureActive() const {
+  // If the path is not active, then adding an op to the path may cause the path
+  // to add the first point of the op as the initial point instead of the actual
+  // current point.
+  if (mPruned && !mPathBuilder->IsActive()) {
+    mPathBuilder->MoveTo(mPathBuilder->CurrentPoint());
+    mPruned = false;
+  }
+}
+
 void CanvasPath::MoveTo(double aX, double aY) {
   EnsurePathBuilder();
 
@@ -6399,6 +6411,8 @@ void CanvasPath::QuadraticCurveTo(double aCpx, double aCpy, double aX,
     return;
   }
 
+  EnsureActive();
+
   mPathBuilder->QuadraticBezierTo(cp1, cp2);
   mPruned = false;
 }
@@ -6518,6 +6532,8 @@ void CanvasPath::Arc(double aX, double aY, double aRadius, double aStartAngle,
 
   EnsurePathBuilder();
 
+  EnsureActive();
+
   mPathBuilder->Arc(Point(aX, aY), aRadius, aStartAngle, aEndAngle,
                     aAnticlockwise);
   mPruned = false;
@@ -6552,6 +6568,8 @@ void CanvasPath::LineTo(const gfx::Point& aPoint) {
     return;
   }
 
+  EnsureActive();
+
   mPathBuilder->LineTo(aPoint);
   mPruned = false;
 }
@@ -6568,6 +6586,8 @@ void CanvasPath::BezierTo(const gfx::Point& aCP1, const gfx::Point& aCP2,
     return;
   }
 
+  EnsureActive();
+
   mPathBuilder->BezierTo(aCP1, aCP2, aCP3);
   mPruned = false;
 }

dom/canvas/CanvasRenderingContext2D.h

@@ -352,6 +352,13 @@ class CanvasRenderingContext2D : public nsICanvasRenderingContextInternal,
     }
   }
 
+  void EnsureActivePath() {
+    if (mPathPruned && !mPathBuilder->IsActive()) {
+      mPathBuilder->MoveTo(mPathBuilder->CurrentPoint());
+      mPathPruned = false;
+    }
+  }
+
   void ClosePath() {
     EnsureWritablePath();
 
@@ -390,6 +397,7 @@ class CanvasRenderingContext2D : public nsICanvasRenderingContextInternal,
       mPathPruned = true;
       return;
     }
+    EnsureActivePath();
     mPathBuilder->QuadraticBezierTo(cp1, cp2);
     mPathPruned = false;
   }
@@ -530,6 +538,7 @@ class CanvasRenderingContext2D : public nsICanvasRenderingContextInternal,
       mPathPruned = true;
       return;
     }
+    EnsureActivePath();
     mPathBuilder->LineTo(aPoint);
     mPathPruned = false;
   }
@@ -545,6 +554,7 @@ class CanvasRenderingContext2D : public nsICanvasRenderingContextInternal,
       mPathPruned = true;
       return;
     }
+    EnsureActivePath();
     mPathBuilder->BezierTo(aCP1, aCP2, aCP3);
     mPathPruned = false;
   }

gfx/2d/2D.h

@@ -1007,7 +1007,7 @@ class Path : public external::AtomicRefCounted<Path> {
 
   virtual Point ComputePointAtLength(Float aLength, Point* aTangent = nullptr);
 
-  virtual bool IsEmpty() const { return false; }
+  virtual bool IsEmpty() const = 0;
 
  protected:
   Path();
@@ -1028,6 +1028,8 @@ class PathBuilder : public PathSink {
   virtual already_AddRefed<Path> Finish() = 0;
 
   virtual BackendType GetBackendType() const = 0;
+
+  virtual bool IsActive() const = 0;
 };
 
 struct Glyph {

gfx/2d/PathCairo.cpp

@@ -239,6 +239,20 @@ void PathCairo::StreamToSink(PathSink* aSink) const {
   }
 }
 
+bool PathCairo::IsEmpty() const {
+  for (size_t i = 0; i < mPathData.size(); i++) {
+    switch (mPathData[i].header.type) {
+      case CAIRO_PATH_MOVE_TO:
+        break;
+      case CAIRO_PATH_CLOSE_PATH:
+        break;
+      default:
+        return false;
+    }
+  }
+  return true;
+}
+
 void PathCairo::EnsureContainingContext(const Matrix& aTransform) const {
   if (mContainingContext) {
     if (mContainingTransform.ExactlyEquals(aTransform)) {

gfx/2d/PathCairo.h

@@ -34,6 +34,8 @@ class PathBuilderCairo : public PathBuilder {
 
   BackendType GetBackendType() const override { return BackendType::CAIRO; }
 
+  bool IsActive() const override { return !mPathData.empty(); }
+
   static already_AddRefed<PathBuilder> Create(FillRule aFillRule);
 
  private:  // data
@@ -80,6 +82,8 @@ class PathCairo : public Path {
   void AppendPathToBuilder(PathBuilderCairo* aBuilder,
                            const Matrix* aTransform = nullptr) const;
 
+  bool IsEmpty() const override;
+
  private:
   void EnsureContainingContext(const Matrix& aTransform) const;
 

gfx/2d/PathD2D.h

@@ -45,6 +45,8 @@ class PathBuilderD2D : public PathBuilder {
 
   bool IsFigureActive() const { return mFigureActive; }
 
+  virtual bool IsActive() const { return IsFigureActive(); }
+
   static already_AddRefed<PathBuilder> Create(FillRule aFillRule);
 
  private:

gfx/2d/PathRecording.cpp

@@ -179,6 +179,25 @@ size_t PathOps::NumberOfOps() const {
   return size;
 }
 
+bool PathOps::IsEmpty() const {
+  const uint8_t* nextByte = mPathData.data();
+  const uint8_t* end = nextByte + mPathData.size();
+  while (nextByte < end) {
+    const OpType opType = *reinterpret_cast<const OpType*>(nextByte);
+    nextByte += sizeof(OpType);
+    switch (opType) {
+      case OpType::OP_MOVETO:
+        nextByte += sizeof(Point);
+        break;
+      case OpType::OP_CLOSE:
+        break;
+      default:
+        return false;
+    }
+  }
+  return true;
+}
+
 void PathBuilderRecording::MoveTo(const Point& aPoint) {
   mPathOps.MoveTo(aPoint);
   mBeginPoint = aPoint;

gfx/2d/PathRecording.h

@@ -71,6 +71,10 @@ class PathOps {
 
   Maybe<Circle> AsCircle() const;
 
+  bool IsActive() const { return !mPathData.empty(); }
+
+  bool IsEmpty() const;
+
  private:
   enum class OpType : uint32_t {
     OP_MOVETO = 0,
@@ -167,6 +171,8 @@ class PathBuilderRecording final : public PathBuilder {
 
   BackendType GetBackendType() const final { return BackendType::RECORDING; }
 
+  bool IsActive() const final { return mPathOps.IsActive(); }
+
  private:
   BackendType mBackendType;
   FillRule mFillRule;
@@ -222,6 +228,8 @@ class PathRecording final : public Path {
 
   FillRule GetFillRule() const final { return mFillRule; }
 
+  bool IsEmpty() const final { return mPathOps.IsEmpty(); }
+
  private:
   friend class DrawTargetWrapAndRecord;
   friend class DrawTargetRecording;

gfx/2d/PathSkia.h

@@ -37,6 +37,8 @@ class PathBuilderSkia : public PathBuilder {
 
   BackendType GetBackendType() const override { return BackendType::SKIA; }
 
+  bool IsActive() const override { return mPath.countPoints() > 0; }
+
   static already_AddRefed<PathBuilder> Create(FillRule aFillRule);
 
  private:

layout/reftests/canvas/1846079-1-ref.html

@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<canvas id="c" width="50" height="50"></canvas>
+</body>
+</html>
+
+

layout/reftests/canvas/1846079-1.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<canvas id="c" width="50" height="50"></canvas>
+
+<script>
+const c = document.getElementById("c");
+const ctx = c.getContext("2d");
+
+ctx.translate(20, 20);
+
+ctx.beginPath();
+ctx.lineTo(0, 0);
+ctx.lineTo(0, 20);
+ctx.stroke();
+
+</script>
+
+</body>
+</html>
+
+

layout/reftests/canvas/reftest.list

@@ -119,3 +119,5 @@ fuzzy-if(winWidget,0-94,0-1575) fuzzy-if(cocoaWidget,0-1,0-34) == 1304353-text-g
 
 == 1817455-1.html 1817455-1-ref.html
 != 1817873-1.html 1817873-1-ref.html
+
+!= 1846079-1.html 1846079-1-ref.html