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

Fix problem on fabric.Line controls and line.getWidth() #3742

Merged
merged 2 commits into from
Feb 25, 2017

Conversation

vudduu
Copy link
Contributor

@vudduu vudduu commented Feb 24, 2017

There is a bug when creating a fabric.Line + strokeWidth.

  • There is a padding in the control.
  • getWidth returns width + strokeWidth.

DEMO MASTER BRANCH
DEMO WITH FIX

@asturur
Copy link
Member

asturur commented Feb 24, 2017

it has to be like that.
we cannot change this or a lot of things break.

@asturur
Copy link
Member

asturur commented Feb 24, 2017

There is an error,
that is this:

image

line has its own getDimensions function.
What is wrong is that we check for dim.x and dim.y while in that if we should check for this.width and this.height.

This should be fixed.
Would you do that?

@vudduu
Copy link
Contributor Author

vudduu commented Feb 24, 2017

I made the changes.
Now it adds the changes in the Line getDimensions function instead of the Objects one.

@asturur
Copy link
Member

asturur commented Feb 25, 2017

i m working a bit on it. there are problems.here and there importing svgs with lines.

i ll push up changes asap

@asturur
Copy link
Member

asturur commented Feb 25, 2017

To me the changes i committed to your branch looks like are fixing all cases ( horizontal line, vertical line, svg import )

@asturur
Copy link
Member

asturur commented Feb 25, 2017

<svg width="120" height="120" viewBox="0 0 120 120"
    xmlns="http://www.w3.org/2000/svg">

  <line x1="15" y1="15" x2="100" y2="15"
      stroke-width="30" stroke="black" stroke-linecap="round" />
    <line x1="0" y1="47" x2="115" y2="47"
      stroke-width="30" stroke="black" stroke-linecap="butt" />
      <line x1="8" y1="79" x2="115" y2="150"
      stroke-width="30" stroke="red" stroke-linecap="butt" />
</svg>

@asturur
Copy link
Member

asturur commented Feb 25, 2017

Please check them, i will merge them once you figure out if they solve what you noticed wrong.

@vudduu
Copy link
Contributor Author

vudduu commented Feb 25, 2017

Last changes solved the problem except for stroke-linecap="round"
DEMO LAST CHANGES - PROBLEM WITH ROUNDED LINES

@asturur
Copy link
Member

asturur commented Feb 25, 2017

what is that fiddle? i see a snake growing. where is the problem?

@asturur
Copy link
Member

asturur commented Feb 25, 2017

if you refer to the fact that getwidth is bigger than width, that is because getWidth should be named differently. i will just remove that method.

@asturur
Copy link
Member

asturur commented Feb 25, 2017

getWidth return the actual width, including scaling, skewing. is just a wrong named method.

it was a shortcut to get width * scaleX, now is a shortcut for getNonTransformedDimensions().x and soon removed.

we should merge this PR i think

@asturur asturur merged commit 1f55de0 into fabricjs:master Feb 25, 2017
asturur pushed a commit that referenced this pull request Feb 27, 2017
* feat(pr/fixLineWidth) Fix problem on fabric.Line controls and line.getWidth()

* fix line boxes and svg import
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.

2 participants