From dbb947b7aa11d06231b1aba31c7967bf79d07f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Tue, 22 Mar 2011 17:15:56 +0000 Subject: [PATCH 1/5] Finish ObservedRectangle, by solving observer notification for all setters. --- src/basic/Rectangle.js | 93 +++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/src/basic/Rectangle.js b/src/basic/Rectangle.js index 4dbdd10f..b9d427e4 100644 --- a/src/basic/Rectangle.js +++ b/src/basic/Rectangle.js @@ -97,7 +97,6 @@ var Rectangle = this.Rectangle = Base.extend({ }, setLeft: function(left) { - // right should not move this.width -= left - this.x; this.x = left; return this; @@ -255,14 +254,17 @@ var Rectangle = this.Rectangle = Base.extend({ getX = 'get' + x, getY = 'get' + y, setX = 'set' + x, - setY = 'set' + y; - this['get' + part] = function() { - return ObservedPoint.create(this, 'set' + part, + setY = 'set' + y, + get = 'get' + part, + set = 'set' + part; + this[get] = function() { + return ObservedPoint.create(this, set, this[getX](), this[getY]()); }; - this['set' + part] = function(point) { + this[set] = function(point) { point = Point.read(arguments); - return this[setX](point.x)[setY](point.y); // Note: call chaining! + // Note: call chaining happens here. + return this[setX](point.x)[setY](point.y); }; }, { beans: true }); }); @@ -280,50 +282,6 @@ var ObservedRectangle = Rectangle.extend({ return this; }, - // TODO: Use loop to create these? - getX: function() { - return this._x; - }, - - setX: function(x) { - this._x = x; - this._observer[this._set](this); - }, - - getY: function() { - return this._y; - }, - - setY: function(y) { - this._y = y; - this._observer[this._set](this); - }, - - getWidth: function() { - return this._width; - }, - - setWidth: function(width) { - this._width = width; - this._observer[this._set](this); - }, - - getHeight: function() { - return this._height; - }, - - setHeight: function(height) { - this._height = height; - this._observer[this._set](this); - }, - - // TODO: Implement for all properties on ObservedRectangle using loop - setCenter: function(center) { - Rectangle.prototype.setCenter.apply(this, center); - this._observer[this._set](this); - return this; - }, - statics: { /** * Provide a faster creator for Points out of two coordinates that @@ -342,4 +300,39 @@ var ObservedRectangle = Rectangle.extend({ return rect; } } +}, new function() { + var proto = Rectangle.prototype; + + return Base.each(['x', 'y', 'width', 'height'], function(key) { + var part = Base.capitalize(key); + var internal = '_' + key; + this['get' + part] = function() { + return this[internal]; + } + + this['set' + part] = function(value) { + this[internal] = value; + // Check if this setter is called from another one which sets + // _dontNotify, as it will notify itself + if (!this._dontNotify) + this._observer[this._set](this); + } + }, Base.each(['Point', 'Size', 'Center', + 'Left', 'Top', 'Right', 'Bottom', 'CenterX', 'CenterY', + 'TopLeft', 'TopRight', 'BottomLeft', 'BottomRight', + 'LeftCenter', 'TopCenter', 'RightCenter', 'BottomCenter'], + function(key) { + var name = 'set' + key; + this[name] = function(value) { + // Make sure the above setters of x, y, width, height do not + // each notify the observer, as we're going to take care of this + // afterwards here, only once per change. + this._dontNotify = true; + proto[name].apply(this, arguments); + delete this._dontNotify; + this._observer[this._set](this); + return this; + } + }, { beans: true }) + ); }); From b5fdecf3d19e2a460736c8ae0fd84c6cd1f6450f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Tue, 22 Mar 2011 17:27:46 +0000 Subject: [PATCH 2/5] Rename ObservedRectangle to LinkedRectangle, ObservedPoint to LinkedPoint, and add more comments about what it is they are doing. --- src/basic/Point.js | 35 ++++++++++++++++------------------- src/basic/Rectangle.js | 34 ++++++++++++++++++---------------- src/item/Group.js | 2 +- src/item/PlacedSymbol.js | 2 +- src/path/CompoundPath.js | 2 +- src/path/Path.js | 2 +- 6 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/basic/Point.js b/src/basic/Point.js index 5eee82e6..dac0df42 100644 --- a/src/basic/Point.js +++ b/src/basic/Point.js @@ -159,7 +159,7 @@ var Point = this.Point = Base.extend({ if (this.isZero()) { if (this._angle != null) { var a = this._angle; - // Use #set() instead of direct assignment, so ObservedPoint + // Use #set() instead of direct assignment, so LinkedPoint // can optimise this.set( Math.cos(a) * length, @@ -177,7 +177,7 @@ var Point = this.Point = Base.extend({ // x and y are 0 this.getAngle(); } - // Use #set() instead of direct assignment, so ObservedPoint + // Use #set() instead of direct assignment, so LinkedPoint // can optimise this.set( this.x * scale, @@ -237,7 +237,7 @@ var Point = this.Point = Base.extend({ angle = this._angle = angle * Math.PI / 180; if (!this.isZero()) { var length = this.getLength(); - // Use #set() instead of direct assignment, so ObservedPoint + // Use #set() instead of direct assignment, so LinkedPoint // can optimise this.set( Math.cos(angle) * length, @@ -573,13 +573,19 @@ var Point = this.Point = Base.extend({ } }); -var ObservedPoint = Point.extend({ +/** + * An internal version of Point that notifies its owner of each change through + * setting itself again on the setter that corresponds to the getter that + * produced this LinkedPoint. See uses of LinkedPoint.create() + * Note: This prototype is not exported. + */ +var LinkedPoint = Point.extend({ beans: true, set: function(x, y) { this._x = x; this._y = y; - this._observer[this._set](this); + this._owner[this._set](this); return this; }, @@ -589,7 +595,7 @@ var ObservedPoint = Point.extend({ setX: function(x) { this._x = x; - this._observer[this._set](this); + this._owner[this._set](this); }, getY: function() { @@ -598,24 +604,15 @@ var ObservedPoint = Point.extend({ setY: function(y) { this._y = y; - this._observer[this._set](this); + this._owner[this._set](this); }, statics: { - /** - * Provide a faster creator for Points out of two coordinates that - * does not rely on Point#initialize at all. This speeds up all math - * operations a lot. - */ - create: function(observer, set, x, y) { - // Don't use the shorter form as we want absolute maximum - // performance here: - // return new Point(Point.dont).set(x, y); - // TODO: Benchmark and decide - var point = new ObservedPoint(ObservedPoint.dont); + create: function(owner, set, x, y) { + var point = new LinkedPoint(LinkedPoint.dont); point._x = x; point._y = y; - point._observer = observer; + point._owner = owner; point._set = set; return point; } diff --git a/src/basic/Rectangle.js b/src/basic/Rectangle.js index b9d427e4..4af4ea14 100644 --- a/src/basic/Rectangle.js +++ b/src/basic/Rectangle.js @@ -71,7 +71,7 @@ var Rectangle = this.Rectangle = Base.extend({ }, getPoint: function() { - return ObservedPoint.create(this, 'setPoint', this.x, this.y); + return LinkedPoint.create(this, 'setPoint', this.x, this.y); }, setPoint: function(point) { @@ -149,7 +149,7 @@ var Rectangle = this.Rectangle = Base.extend({ }, getCenter: function() { - return ObservedPoint.create(this, 'setCenter', + return LinkedPoint.create(this, 'setCenter', this.getCenterX(), this.getCenterY()); }, @@ -258,7 +258,7 @@ var Rectangle = this.Rectangle = Base.extend({ get = 'get' + part, set = 'set' + part; this[get] = function() { - return ObservedPoint.create(this, set, + return LinkedPoint.create(this, set, this[getX](), this[getY]()); }; this[set] = function(point) { @@ -269,7 +269,13 @@ var Rectangle = this.Rectangle = Base.extend({ }, { beans: true }); }); -var ObservedRectangle = Rectangle.extend({ +/** + * An internal version of Rectangle that notifies its owner of each change + * through setting itself again on the setter that corresponds to the getter + * that produced this LinkedRectangle. See uses of LinkedRectangle.create() + * Note: This prototype is not exported. + */ +var LinkedRectangle = Rectangle.extend({ beans: true, set: function(x, y, width, height) { @@ -277,8 +283,8 @@ var ObservedRectangle = Rectangle.extend({ this._y = y; this._width = width; this._height = height; - if (this._observer) - this._observer[this._set](this); + if (this._owner) + this._owner[this._set](this); return this; }, @@ -288,14 +294,10 @@ var ObservedRectangle = Rectangle.extend({ * does not rely on Point#initialize at all. This speeds up all math * operations a lot. */ - create: function(observer, set, x, y, width, height) { - // Don't use the shorter form as we want absolute maximum - // performance here: - // return new Point(Point.dont).set(x, y); - // TODO: Benchmark and decide - var rect = new ObservedRectangle(ObservedRectangle.dont).set(x, y, + create: function(owner, set, x, y, width, height) { + var rect = new LinkedRectangle(LinkedRectangle.dont).set(x, y, width, height); - rect._observer = observer; + rect._owner = owner; rect._set = set; return rect; } @@ -315,7 +317,7 @@ var ObservedRectangle = Rectangle.extend({ // Check if this setter is called from another one which sets // _dontNotify, as it will notify itself if (!this._dontNotify) - this._observer[this._set](this); + this._owner[this._set](this); } }, Base.each(['Point', 'Size', 'Center', 'Left', 'Top', 'Right', 'Bottom', 'CenterX', 'CenterY', @@ -325,12 +327,12 @@ var ObservedRectangle = Rectangle.extend({ var name = 'set' + key; this[name] = function(value) { // Make sure the above setters of x, y, width, height do not - // each notify the observer, as we're going to take care of this + // each notify the owner, as we're going to take care of this // afterwards here, only once per change. this._dontNotify = true; proto[name].apply(this, arguments); delete this._dontNotify; - this._observer[this._set](this); + this._owner[this._set](this); return this; } }, { beans: true }) diff --git a/src/item/Group.js b/src/item/Group.js index 2c7af3e8..f5b09e04 100644 --- a/src/item/Group.js +++ b/src/item/Group.js @@ -43,7 +43,7 @@ var Group = this.Group = Item.extend({ y2 = Math.max(rect2.y + rect2.height, y1 + y2 - y1); } } - return ObservedRectangle.create(this, 'setBounds', + return LinkedRectangle.create(this, 'setBounds', x1, y1, x2 - x1, y2 - y1); }, diff --git a/src/item/PlacedSymbol.js b/src/item/PlacedSymbol.js index a50e1f87..54789f98 100644 --- a/src/item/PlacedSymbol.js +++ b/src/item/PlacedSymbol.js @@ -44,7 +44,7 @@ var PlacedSymbol = this.PlacedSymbol = Item.extend({ getBounds: function() { var bounds = this.symbol._definition.getStrokeBounds(this.matrix, true); - return ObservedRectangle.create(this, 'setBounds', + return LinkedRectangle.create(this, 'setBounds', bounds.x, bounds.y, bounds.width, bounds.height); }, diff --git a/src/path/CompoundPath.js b/src/path/CompoundPath.js index a4672489..687c216e 100644 --- a/src/path/CompoundPath.js +++ b/src/path/CompoundPath.js @@ -42,7 +42,7 @@ var CompoundPath = this.CompoundPath = PathItem.extend({ y2 = Math.max(rect2.y + rect2.height, y1 + y2 - y1); } } - return ObservedRectangle.create(this, 'setBounds', + return LinkedRectangle.create(this, 'setBounds', x1, y1, x2 - x1, y2 - y1); }, diff --git a/src/path/Path.js b/src/path/Path.js index 26a43c36..28df816d 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -701,7 +701,7 @@ var Path = this.Path = PathItem.extend({ // Pass the matrix hidden from Bootstrap, so it still inject // getBounds as bean too. var bounds = getBounds(this, arguments[0]); - return ObservedRectangle.create(this, 'setBounds', + return LinkedRectangle.create(this, 'setBounds', bounds.x, bounds.y, bounds.width, bounds.height); }, From 306c22e00cdee18a67a7520e295408c901c1d69c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Fri, 25 Mar 2011 19:57:49 +0200 Subject: [PATCH 3/5] Remove TODO about bug that was fixed in the meantime. --- examples/Tools/SquareRounded.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/examples/Tools/SquareRounded.html b/examples/Tools/SquareRounded.html index 1c47d5ab..9b4e6611 100644 --- a/examples/Tools/SquareRounded.html +++ b/examples/Tools/SquareRounded.html @@ -70,9 +70,7 @@ curHandleSeg = path.lastSegment; // clone as we want the unmodified one: prevPoint = curHandleSeg.point.clone(); - // TODO: potential bug - in SG we don't need to clone this - // point, why? - path.add(curHandleSeg.clone()); + path.add(curHandleSeg); curPoint = path.lastSegment.point; } } From e83195bb8fa6176af9904108abc70ea971adddbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Fri, 25 Mar 2011 19:58:02 +0200 Subject: [PATCH 4/5] Add #first/lastCurve getters. --- src/path/Path.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/path/Path.js b/src/path/Path.js index 28df816d..e43c0b46 100644 --- a/src/path/Path.js +++ b/src/path/Path.js @@ -59,6 +59,15 @@ var Path = this.Path = PathItem.extend({ return this._segments[this._segments.length - 1]; }, + getFirstCurve: function() { + return this.getCurves()[0]; + }, + + getLastCurve: function() { + var curves = this.getCurves(); + return curves[curves - 1]; + }, + // TODO: Consider adding getSubPath(a, b), returning a part of the current // path, with the added benefit that b can be < a, and closed looping is // taken into account. From 08d0499251409a73d705b3e70e63d322df7ad7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Lehni?= Date: Fri, 25 Mar 2011 19:58:20 +0200 Subject: [PATCH 5/5] Change comment. --- src/path/Curve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path/Curve.js b/src/path/Curve.js index 9738eb50..1ee7d559 100644 --- a/src/path/Curve.js +++ b/src/path/Curve.js @@ -301,7 +301,7 @@ var Curve = this.Curve = Base.extend({ } } - // Amount of integral evaluations + // Amount of integral evaluations for the interval 0 <= a < b <= 1 function getIterations(a, b) { // Guess required precision based and size of range... // TODO: There should be much better educated guesses for