Various fixes on handling of #strokeScaling and #strokeBounds calculations.

This commit is contained in:
Jürg Lehni 2016-01-17 23:57:56 +01:00
parent ea7216d9fb
commit 1ac8e46d55
4 changed files with 50 additions and 45 deletions

View file

@ -781,12 +781,12 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// required by the code that uses these methods internally, but make // required by the code that uses these methods internally, but make
// sure they can be cached like all the others as well. // sure they can be cached like all the others as well.
// Pass on the getter that these version actually use, untransformed, // Pass on the getter that these version actually use, untransformed,
// as internalGetter. // as `internal`.
// NOTE: These need to be versions of other methods, as otherwise the // NOTE: These need to be versions of other methods, as otherwise the
// cache gets messed up. // cache gets messed up.
var getter = 'get' + Base.capitalize(key), var getter = 'get' + Base.capitalize(key),
match = key.match(/^internal(.*)$/), match = key.match(/^internal(.*)$/),
internalGetter = match ? 'get' + match[1] : null; internal = match ? 'get' + match[1] : null;
this[getter] = function(_matrix) { this[getter] = function(_matrix) {
// TODO: If we're getting stroke based bounds (strokeBounds, // TODO: If we're getting stroke based bounds (strokeBounds,
// roughBounds, internalRoughBounds), and the object does not have // roughBounds, internalRoughBounds), and the object does not have
@ -798,11 +798,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// Allow subclasses to override _boundsGetter if they use the // Allow subclasses to override _boundsGetter if they use the
// same calculations for multiple type of bounds. // same calculations for multiple type of bounds.
// The default is getter: // The default is getter:
name = !internalGetter && (typeof boundsGetter === 'string' name = !internal && (typeof boundsGetter === 'string'
? boundsGetter : boundsGetter && boundsGetter[getter]) ? boundsGetter : boundsGetter && boundsGetter[getter])
|| getter, || getter,
bounds = this._getCachedBounds(name, _matrix, this, bounds = this._getCachedBounds(name, _matrix, this, internal);
internalGetter);
// If we're returning 'bounds', create a LinkedRectangle that uses // If we're returning 'bounds', create a LinkedRectangle that uses
// the setBounds() setter to update the Item whenever the bounds are // the setBounds() setter to update the Item whenever the bounds are
// changed: // changed:
@ -823,7 +822,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* Subclasses override it to define calculations for the various required * Subclasses override it to define calculations for the various required
* bounding types. * bounding types.
*/ */
_getBounds: function(getter, matrix, cacheItem) { _getBounds: function(getter, matrix, cacheItem, internal) {
// NOTE: We cannot cache these results here, since we do not get // NOTE: We cannot cache these results here, since we do not get
// _changed() notifications here for changing geometry in children. // _changed() notifications here for changing geometry in children.
// But cacheName is used in sub-classes such as PlacedSymbol and Raster. // But cacheName is used in sub-classes such as PlacedSymbol and Raster.
@ -844,7 +843,8 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
var child = children[i]; var child = children[i];
if (child._visible && !child.isEmpty()) { if (child._visible && !child.isEmpty()) {
var rect = child._getCachedBounds(getter, var rect = child._getCachedBounds(getter,
matrix && matrix.appended(child._matrix), cacheItem); matrix && matrix.appended(child._matrix),
cacheItem, internal);
x1 = Math.min(rect.x, x1); x1 = Math.min(rect.x, x1);
y1 = Math.min(rect.y, y1); y1 = Math.min(rect.y, y1);
x2 = Math.max(rect.x + rect.width, x2); x2 = Math.max(rect.x + rect.width, x2);
@ -881,13 +881,13 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* Private method that deals with the calling of _getBounds, recursive * Private method that deals with the calling of _getBounds, recursive
* matrix concatenation and handles all the complicated caching mechanisms. * matrix concatenation and handles all the complicated caching mechanisms.
*/ */
_getCachedBounds: function(getter, matrix, cacheItem, internalGetter) { _getCachedBounds: function(getter, matrix, cacheItem, internal) {
// See if we can cache these bounds. We only cache the bounds // See if we can cache these bounds. We only cache the bounds
// transformed with the internally stored _matrix, (the default if no // transformed with the internally stored _matrix, (the default if no
// matrix is passed). // matrix is passed).
matrix = matrix && matrix._orNullIfIdentity(); matrix = matrix && matrix._orNullIfIdentity();
// Do not transform by the internal matrix if there is a internalGetter. // Do not transform by the internal matrix if there is a internal getter
var _matrix = internalGetter ? null : this._matrix._orNullIfIdentity(), var _matrix = internal ? null : this._matrix._orNullIfIdentity(),
cache = (!matrix || matrix.equals(_matrix)) && getter; cache = (!matrix || matrix.equals(_matrix)) && getter;
// NOTE: This needs to happen before returning cached values, since even // NOTE: This needs to happen before returning cached values, since even
// then, _boundsCache needs to be kept up-to-date. // then, _boundsCache needs to be kept up-to-date.
@ -899,8 +899,8 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// getInternalBounds is getBounds untransformed. Do not replace earlier, // getInternalBounds is getBounds untransformed. Do not replace earlier,
// so we can cache both separately, since they're not in the same // so we can cache both separately, since they're not in the same
// transformation space! // transformation space!
var bounds = this._getBounds(internalGetter || getter, var bounds = this._getBounds(internal || getter, matrix || _matrix,
matrix || _matrix, cacheItem); cacheItem, internal);
// If we can cache the result, update the _bounds cache structure // If we can cache the result, update the _bounds cache structure
// before returning // before returning
if (cache) { if (cache) {
@ -908,7 +908,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
this._bounds = {}; this._bounds = {};
var cached = this._bounds[cache] = bounds.clone(); var cached = this._bounds[cache] = bounds.clone();
// Mark as internal, so Item#transform() won't transform it! // Mark as internal, so Item#transform() won't transform it!
cached._internal = !!internalGetter; cached._internal = !!internal;
} }
return bounds; return bounds;
}, },
@ -918,9 +918,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* when calculating bounds: the item's matrix if {@link #strokeScaling} is * when calculating bounds: the item's matrix if {@link #strokeScaling} is
* `true`, otherwise the shiftless, inverted view matrix. * `true`, otherwise the shiftless, inverted view matrix.
*/ */
_getStrokeMatrix: function(matrix) { _getStrokeMatrix: function(matrix, internal) {
return this.getStrokeScaling() ? matrix return this.getStrokeScaling() ? matrix
: this._parent.getViewMatrix().invert()._shiftless(); : (internal ? this : this._parent).getViewMatrix()
.invert()._shiftless();
}, },
statics: { statics: {
@ -1746,11 +1747,9 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// Calculate the transformed padding as 2D size that describes the // Calculate the transformed padding as 2D size that describes the
// transformed tolerance circle / ellipse. Make sure it's never 0 // transformed tolerance circle / ellipse. Make sure it's never 0
// since we're using it for division. // since we're using it for division.
tolerance = Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE),
tolerancePadding = options._tolerancePadding = new Size( tolerancePadding = options._tolerancePadding = new Size(
Path._getStrokePadding(1, strokeMatrix) Path._getStrokePadding(tolerance, strokeMatrix));
).multiply(
Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE)
);
// Transform point to local coordinates. // Transform point to local coordinates.
point = matrix._inverseTransform(point); point = matrix._inverseTransform(point);
// If the matrix is non-reversible, point will now be `null`: // If the matrix is non-reversible, point will now be `null`:
@ -1794,21 +1793,23 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
} }
} }
options._viewMatrix = viewMatrix;
options._strokeMatrix = strokeMatrix;
var children = !res && this._children; var children = !res && this._children;
if (children) { if (children) {
var opts = this._getChildHitTestOptions(options); var opts = this._getChildHitTestOptions(options);
opts._viewMatrix = viewMatrix;
// Loop backwards, so items that get drawn last are tested first // Loop backwards, so items that get drawn last are tested first
for (var i = children.length - 1; i >= 0 && !res; i--) for (var i = children.length - 1; i >= 0 && !res; i--)
res = children[i]._hitTest(point, opts); res = children[i]._hitTest(point, opts);
// Restore viewMatrix for next child, as opts === options sometimes.
opts._viewMatrix = parentViewMatrix;
} }
if (!res && checkSelf) if (!res && checkSelf)
res = this._hitTestSelf(point, options, strokeMatrix); res = this._hitTestSelf(point, options);
// Transform the point back to the outer coordinate system. // Transform the point back to the outer coordinate system.
if (res && res.point) if (res && res.point)
res.point = matrix.transform(res.point); res.point = matrix.transform(res.point);
// Restore viewMatrix for next child, so appended matrix chains are
// calculated correctly.
options._viewMatrix = parentViewMatrix;
return res; return res;
}, },

View file

@ -108,11 +108,12 @@ var PlacedSymbol = Item.extend(/** @lends PlacedSymbol# */{
}, },
_getBounds: function(getter, matrix, cacheItem) { _getBounds: function(getter, matrix, cacheItem, internal) {
var definition = this.symbol._definition; var definition = this.symbol._definition;
// Redirect the call to the symbol definition to calculate the bounds // Redirect the call to the symbol definition to calculate the bounds
return definition._getCachedBounds(getter, return definition._getCachedBounds(getter,
matrix && matrix.appended(definition._matrix), cacheItem); matrix && matrix.appended(definition._matrix),
cacheItem, internal);
}, },
_hitTestSelf: function(point, options) { _hitTestSelf: function(point, options) {

View file

@ -275,18 +275,18 @@ var Shape = Item.extend(/** @lends Shape# */{
return !(this.hasFill() && this.hasStroke()); return !(this.hasFill() && this.hasStroke());
}, },
_getBounds: function(getter, matrix) { _getBounds: function(getter, matrix, cacheItem, internal) {
var rect = new Rectangle(this._size).setCenter(0, 0), var rect = new Rectangle(this._size).setCenter(0, 0),
style = this._style, style = this._style,
strokeWidth = style.hasStroke() && strokeWidth = style.hasStroke() &&
/^getStrokeBounds$|^get.*RoughBounds$/.test(getter) && /^get(?:Stroke|Rough)Bounds$/.test(getter) &&
style.getStrokeWidth(); style.getStrokeWidth();
// If we're getting the strokeBounds, include the stroke width before // If we're getting the strokeBounds, include the stroke width before
// or after transforming the rect, based on strokeScaling. // or after transforming the rect, based on strokeScaling.
if (matrix) if (matrix)
rect = matrix._transformBounds(rect); rect = matrix._transformBounds(rect);
return strokeWidth ? rect.expand(Path._getStrokePadding( return strokeWidth ? rect.expand(Path._getStrokePadding(
strokeWidth, this._getStrokeMatrix(matrix))) : rect; strokeWidth, this._getStrokeMatrix(matrix, internal))) : rect;
} }
}, },
new function() { // Scope for _contains() and _hitTestSelf() code. new function() { // Scope for _contains() and _hitTestSelf() code.

View file

@ -1518,7 +1518,7 @@ var Path = PathItem.extend(/** @lends Path# */{
toPath: '#clone', toPath: '#clone',
_hitTestSelf: function(point, options, strokeMatrix) { _hitTestSelf: function(point, options) {
var that = this, var that = this,
style = this.getStyle(), style = this.getStyle(),
segments = this._segments, segments = this._segments,
@ -1533,23 +1533,23 @@ var Path = PathItem.extend(/** @lends Path# */{
hitStroke = options.stroke && style.hasStroke(), hitStroke = options.stroke && style.hasStroke(),
hitFill = options.fill && style.hasFill(), hitFill = options.fill && style.hasFill(),
hitCurves = options.curves, hitCurves = options.curves,
radius = hitStroke strokeRadius = hitStroke
? style.getStrokeWidth() / 2 ? style.getStrokeWidth() / 2
// Set radius to 0 when we're hit-testing fills with // Set radius to 0 when we're hit-testing fills with
// tolerance, to handle tolerance through stroke hit-test // tolerance, to handle tolerance through stroke hit-test
// functionality. Also use 0 when hit-testing curves. // functionality. Also use 0 when hit-testing curves.
: hitFill && options.tolerance > 0 || hitCurves : hitFill && options.tolerance > 0 || hitCurves
? 0 : null; ? 0 : null;
if (radius !== null) { if (strokeRadius !== null) {
if (radius > 0) { if (strokeRadius > 0) {
join = style.getStrokeJoin(); join = style.getStrokeJoin();
cap = style.getStrokeCap(); cap = style.getStrokeCap();
miterLimit = radius * style.getMiterLimit(); miterLimit = strokeRadius * style.getMiterLimit();
// Add the stroke radius to tolerance padding, taking // Add the stroke radius to tolerance padding, taking
// #strokeScaling into account through _getStrokeMatrix(). // #strokeScaling into account through _getStrokeMatrix().
strokePadding = tolerancePadding.add( strokePadding = tolerancePadding.add(
Path._getStrokePadding(radius, Path._getStrokePadding(strokeRadius,
!style.getStrokeScaling() && strokeMatrix)); !style.getStrokeScaling() && options._strokeMatrix));
} else { } else {
join = cap = 'round'; join = cap = 'round';
} }
@ -1606,11 +1606,12 @@ var Path = PathItem.extend(/** @lends Path# */{
if (join !== 'round' && (segment._handleIn.isZero() if (join !== 'round' && (segment._handleIn.isZero()
|| segment._handleOut.isZero())) || segment._handleOut.isZero()))
// _addBevelJoin() handles both 'bevel' and 'miter'! // _addBevelJoin() handles both 'bevel' and 'miter'!
Path._addBevelJoin(segment, join, radius, miterLimit, Path._addBevelJoin(segment, join, strokeRadius,
addToArea, true); miterLimit, addToArea, true);
} else if (cap !== 'round') { } else if (cap !== 'round') {
// It's a cap // It's a cap
Path._addSquareCap(segment, cap, radius, addToArea, true); Path._addSquareCap(segment, cap, strokeRadius, addToArea,
true);
} }
// See if the above produced an area to check for // See if the above produced an area to check for
if (!area.isEmpty()) { if (!area.isEmpty()) {
@ -1639,7 +1640,7 @@ var Path = PathItem.extend(/** @lends Path# */{
return res; return res;
} }
// If we're querying for stroke, perform that before fill // If we're querying for stroke, perform that before fill
if (radius !== null) { if (strokeRadius !== null) {
loc = this.getNearestLocation(point); loc = this.getNearestLocation(point);
// Note that paths need at least two segments to have an actual // Note that paths need at least two segments to have an actual
// stroke. But we still check for segments with the radius fallback // stroke. But we still check for segments with the radius fallback
@ -2712,8 +2713,9 @@ new function() { // PostScript-style drawing commands
// Curve. But not all of them use all these parameters, and some define // Curve. But not all of them use all these parameters, and some define
// additional ones after. // additional ones after.
_getBounds: function(getter, matrix) { _getBounds: function(getter, matrix, cacheItem, internal) {
return Path[getter](this._segments, this._closed, this, matrix); return Path[getter](this._segments, this._closed, this, matrix,
internal);
}, },
// Mess with indentation in order to get more line-space below: // Mess with indentation in order to get more line-space below:
@ -2764,14 +2766,14 @@ statics: {
* *
* @private * @private
*/ */
getStrokeBounds: function(segments, closed, path, matrix) { getStrokeBounds: function(segments, closed, path, matrix, internal) {
var style = path._style; var style = path._style;
if (!style.hasStroke()) if (!style.hasStroke())
return Path.getBounds(segments, closed, path, matrix); return Path.getBounds(segments, closed, path, matrix);
var length = segments.length - (closed ? 0 : 1), var length = segments.length - (closed ? 0 : 1),
strokeWidth = style.getStrokeWidth(), strokeWidth = style.getStrokeWidth(),
strokeRadius = strokeWidth / 2, strokeRadius = strokeWidth / 2,
strokeMatrix = path._getStrokeMatrix(matrix), strokeMatrix = path._getStrokeMatrix(matrix, internal),
strokePadding = Path._getStrokePadding(strokeWidth, strokeMatrix), strokePadding = Path._getStrokePadding(strokeWidth, strokeMatrix),
// Start with normal path bounds with added stroke padding. Then we // Start with normal path bounds with added stroke padding. Then we
// only need to look at each segment and handle join / cap / miter. // only need to look at each segment and handle join / cap / miter.
@ -2971,14 +2973,15 @@ statics: {
* *
* @private * @private
*/ */
getRoughBounds: function(segments, closed, path, matrix) { getRoughBounds: function(segments, closed, path, matrix, internal) {
// Delegate to handleBounds, but pass on radius values for stroke and // Delegate to handleBounds, but pass on radius values for stroke and
// joins. Handle miter joins specially, by passing the largest radius // joins. Handle miter joins specially, by passing the largest radius
// possible. // possible.
var style = path._style, var style = path._style,
strokeRadius = style.hasStroke() ? style.getStrokeWidth() / 2 : 0, strokeRadius = style.hasStroke() ? style.getStrokeWidth() / 2 : 0,
joinRadius = strokeRadius, joinRadius = strokeRadius,
strokeMatrix = strokeRadius && path._getStrokeMatrix(matrix); strokeMatrix = strokeRadius &&
path._getStrokeMatrix(matrix, internal);
if (strokeRadius > 0) { if (strokeRadius > 0) {
if (style.getStrokeJoin() === 'miter') if (style.getStrokeJoin() === 'miter')
joinRadius = strokeRadius * style.getMiterLimit(); joinRadius = strokeRadius * style.getMiterLimit();