From cf0777e8bcd9fb6b5f25acc66b22ef1de707b61d Mon Sep 17 00:00:00 2001 From: Andy Bayer Date: Tue, 12 Apr 2022 16:48:59 -0700 Subject: [PATCH 1/4] Decouple Groups' bounds from their position to fix anchoring issues (#142) * WIP Group bounds fix. Fixes #141 * Groups x and y position are their minimum x and y values, not the coordinates of their bounding box. * More polygon anchoring tests --- .../negativecoordinates.js | 15 ++++++ .../negativecoordinates.md | 7 +++ site/examples/groups/anchor/anchor.js | 4 ++ src/graphics/group.js | 49 +++++++++++++------ src/graphics/thing.js | 1 + test/group.test.js | 20 +++++++- test/polygon.test.js | 31 ++++++++++++ 7 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 site/examples/graphics/polygon/negativecoordinates/negativecoordinates.js create mode 100644 site/examples/graphics/polygon/negativecoordinates/negativecoordinates.md diff --git a/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.js b/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.js new file mode 100644 index 00000000..26d74fc5 --- /dev/null +++ b/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.js @@ -0,0 +1,15 @@ +let darkMode = window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches; + +let color = 'blue'; +if (darkMode) { + color = 'yellow'; +} +const t1 = new Polygon(); +t1.addPoint(-30, 0); +t1.addPoint(30, 30); +t1.addPoint(0, 30); +t1.debug = true; +t1.setPosition(getWidth() / 2, getHeight() / 2); +t1.setAnchor({ horizontal: 0, vertical: 0 }); +t1.setColor(color); +add(t1); diff --git a/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.md b/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.md new file mode 100644 index 00000000..e86d1204 --- /dev/null +++ b/site/examples/graphics/polygon/negativecoordinates/negativecoordinates.md @@ -0,0 +1,7 @@ +--- +title: Polygon - Negative Coordiantes +layout: example +code: negativecoordinates.js +--- + +Polygons can contain points which are negative relative to their position. This affects bounding box calculations. diff --git a/site/examples/groups/anchor/anchor.js b/site/examples/groups/anchor/anchor.js index de563658..cf259ed6 100644 --- a/site/examples/groups/anchor/anchor.js +++ b/site/examples/groups/anchor/anchor.js @@ -42,3 +42,7 @@ g3.setPosition((3 * getWidth()) / 4, getHeight() / 2); g3.setAnchor({ vertical: 1, horizontal: 1 }); g3.debug = true; add(g3); + +console.log(g1.getBounds()); +console.log(g2.getBounds()); +console.log(g3.getBounds()); diff --git a/src/graphics/group.js b/src/graphics/group.js index 9bbfe7c2..b2f96772 100644 --- a/src/graphics/group.js +++ b/src/graphics/group.js @@ -47,6 +47,18 @@ class Group extends Thing { this._hiddenContext = this._hiddenCanvas.getContext('2d'); this._lastRecordedBounds = {}; this.bounds = null; + /** + * The left-most x coordinate of elements in this group, which is considered its x value. + * @private + * @type {number} + */ + this._minX = 0; + /** + * The top-most y coordinate of elements in this group, which is considered its y value. + * @private + * @type {number} + */ + this._minY = 0; } /** @@ -54,14 +66,17 @@ class Group extends Thing { * @type {number} */ get x() { - return this.getBounds().left; + if (this._boundsInvalidated) { + this._updateBounds(); + } + return this._minX; } set x(x) { if (!this.bounds) { return; } - this.setPosition(x, this.bounds.top); + this.setPosition(x, this._minY); } /** @@ -69,14 +84,17 @@ class Group extends Thing { * @type {number} */ get y() { - return this.getBounds().top; + if (this._boundsInvalidated) { + this._updateBounds(); + } + return this._minY; } set y(y) { if (!this.bounds) { return; } - this.setPosition(this.bounds.left, y); + this.setPosition(this._minX, y); } /** @@ -157,9 +175,8 @@ class Group extends Thing { * @param {number} y */ setPosition(x, y) { - const bounds = this.getBounds(); - const dx = x - bounds.left; - const dy = y - bounds.top; + const dx = x - this.x; + const dy = y - this.y; this.move(dx, dy); } @@ -184,14 +201,14 @@ class Group extends Thing { // in the top left corner. // this means that only the bounding box surrounding the top // left corner needs to be drawn to the destination canvas - this._hiddenContext.translate(-bounds.left, -bounds.top); + this._hiddenContext.translate(-this.x, -this.y); this.elements .filter(element => element.alive) .sort((a, b) => a.layer - b.layer) .forEach(element => { element.draw(this._hiddenContext); }); - this._hiddenContext.translate(bounds.left, bounds.top); + this._hiddenContext.translate(this.x, this.y); context.drawImage(this._hiddenCanvas, 0, 0, width, height); context.closePath(); }); @@ -271,14 +288,16 @@ class Group extends Thing { maxX = Math.max(maxX, right); maxY = Math.max(maxY, bottom); }); - this.bounds = { - left: minX, - right: maxX, - top: minY, - bottom: maxY, - }; const width = maxX - minX; const height = maxY - minY; + this.bounds = { + left: minX - this.anchor.horizontal * width, + right: maxX - this.anchor.horizontal * width, + top: minY - this.anchor.vertical * height, + bottom: maxY - this.anchor.vertical * height, + }; + this._minX = minX; + this._minY = minY; this._hiddenCanvas.width = this.devicePixelRatio * width; this._hiddenCanvas.height = this.devicePixelRatio * height; this._hiddenCanvas.style.width = `${width}px`; diff --git a/src/graphics/thing.js b/src/graphics/thing.js index cdda9270..c2641fdf 100644 --- a/src/graphics/thing.js +++ b/src/graphics/thing.js @@ -571,6 +571,7 @@ class Thing { context.strokeStyle = 'red'; context.fill(); const bounds = this.getBounds(); + // move back to the origin context.translate(-drawX, -drawY); context.strokeRect( bounds.left, diff --git a/test/group.test.js b/test/group.test.js index 45d32f88..aa83a038 100644 --- a/test/group.test.js +++ b/test/group.test.js @@ -149,9 +149,18 @@ describe('Groups', () => { right: 5, }); }); + it('Considers anchoring', () => { + const g = new Group(); + g.add(new Rectangle(10, 10)); + expect(g.getBounds().left).toEqual(0); + g.setAnchor({ horizontal: 1.0, vertical: 0 }); + expect(g.getBounds().left).toEqual(-10); + g.setAnchor({ horizontal: 0.5, vertical: 0.5 }); + expect(g.getBounds()).toEqual({ top: -5, left: -5, right: 5, bottom: 5 }); + }); }); describe('Positioning', () => { - it('A groups x is its left bound', () => { + it("A Group's x is its left bound", () => { const g = new Group(); g.add(new Rectangle(20, 20)); expect(g.x).toEqual(g.getBounds().left); @@ -160,6 +169,15 @@ describe('Groups', () => { expect(g.x).toEqual(g.getBounds().left); expect(g.x).toEqual(10); }); + it("A Group's anchoring doesn't affect its position", () => { + const g = new Group(); + g.add(new Rectangle(20, 20)); + expect(g.x).toEqual(0); + g.setPosition(10, 10); + expect(g.x).toEqual(10); + g.setAnchor({ horizontal: 1.0, vertical: 0 }); + expect(g.x).toEqual(10); + }); }); describe('containsPoint', () => { it('Should be true if any of its children contain the point', () => { diff --git a/test/polygon.test.js b/test/polygon.test.js index 90244dc2..26c39079 100644 --- a/test/polygon.test.js +++ b/test/polygon.test.js @@ -7,6 +7,15 @@ describe('Polygon', () => { expect(new Polygon().type).toEqual('Polygon'); }); }); + describe("A Polygon's position (x, y)", () => { + it('Is unaffected by adding points', () => { + const p = new Polygon(); + p.addPoint(10, 10); + p.addPoint(20, 10); + p.addPoint(10, 20); + expect(p.x).toEqual(0); + }); + }); describe('addPoint', () => { it("Invalidates the superclass's bounds", () => { const p = new Polygon(); @@ -27,6 +36,7 @@ describe('Polygon', () => { p.addPoint(30, 0); p.addPoint(200, 0); expect(p.getWidth()).toBe(200); + expect(p.width).toBe(200); }); }); describe('getHeight()', () => { @@ -37,6 +47,7 @@ describe('Polygon', () => { p.addPoint(0, 120); p.addPoint(0, 90); expect(p.getHeight()).toBe(100); + expect(p.height).toBe(100); }); }); describe('move()', () => { @@ -109,5 +120,25 @@ describe('Polygon', () => { right: 0, }); }); + it('Its bounds are affected by anchoring', () => { + const p = new Polygon(); + p.addPoint(-10, 0); + p.addPoint(10, 0); + p.addPoint(10, 20); + p.addPoint(-10, 20); + expect(p.getBounds()).toEqual({ + top: 0, + left: -10, + right: 10, + bottom: 20, + }); + p.setAnchor({ vertical: 1, horizontal: 1 }); + expect(p.getBounds()).toEqual({ + top: -20, + left: -30, + bottom: 0, + right: -10, + }); + }); }); }); From f99dd696cf85ba5a12cdad4c80710e764722e10c Mon Sep 17 00:00:00 2001 From: Zach Galant Date: Thu, 17 Nov 2022 15:20:53 -0600 Subject: [PATCH 2/4] call resetDimensions on Text when we update the label with setText or setLabel. fixes #145 --- src/graphics/text.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/graphics/text.js b/src/graphics/text.js index 37b00355..3b51930a 100644 --- a/src/graphics/text.js +++ b/src/graphics/text.js @@ -122,6 +122,7 @@ class Text extends Thing { ); } this.label = label; + this.resetDimensions() } /** @@ -143,6 +144,7 @@ class Text extends Thing { ); } this.label = label; + this.resetDimensions() } /** From 516309d60ddd4e93dc91758c8454c23af405efb8 Mon Sep 17 00:00:00 2001 From: Zach Galant Date: Thu, 17 Nov 2022 15:23:57 -0600 Subject: [PATCH 3/4] update tests --- test/text.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/text.test.js b/test/text.test.js index 8b018cc8..818c7d1f 100644 --- a/test/text.test.js +++ b/test/text.test.js @@ -87,6 +87,12 @@ describe('Text', () => { t.setText('fdsa'); expect(t.label).toEqual('fdsa'); }); + it('Updates dimensions', () => { + const t = new Text('mmmm'); + const originalWidth = t.getWidth(); + t.setText('mmmmmmm'); + expect(t.getWidth()).toBeGreaterThan(originalWidth); + }); }); describe('setLabel', () => { it('Errors for invalid arguments', () => { @@ -109,6 +115,12 @@ describe('Text', () => { t.setLabel('fdsa'); expect(t.label).toEqual('fdsa'); }); + it('Updates dimensions', () => { + const t = new Text('mmmm'); + const originalWidth = t.getWidth(); + t.setLabel('mmmmmmm'); + expect(t.getWidth()).toBeGreaterThan(originalWidth); + }); }); describe('containsPoint', () => { it('Checks if a point is contained in the Text', () => { From 9046bc3adbeadd5448542946a653b3cd633eba89 Mon Sep 17 00:00:00 2001 From: Zach Galant Date: Thu, 17 Nov 2022 15:41:32 -0600 Subject: [PATCH 4/4] 0.3.5 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a3aa48b4..23c1724e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,11 +1,11 @@ { "name": "chs-js-lib", - "version": "0.3.4", + "version": "0.3.5", "lockfileVersion": 2, "requires": true, "packages": { "": { - "version": "0.3.4", + "version": "0.3.5", "license": "ISC", "dependencies": { "http-server": "^14.1.0", diff --git a/package.json b/package.json index 1a9729cf..f40c7e35 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chs-js-lib", - "version": "0.3.4", + "version": "0.3.5", "description": "JavaScript graphics library used in CodeHS's platform.", "main": "dist/chs.cjs", "module": "dist/chs.mjs",