Skip to content

Commit

Permalink
Various fixes on handling of #strokeScaling and #strokeBounds calcula…
Browse files Browse the repository at this point in the history 
…tions.
  • Loading branch information
@lehni
lehni committed Jan 17, 2016
1 parent ea7216d commit 1ac8e46
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 45 deletions.
47 changes: 24 additions & 23 deletions src/item/Item.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -781,12 +781,12 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// required by the code that uses these methods internally, but make
// sure they can be cached like all the others as well.
// 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
// cache gets messed up.
var getter = 'get' + Base.capitalize(key),
match = key.match(/^internal(.*)$/),
internalGetter = match ? 'get' + match[1] : null;
internal = match ? 'get' + match[1] : null;
this[getter] = function(_matrix) {
// TODO: If we're getting stroke based bounds (strokeBounds,
// roughBounds, internalRoughBounds), and the object does not have
Expand All @@ -798,11 +798,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// Allow subclasses to override _boundsGetter if they use the
// same calculations for multiple type of bounds.
// The default is getter:
name = !internalGetter && (typeof boundsGetter === 'string'
name = !internal && (typeof boundsGetter === 'string'
? boundsGetter : boundsGetter && boundsGetter[getter])
|| getter,
bounds = this._getCachedBounds(name, _matrix, this,
internalGetter);
bounds = this._getCachedBounds(name, _matrix, this, internal);
// If we're returning 'bounds', create a LinkedRectangle that uses
// the setBounds() setter to update the Item whenever the bounds are
// changed:
Expand All @@ -823,7 +822,7 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* Subclasses override it to define calculations for the various required
* 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
// _changed() notifications here for changing geometry in children.
// But cacheName is used in sub-classes such as PlacedSymbol and Raster.
Expand All @@ -844,7 +843,8 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
var child = children[i];
if (child._visible && !child.isEmpty()) {
var rect = child._getCachedBounds(getter,
matrix && matrix.appended(child._matrix), cacheItem);
matrix && matrix.appended(child._matrix),
cacheItem, internal);
x1 = Math.min(rect.x, x1);
y1 = Math.min(rect.y, y1);
x2 = Math.max(rect.x + rect.width, x2);
Expand Down Expand Up @@ -881,13 +881,13 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* Private method that deals with the calling of _getBounds, recursive
* 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
// transformed with the internally stored _matrix, (the default if no
// matrix is passed).
matrix = matrix && matrix._orNullIfIdentity();
// Do not transform by the internal matrix if there is a internalGetter.
var _matrix = internalGetter ? null : this._matrix._orNullIfIdentity(),
// Do not transform by the internal matrix if there is a internal getter
var _matrix = internal ? null : this._matrix._orNullIfIdentity(),
cache = (!matrix || matrix.equals(_matrix)) && getter;
// NOTE: This needs to happen before returning cached values, since even
// then, _boundsCache needs to be kept up-to-date.
Expand All @@ -899,16 +899,16 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// getInternalBounds is getBounds untransformed. Do not replace earlier,
// so we can cache both separately, since they're not in the same
// transformation space!
var bounds = this._getBounds(internalGetter || getter,
matrix || _matrix, cacheItem);
var bounds = this._getBounds(internal || getter, matrix || _matrix,
cacheItem, internal);
// If we can cache the result, update the _bounds cache structure
// before returning
if (cache) {
if (!this._bounds)
this._bounds = {};
var cached = this._bounds[cache] = bounds.clone();
// Mark as internal, so Item#transform() won't transform it!
cached._internal = !!internalGetter;
cached._internal = !!internal;
}
return bounds;
},
Expand All @@ -918,9 +918,10 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
* when calculating bounds: the item's matrix if {@link #strokeScaling} is
* `true`, otherwise the shiftless, inverted view matrix.
*/
_getStrokeMatrix: function(matrix) {
_getStrokeMatrix: function(matrix, internal) {
return this.getStrokeScaling() ? matrix
: this._parent.getViewMatrix().invert()._shiftless();
: (internal ? this : this._parent).getViewMatrix()
.invert()._shiftless();
},

statics: {
Expand Down Expand Up @@ -1746,11 +1747,9 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
// Calculate the transformed padding as 2D size that describes the
// transformed tolerance circle / ellipse. Make sure it's never 0
// since we're using it for division.
tolerance = Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE),
tolerancePadding = options._tolerancePadding = new Size(
Path._getStrokePadding(1, strokeMatrix)
).multiply(
Math.max(options.tolerance, /*#=*/Numerical.TOLERANCE)
);
Path._getStrokePadding(tolerance, strokeMatrix));
// Transform point to local coordinates.
point = matrix._inverseTransform(point);
// If the matrix is non-reversible, point will now be `null`:
Expand Down Expand Up @@ -1794,21 +1793,23 @@ var Item = Base.extend(Emitter, /** @lends Item# */{
}
}

options._viewMatrix = viewMatrix;
options._strokeMatrix = strokeMatrix;
var children = !res && this._children;
if (children) {
var opts = this._getChildHitTestOptions(options);
opts._viewMatrix = viewMatrix;
// Loop backwards, so items that get drawn last are tested first
for (var i = children.length - 1; i >= 0 && !res; i--)
res = children[i]._hitTest(point, opts);
// Restore viewMatrix for next child, as opts === options sometimes.
opts._viewMatrix = parentViewMatrix;
}
if (!res && checkSelf)
res = this._hitTestSelf(point, options, strokeMatrix);
res = this._hitTestSelf(point, options);
// Transform the point back to the outer coordinate system.
if (res && 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;
},

Expand Down
5 changes: 3 additions & 2 deletions src/item/PlacedSymbol.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
// Redirect the call to the symbol definition to calculate the bounds
return definition._getCachedBounds(getter,
matrix && matrix.appended(definition._matrix), cacheItem);
matrix && matrix.appended(definition._matrix),
cacheItem, internal);
},

_hitTestSelf: function(point, options) {
Expand Down
6 changes: 3 additions & 3 deletions src/item/Shape.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,18 +275,18 @@ var Shape = Item.extend(/** @lends Shape# */{
return !(this.hasFill() && this.hasStroke());
},

_getBounds: function(getter, matrix) {
_getBounds: function(getter, matrix, cacheItem, internal) {
var rect = new Rectangle(this._size).setCenter(0, 0),
style = this._style,
strokeWidth = style.hasStroke() &&
/^getStrokeBounds$|^get.*RoughBounds$/.test(getter) &&
/^get(?:Stroke|Rough)Bounds$/.test(getter) &&
style.getStrokeWidth();
// If we're getting the strokeBounds, include the stroke width before
// or after transforming the rect, based on strokeScaling.
if (matrix)
rect = matrix._transformBounds(rect);
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.
Expand Down
37 changes: 20 additions & 17 deletions src/path/Path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1518,7 +1518,7 @@ var Path = PathItem.extend(/** @lends Path# */{

toPath: '#clone',

_hitTestSelf: function(point, options, strokeMatrix) {
_hitTestSelf: function(point, options) {
var that = this,
style = this.getStyle(),
segments = this._segments,
Expand All @@ -1533,23 +1533,23 @@ var Path = PathItem.extend(/** @lends Path# */{
hitStroke = options.stroke && style.hasStroke(),
hitFill = options.fill && style.hasFill(),
hitCurves = options.curves,
radius = hitStroke
strokeRadius = hitStroke
? style.getStrokeWidth() / 2
// Set radius to 0 when we're hit-testing fills with
// tolerance, to handle tolerance through stroke hit-test
// functionality. Also use 0 when hit-testing curves.
: hitFill && options.tolerance > 0 || hitCurves
? 0 : null;
if (radius !== null) {
if (radius > 0) {
if (strokeRadius !== null) {
if (strokeRadius > 0) {
join = style.getStrokeJoin();
cap = style.getStrokeCap();
miterLimit = radius * style.getMiterLimit();
miterLimit = strokeRadius * style.getMiterLimit();
// Add the stroke radius to tolerance padding, taking
// #strokeScaling into account through _getStrokeMatrix().
strokePadding = tolerancePadding.add(
Path._getStrokePadding(radius,
!style.getStrokeScaling() && strokeMatrix));
Path._getStrokePadding(strokeRadius,
!style.getStrokeScaling() && options._strokeMatrix));
} else {
join = cap = 'round';
}
Expand Down Expand Up @@ -1606,11 +1606,12 @@ var Path = PathItem.extend(/** @lends Path# */{
if (join !== 'round' && (segment._handleIn.isZero()
|| segment._handleOut.isZero()))
// _addBevelJoin() handles both 'bevel' and 'miter'!
Path._addBevelJoin(segment, join, radius, miterLimit,
addToArea, true);
Path._addBevelJoin(segment, join, strokeRadius,
miterLimit, addToArea, true);
} else if (cap !== 'round') {
// 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
if (!area.isEmpty()) {
Expand Down Expand Up @@ -1639,7 +1640,7 @@ var Path = PathItem.extend(/** @lends Path# */{
return res;
}
// If we're querying for stroke, perform that before fill
if (radius !== null) {
if (strokeRadius !== null) {
loc = this.getNearestLocation(point);
// Note that paths need at least two segments to have an actual
// stroke. But we still check for segments with the radius fallback
Expand Down Expand Up @@ -2712,8 +2713,9 @@ new function() { // PostScript-style drawing commands
// Curve. But not all of them use all these parameters, and some define
// additional ones after.

_getBounds: function(getter, matrix) {
return Path[getter](this._segments, this._closed, this, matrix);
_getBounds: function(getter, matrix, cacheItem, internal) {
return Path[getter](this._segments, this._closed, this, matrix,
internal);
},

// Mess with indentation in order to get more line-space below:
Expand Down Expand Up @@ -2764,14 +2766,14 @@ statics: {
*
* @private
*/
getStrokeBounds: function(segments, closed, path, matrix) {
getStrokeBounds: function(segments, closed, path, matrix, internal) {
var style = path._style;
if (!style.hasStroke())
return Path.getBounds(segments, closed, path, matrix);
var length = segments.length - (closed ? 0 : 1),
strokeWidth = style.getStrokeWidth(),
strokeRadius = strokeWidth / 2,
strokeMatrix = path._getStrokeMatrix(matrix),
strokeMatrix = path._getStrokeMatrix(matrix, internal),
strokePadding = Path._getStrokePadding(strokeWidth, strokeMatrix),
// Start with normal path bounds with added stroke padding. Then we
// only need to look at each segment and handle join / cap / miter.
Expand Down Expand Up @@ -2971,14 +2973,15 @@ statics: {
*
* @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
// joins. Handle miter joins specially, by passing the largest radius
// possible.
var style = path._style,
strokeRadius = style.hasStroke() ? style.getStrokeWidth() / 2 : 0,
joinRadius = strokeRadius,
strokeMatrix = strokeRadius && path._getStrokeMatrix(matrix);
strokeMatrix = strokeRadius &&
path._getStrokeMatrix(matrix, internal);
if (strokeRadius > 0) {
if (style.getStrokeJoin() === 'miter')
joinRadius = strokeRadius * style.getMiterLimit();
Expand Down

0 comments on commit 1ac8e46

Please sign in to comment.