Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strokeUniform property #5473

Merged
merged 3 commits into from
Feb 16, 2019
Merged

Conversation

stefanhayden
Copy link
Member

new Object property strokeUniform allows the stroke width to always stay the same width as object scales.

This code has the stroke staying uniform but I'm still doing something wrong with the bounding box as it's still growing and shrinking as if the stroke would be scaling.

example5

src/shapes/object.class.js Outdated Show resolved Hide resolved
Co-Authored-By: stefanhayden <alt255@gmail.com>
@asturur
Copy link
Member

asturur commented Jan 12, 2019

This will be for sure a multi pr feature. I would start with this. I m adding a couple of tests and merge it
I ll follow up with control box and then svg import/export

@asturur
Copy link
Member

asturur commented Jan 12, 2019

is strokeUniform a good name?

@asturur
Copy link
Member

asturur commented Jan 12, 2019

and text, we need to workout a way for text. Because in our prototype we did not need it for other reasons

@@ -592,8 +592,16 @@
if (skewX === 0 && skewY === 0) {
return { x: dimensions.x * this.scaleX, y: dimensions.y * this.scaleY };
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was supposed to be a shortcut to save on calculations... keeping it now kills the feature. Or maybe we need to reorganize a bit the code. Most of the peple work without the skew values, not point in going for 4 point * matrix + bbox calculations. But i can agree is not probably a bottleneck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this? would keep the shortcut somehow.

_getTransformedDimensions: function(skewX, skewY) {
      if (typeof skewX === 'undefined') {
        skewX = this.skewX;
      }
      if (typeof skewY === 'undefined') {
        skewY = this.skewY;
      }
      var dimensions = this._getNonTransformedDimensions(), dimX, dimY,
            noSkew = skewX === 0 && skewY === 0;

      if (this.strokeUniform) {
        dimX = this.width;
        dimY = this.height;
      }
      else {
        dimX = dimensions.x;
        dimY = dimensions.y;
      }
      if (noSkew) {
       return _finalizeDiemensions(dimX, dimY);
      }
      else {
        dimX /= 2;
        dimY /= 2;
      }
      var points = [
            {
              x: -dimX,
              y: -dimY
            },
            {
              x: dimX,
              y: -dimY
            },
            {
              x: -dimX,
              y: dimY
            },
            {
              x: dimX,
              y: dimY
            }],
          i, transformMatrix = this._calcDimensionsTransformMatrix(skewX, skewY, false),
          bbox;
      for (i = 0; i < points.length; i++) {
        points[i] = fabric.util.transformPoint(points[i], transformMatrix);
      }
      bbox = fabric.util.makeBoundingBoxFromPoints(points);
      return this._finalizeDiemensions(bbox.width, bbox.height);
    },

    _finalizeDiemensions(width, height) {
      return this.strokeUniform ?
        { x: width + this.strokeWidth, y: height + this.strokeWidth }
        :
        { x: width, y: height };
    }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup that makes sense.

@asturur
Copy link
Member

asturur commented Feb 16, 2019

need to finish this but i broke my branches, merging this and fixing.

@asturur asturur merged commit 850c617 into fabricjs:master Feb 16, 2019
@asturur asturur mentioned this pull request Mar 2, 2019
thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
* new Object property strokeUniform allows the stroke width to always stay the same width as object scales

* fix for height

* Update src/shapes/object.class.js

Co-Authored-By: stefanhayden <alt255@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants