From 60792807eee29f1b748c9b12d308c477f7c1227d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ulrich-Matthias=20Sch=C3=A4fer?= Date: Sat, 12 Jan 2019 20:32:39 +0100 Subject: [PATCH] Fix move and size of groups, removed setting of a default font so we dont act against user intention, fixed bug in `font()` --- CHANGELOG.md | 9 +++ spec/spec/elements/G.js | 142 ++++++++++++++++++++++++++++++---- src/elements/Ellipse.js | 2 +- src/elements/G.js | 77 +++++++++--------- src/elements/Text.js | 4 - src/elements/Tspan.js | 8 +- src/modules/optional/sugar.js | 1 + src/types/Box.js | 6 +- src/types/Point.js | 21 +++-- src/types/PointArray.js | 2 +- src/utils/adopter.js | 24 ++++++ 11 files changed, 228 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ad260b..0e8f89d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,16 @@ The document follows the conventions described in [“Keep a CHANGELOG”](http: ==== +## [3.0.6] + +### Fixed + - fixed group move and size commands + - default font size is not set anymore because it mostly goes against what the user wants + - fix bug in `font()` which set wrong values + ## [3.0.5] - 2018-12-12 + +### Fixed - fixed `parser` which didnt have all required css rules and not focusable=false - group `x(), y(), width(), height(), dx(), dy()` now correctly change the bbox of the group by moving/resizing all children - fixed timeline which fired `finished` too early diff --git a/spec/spec/elements/G.js b/spec/spec/elements/G.js index 2c2efc8..385419e 100644 --- a/spec/spec/elements/G.js +++ b/spec/spec/elements/G.js @@ -1,4 +1,4 @@ -import { G, Rect, makeInstance } from '../../../src/main'; +import { Box, G, Rect, makeInstance } from '../../../src/main.js'; const { any, createSpy, objectContaining } = jasmine @@ -15,6 +15,68 @@ describe('G.js', () => { }) }) + describe('Container', () => { + describe('group()', () => { + it('creates a group in the container', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + expect(g).toEqual(any(G)) + expect(g.parent()).toBe(canvas) + }) + }) + }) + + describe('dmove()', () => { + it('moves the bbox of the group by a certain amount (1)', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + + g.add(new Rect({width:100, height:120, x:10, y:20})) + g.add(new Rect({width:70, height:100, x:50, y:60})) + + g.dmove(10, 10) + + const box = g.bbox() + expect(box).toEqual(objectContaining({ + x: 20, y: 30, width: box.width, height: box.height + })) + }) + + it('moves the bbox of the group by a certain amount (2)', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + + g.rect(400, 200).move(123, 312).rotate(34).skew(12) + g.rect(100, 50).move(11, 43).translate(123, 32).skew(-12) + g.rect(400, 200).rotate(90) + g.group().rotate(23).group().skew(32).rect(100, 40).skew(11).rotate(12) + + const oldBox = g.bbox() + + g.dmove(10, 10) + + const newBox = g.bbox() + + expect(newBox.x).toBeCloseTo(oldBox.x + 10, 4) + expect(newBox.y).toBeCloseTo(oldBox.y + 10, 4) + expect(newBox.w).toBeCloseTo(oldBox.w, 4) + expect(newBox.h).toBeCloseTo(oldBox.h, 4) + }) + }) + + describe('move()', () => { + it('calls dmove() with the correct difference', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + g.rect(100, 200).move(111, 223) + + spyOn(g, 'dmove') + + g.move(100, 150) + expect(g.dmove).toHaveBeenCalledWith(-11, -73) + }) + }) + describe('x()', () => { it('gets the x value of the bbox', () => { const canvas = makeInstance().addTo('#canvas') @@ -28,19 +90,15 @@ describe('G.js', () => { expect(g.x()).toBe(g.bbox().x) expect(g.x()).toBe(10) }) - it('sets the x value of the bbox by moving all children', () => { + it('calls move with the paramater as x', () => { const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + g.rect(100, 200).move(111, 223) - const g = new G() - g.add(new Rect({width:100, height:120, x:10, y:20})) - g.add(new Rect({width:70, height:100, x:50, y:60})) - - g.addTo(canvas) + spyOn(g, 'move') - expect(g.x(0)).toBe(g) - expect(g.bbox().x).toBe(0) - expect(g.children()[0].x()).toBe(0) - expect(g.children()[1].x()).toBe(40) + g.x(100) + expect(g.move).toHaveBeenCalledWith(100, g.bbox().y, any(Box)) }) }) @@ -57,7 +115,20 @@ describe('G.js', () => { expect(g.y()).toBe(g.bbox().y) expect(g.y()).toBe(20) }) - it('sets the y value of the bbox by moving all children', () => { + it('calls move with the paramater as y', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + g.rect(100, 200).move(111, 223) + + spyOn(g, 'move') + + g.y(100) + expect(g.move).toHaveBeenCalledWith(g.bbox().x, 100, any(Box)) + }) + }) + + describe('size()', () => { + it('changes the dimensions of the bbox (1)', () => { const canvas = makeInstance().addTo('#canvas') const g = new G() @@ -66,11 +137,50 @@ describe('G.js', () => { g.addTo(canvas) - expect(g.y(0)).toBe(g) - expect(g.bbox().y).toBe(0) - expect(g.children()[0].y()).toBe(0) - expect(g.children()[1].y()).toBe(40) + const oldBox = g.bbox() + + expect(g.size(100, 100)).toBe(g) + + const newBox = g.bbox() + + expect(newBox.x).toBeCloseTo(oldBox.x, 4) + expect(newBox.y).toBeCloseTo(oldBox.y, 4) + expect(newBox.w).toBeCloseTo(100, 4) + expect(newBox.h).toBeCloseTo(100, 4) + + expect(g.children()[0].width()).toBeCloseTo(90.909, 3) + expect(g.children()[1].width()).toBeCloseTo(63.636, 3) + + expect(g.children()[0].x()).toBeCloseTo(10, 3) + expect(g.children()[1].x()).toBeCloseTo(46.364, 3) + expect(g.children()[0].height()).toBeCloseTo(85.714, 3) + expect(g.children()[1].height()).toBeCloseTo(71.429, 3) + expect(g.children()[0].y()).toBeCloseTo(20, 3) + expect(g.children()[1].y()).toBeCloseTo(48.571, 3) + }) + + it('changes the dimensions of the bbox (2)', () => { + const canvas = makeInstance().addTo('#canvas') + const g = canvas.group() + + g.rect(400, 200).move(123, 312).rotate(34).skew(12) + g.rect(100, 50).move(11, 43).translate(123, 32).skew(-12) + g.rect(400, 200).rotate(90) + g.group().rotate(23).group().skew(32).rect(100, 40).skew(11).rotate(12) + + const oldBox = g.bbox() + + g.size(100, 100) + + const newBox = g.bbox() + + expect(newBox.x).toBeCloseTo(oldBox.x, 4) + expect(newBox.y).toBeCloseTo(oldBox.y, 4) + expect(newBox.w).toBeCloseTo(100, 4) + expect(newBox.h).toBeCloseTo(100, 4) }) + + }) describe('width()', () => { diff --git a/src/elements/Ellipse.js b/src/elements/Ellipse.js index 0350f1f..beba228 100644 --- a/src/elements/Ellipse.js +++ b/src/elements/Ellipse.js @@ -28,7 +28,7 @@ extend(Ellipse, circled) registerMethods('Container', { // Create an ellipse - ellipse: wrapWithAttrCheck(function (width, height) { + ellipse: wrapWithAttrCheck(function (width = 0, height = width) { return this.put(new Ellipse()).size(width, height).move(0, 0) }) }) diff --git a/src/elements/G.js b/src/elements/G.js index 8171fed..a9e8b55 100644 --- a/src/elements/G.js +++ b/src/elements/G.js @@ -2,7 +2,8 @@ import { nodeOrNew, register, wrapWithAttrCheck } from '../utils/adopter.js' import { proportionalSize } from '../utils/utils.js' import { registerMethods } from '../utils/methods.js' import Container from './Container.js' -import SVGNumber from '../types/SVGNumber.js' +import Matrix from '../types/Matrix.js' +import Point from '../types/Point.js' export default class G extends Container { constructor (node) { @@ -11,70 +12,68 @@ export default class G extends Container { x (x, box = this.bbox()) { if (x == null) return box.x - - this.children().dx(x - box.x) - return this + return this.move(x, box.y, box) } y (y, box = this.bbox()) { if (y == null) return box.y - - this.children().dy(y - box.y) - return this + return this.move(box.x, y, box) } - move (x, y) { - const box = this.bbox() - return this.x(x, box).y(y, box) + move (x = 0, y = 0, box = this.bbox()) { + const dx = x - box.x + const dy = y - box.y + + return this.dmove(dx, dy) } dx (dx) { - return this.children().dx(dx) + return this.dmove(dx, 0) } dy (dy) { - return this.children().dy(dy) + return this.dmove(0, dy) } - width (width, box = this.bbox()) { - if (width == null) return box.width - - const scale = width / box.width - - this.each(function () { - const _width = this.width() - const _x = this.x() - - this.width(_width * scale) - this.x((_x - box.x) * scale + box.x) + dmove (dx, dy) { + this.children().forEach((child, i) => { + // Get the childs bbox + const bbox = child.bbox() + // Get childs matrix + const m = new Matrix(child) + // Translate childs matrix by amount and + // transform it back into parents space + const matrix = m.translate(dx, dy).transform(m.inverse()) + // Calculate new x and y from old box + const p = new Point(bbox.x, bbox.y).transform(matrix) + // Move element + child.move(p.x, p.y) }) return this } + width (width, box = this.bbox()) { + if (width == null) return box.width + return this.size(width, box.height, box) + } + height (height, box = this.bbox()) { if (height == null) return box.height - - const scale = height / box.height - - this.each(function () { - const _height = this.height() - const _y = this.y() - - this.height(_height * scale) - this.y((_y - box.y) * scale + box.y) - }) - - return this + return this.size(box.width, height, box) } - size (width, height) { - const box = this.bbox() + size (width, height, box = this.bbox()) { const p = proportionalSize(this, width, height, box) + const scaleX = p.width / box.width + const scaleY = p.height / box.height + + this.children().forEach((child, i) => { + const o = new Point(box).transform(new Matrix(child).inverse()) + child.scale(scaleX, scaleY, o.x, o.y) + }) return this - .width(new SVGNumber(p.width), box) - .height(new SVGNumber(p.height), box) } } diff --git a/src/elements/Text.js b/src/elements/Text.js index db9c2ee..cfa3f51 100644 --- a/src/elements/Text.js +++ b/src/elements/Text.js @@ -5,7 +5,6 @@ import { register, wrapWithAttrCheck } from '../utils/adopter.js' -import { attrs } from '../modules/core/defaults.js' import { registerMethods } from '../utils/methods.js' import SVGNumber from '../types/SVGNumber.js' import Shape from './Shape.js' @@ -20,9 +19,6 @@ export default class Text extends Shape { this.dom.leading = new SVGNumber(1.3) // store leading value for rebuilding this._rebuild = true // enable automatic updating of dy values this._build = false // disable build mode for adding multiple lines - - // set default font - this.attr('font-family', attrs['font-family']) } // Move over x-axis diff --git a/src/elements/Tspan.js b/src/elements/Tspan.js index abd032f..64895fc 100644 --- a/src/elements/Tspan.js +++ b/src/elements/Tspan.js @@ -4,7 +4,9 @@ import { register, wrapWithAttrCheck } from '../utils/adopter.js' +import { globals } from '../utils/window.js' import { registerMethods } from '../utils/methods.js' +import SVGNumber from '../types/SVGNumber.js' import Text from './Text.js' import * as textable from '../modules/core/textable.js' @@ -41,8 +43,12 @@ export default class Tspan extends Text { // mark new line this.dom.newLined = true + var fontSize = globals.window.getComputedStyle(this.node) + .getPropertyValue('font-size') + var dy = t.dom.leading * new SVGNumber(fontSize) + // apply new position - return this.dy(t.dom.leading * t.attr('font-size')).attr('x', t.x()) + return this.dy(dy).attr('x', t.x()) } } diff --git a/src/modules/optional/sugar.js b/src/modules/optional/sugar.js index c05512d..7aba0f7 100644 --- a/src/modules/optional/sugar.js +++ b/src/modules/optional/sugar.js @@ -136,6 +136,7 @@ registerMethods([ 'Element', 'Runner' ], { font: function (a, v) { if (typeof a === 'object') { for (v in a) this.font(v, a[v]) + return this } return a === 'leading' diff --git a/src/types/Box.js b/src/types/Box.js index eb43d07..8cbda30 100644 --- a/src/types/Box.js +++ b/src/types/Box.js @@ -59,6 +59,10 @@ export default class Box { } transform (m) { + if (!(m instanceof Matrix)) { + m = new Matrix(m) + } + let xMin = Infinity let xMax = -Infinity let yMin = Infinity @@ -130,7 +134,7 @@ export function bbox () { clone.remove() return box } catch (e) { - throw new Error('Getting bbox of element "' + el.node.nodeName + '" is not possible') + throw new Error('Getting bbox of element "' + el.node.nodeName + '" is not possible. ' + e.toString()) } })) } diff --git a/src/types/Point.js b/src/types/Point.js index f1c85a1..329b37d 100644 --- a/src/types/Point.js +++ b/src/types/Point.js @@ -1,3 +1,5 @@ +import Matrix from './Matrix.js' + export default class Point { // Initialize constructor (...args) { @@ -25,14 +27,23 @@ export default class Point { return new Point(this) } - // transform point with matrix transform (m) { + return this.clone().transformO(m) + } + + // Transform point with matrix + transformO (m) { + if (!Matrix.isMatrixLike(m)) { + m = new Matrix(m) + } + + let { x, y } = this + // Perform the matrix multiplication - var x = m.a * this.x + m.c * this.y + m.e - var y = m.b * this.x + m.d * this.y + m.f + this.x = m.a * x + m.c * y + m.e + this.y = m.b * x + m.d * y + m.f - // Return the required point - return new Point(x, y) + return this } toArray () { diff --git a/src/types/PointArray.js b/src/types/PointArray.js index 2246bbd..6a869d7 100644 --- a/src/types/PointArray.js +++ b/src/types/PointArray.js @@ -63,7 +63,7 @@ extend(PointArray, { // Odd number of coordinates is an error. In such cases, drop the last odd coordinate. if (array.length % 2 !== 0) array.pop() - // wrap points in two-tuples and parse points as floats + // wrap points in two-tuples for (var i = 0, len = array.length; i < len; i = i + 2) { points.push([ array[i], array[i + 1] ]) } diff --git a/src/utils/adopter.js b/src/utils/adopter.js index f091c96..45e9bd3 100644 --- a/src/utils/adopter.js +++ b/src/utils/adopter.js @@ -138,3 +138,27 @@ export function wrapWithAttrCheck (fn) { } } } + +export function invent (config) { + // Create element initializer + var initializer = typeof config.create === 'function' + ? config.create + : function (node) { + this.constructor(node || create(config.create)) + } + + // Inherit prototype + if (config.inherit) { + /* eslint new-cap: off */ + initializer.prototype = new config.inherit() + initializer.prototype.constructor = initializer + } + + // Extend with methods + if (config.extend) { extend(initializer, config.extend) } + + // Attach construct method to parent + if (config.construct) { extend(config.parent || elements['Container'], config.construct) } + + return initializer +} -- 2.39.5