From c4d1fb2981b00415b465701f63a488666fd2e825 Mon Sep 17 00:00:00 2001
From: lekha <305097+lekha@users.noreply.github.com>
Date: Fri, 4 Dec 2020 00:48:05 -0500
Subject: [PATCH] Fix(clip): delete clipping region with restore (#73)

@hustcc please release a patch version after this pull request to version 2.2.1.

Thanks!

* Fix(clip): delete clipping region with restore

Previously, the restore() method would update its pointer to the previous
clipping region (stackIndex = n-1) in the stack without deleting the current
clipping region (stackIndex = n). This caused problems for future save() calls,
which would push a new clipping region onto the stack at location n+1 while
only incrementing stackIndex to n, thus referencing the wrong region.

This commit fixes the issue by deleting the current clipping region when
restore() is called. By doing this, we lose the ability to inspect old
clipping regions, but this is easy to change in the future if it is desired.

* Address PR comments

Tests have been combined and make better use of the afterEach snapshots.

* Remove excess checks in test
---
 README.md                                     |  1 +
 ...vasRenderingContext2D.__getClippingPath.js | 11 ++++++++++
 .../CanvasRenderingContext2D.restore.js       | 20 +++++++++++++++++
 ...nderingContext2D.__getClippingPath.js.snap | 22 +++++++++++++++++++
 src/classes/CanvasRenderingContext2D.js       |  1 +
 5 files changed, 55 insertions(+)
 create mode 100644 __tests__/classes/CanvasRenderingContext2D.restore.js

diff --git a/README.md b/README.md
index 81e0949..a5190f1 100644
--- a/README.md
+++ b/README.md
@@ -178,6 +178,7 @@ canvas.toDataURL.mockReturnValueOnce(
 - [@hustcc](https://github.com/hustcc)
 - [@jtenner](https://github.com/jtenner)
 - [@evanoc0](https://github.com/evanoc0)
+- [@lekha](https://github.com/lekha)
 
 ## License
 
diff --git a/__tests__/classes/CanvasRenderingContext2D.__getClippingPath.js b/__tests__/classes/CanvasRenderingContext2D.__getClippingPath.js
index 70113ab..59dae6a 100644
--- a/__tests__/classes/CanvasRenderingContext2D.__getClippingPath.js
+++ b/__tests__/classes/CanvasRenderingContext2D.__getClippingPath.js
@@ -51,4 +51,15 @@ describe('__getClippingRegion', () => {
     ctx.restore();
     expect(region).toStrictEqual(ctx.__getClippingRegion());
   });
+
+  it('should delete current clipping region when restored', () => {
+    ctx.rect(1, 2, 3, 4);
+    ctx.clip();
+    ctx.save();
+    ctx.rect(1, 2, 3, 4);
+    ctx.arc(1, 2, 3, 4, 5);
+    ctx.clip();
+    ctx.restore();
+    ctx.save();
+  });
 });
diff --git a/__tests__/classes/CanvasRenderingContext2D.restore.js b/__tests__/classes/CanvasRenderingContext2D.restore.js
new file mode 100644
index 0000000..5924caa
--- /dev/null
+++ b/__tests__/classes/CanvasRenderingContext2D.restore.js
@@ -0,0 +1,20 @@
+let canvas;
+let ctx;
+
+beforeEach(() => {
+  canvas = document.createElement('canvas');
+  ctx = canvas.getContext('2d');
+  canvas.width = 400;
+  canvas.height = 300;
+});
+
+describe('save', () => {
+  it('should be a function', () => {
+    expect(typeof ctx.restore).toBe('function');
+  });
+
+  it('should be callable', () => {
+    ctx.restore();
+    expect(ctx.restore).toBeCalled();
+  });
+});
diff --git a/__tests__/classes/__snapshots__/CanvasRenderingContext2D.__getClippingPath.js.snap b/__tests__/classes/__snapshots__/CanvasRenderingContext2D.__getClippingPath.js.snap
index 7dd3119..bc02a2d 100644
--- a/__tests__/classes/__snapshots__/CanvasRenderingContext2D.__getClippingPath.js.snap
+++ b/__tests__/classes/__snapshots__/CanvasRenderingContext2D.__getClippingPath.js.snap
@@ -2,6 +2,28 @@
 
 exports[`__getClippingRegion should be empty when there are no path elements 1`] = `Array []`;
 
+exports[`__getClippingRegion should delete current clipping region when restored 1`] = `
+Array [
+  Object {
+    "props": Object {
+      "height": 4,
+      "width": 3,
+      "x": 1,
+      "y": 2,
+    },
+    "transform": Array [
+      1,
+      0,
+      0,
+      1,
+      0,
+      0,
+    ],
+    "type": "rect",
+  },
+]
+`;
+
 exports[`__getClippingRegion should save the clipping region correctly when saved 1`] = `
 Array [
   Object {
diff --git a/src/classes/CanvasRenderingContext2D.js b/src/classes/CanvasRenderingContext2D.js
index c44fa96..d6b897e 100644
--- a/src/classes/CanvasRenderingContext2D.js
+++ b/src/classes/CanvasRenderingContext2D.js
@@ -1477,6 +1477,7 @@ export default class CanvasRenderingContext2D {
     if (this._stackIndex <= 0) return;
 
     this._transformStack.pop();
+    this._clipStack.pop();
     this._directionStack.pop();
     this._fillStyleStack.pop();
     this._filterStack.pop();