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

Feature Request : Add option to size Object strokeWidth independently of scale #2012

Closed
gordyr opened this issue Mar 4, 2015 · 11 comments
Closed

Comments

@gordyr
Copy link
Contributor

gordyr commented Mar 4, 2015

In most visual editors when you set a border or strokeWidth of an object such as a circle or rectangle the width is maintained at its correct pixel dimensions as you scale the object with the controls.

e.g If I draw a rectangle and set its border (strokeWidth) to 2px, then drag the corner controls so that it is then 3 times its original size, the width of the border outline would remain at 2px.

I understand the current behaviour and believe in many scenarios it is desirable. however for many others it is certainly not.

I would like to propose adding a property to the Object class perhaps something like lockStrokeWidth which would then implement the suggested behaviour.

This would also be very useful when dealing with text strokes in IText.

As far as I am aware this should be reasonably simple to approximate by simply dividing by the objects scale (Math.max(this.getScaleX(), this.getScaleY())) properties in _setStrokeStyles and also _calculateCurrentDimensions. which would work very well for most objects.

However a further improvement for the shape classes such like Rect and Circle would be to have the controls actually set the width and height properties of the objects themselves while forcing the scale properties to 1 to ensure consistency with modern native editors.

Any thoughts on this?

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@asturur
Copy link
Member

asturur commented Mar 4, 2015

i tell you clearly my opinion, i do not like this.
fortunately i m not the one who leads the project.

In short i would be more for a solution like this:

Every corner as a configurable icon and function.
Then if you want to resize your object and not scale them, you ll do your proper width and height change related to mouse movement.
You want bigger font and small strokewidth, you will configure the corner to have a font size effect putting plus and minus icon on corners.
On other size if we are scaling the object we are scaling everything.
Also consider more complex scenario in which the border is filled with gradient, we make the border thinner but the gradient will be scaled with canvas.....

Also i m against something that is called lockStrokeWidth and actually change the strokeWidth 60 times per second when you resize the object.

@asturur
Copy link
Member

asturur commented Mar 4, 2015

i m aware the fact that a square, pulled from one side to be a rect, becomes a terrible 2 different sized borders object. but i do not think that a simple true / false var is the solution. a more configurable scenario will solve this defect and more request in future.

@gordyr
Copy link
Contributor Author

gordyr commented Mar 4, 2015

@asturur just for you information, my propososal of lockStrokeWidth wouldn't be constantly changing the strokeWidth exactly. It would simply be changing the way in which it is currently calculated. I have monkeypatched it with a single line of code and it appears to work perfectly. Though a better solution was proposed by both of us. Though to be honest these shouldn't be hard to hack either.

It's annoying however. My current version of fabric contains far too much in the way of extensions and patches in order to get my desired behaviour. And most of it is to make fabric closer to the behaviour of commercial editors like adobe illustrator etc. It would be great to get some of these things in the core library.

@asturur
Copy link
Member

asturur commented Mar 4, 2015

i know i know, that s why i m for a more configurable option other than another bool property. but anyway i m just a frequent contributor, and so, as you i m telling my opinion.

@gordyr
Copy link
Contributor Author

gordyr commented Mar 4, 2015

@asturur indeed. I agree... All I care about is getting this kind of natural/native app behaviour into the core library. Whether it be via individual object properties or by global options as you suggest doesn't matter to me.

@kangax
Copy link
Member

kangax commented Mar 5, 2015

I'm torn between adding even more complexity to Fabric and having a pretty common/useful feature such as this one (there were few requests for it in the past as well). Marking as "possible feature" for now.

@gordyr
Copy link
Contributor Author

gordyr commented Mar 17, 2015

@kangax @asturur

After a bit of research i've found that this is actually extremely easy to implement perfectly.

All it requires is that the transform is restored before the actual stroke.

Therefore this could be implemented for each shape/object individually with a single call to ctx.restore in each objects class just before calling _renderStroke:

this._renderStroke();

Or it could be implemented globally by modifying fabrics _renderStroke function.

Also a small modification to _calcDimensions would be required so the the objects hit area and coords are correctly calculated.

I've just tried it out by hacking the fabric source code directly in the Polygon class and it works perfectly.

Take a look at this for more info:

http://www.bit-101.com/blog/?p=3690

@gordyr
Copy link
Contributor Author

gordyr commented Mar 17, 2015

@kangax Here's a quick demo with a couple of monkey patched functions

http://jsfiddle.net/2fs872ce/

Obviously this is using version 1.40 so the render function is slightly different than the latest release. Also in version 1.4 there is no _calculateDimensions function so the modification to account for the strokeWidth scale when calculating object coordinates would have to go in setCoords instead.... I didn't bother implementing it in this demo. But it is very simple to do so.

Better to implement it on a per object basis though in my opinion.

All it requires is restoring the context just before applying the stroke. Then reapplying the previous transform just before drawing the controls.

@kangax
Copy link
Member

kangax commented Mar 25, 2015

@gordyr feel free to send a PR, and I'll take a look at it :)

@antucg
Copy link
Contributor

antucg commented Dec 2, 2016

Hi guys, I have been fighting against this for the last couple of days. After a lot of tries I could make it work.
Here is the code with the example: https://jsfiddle.net/m9ywmphs/5/
In case of groups, both the group and the objects within it, have to be rescaled.
I hope this can help you.
Good luck.

@jmlop
Copy link

jmlop commented Dec 17, 2016

Hello, thanks all for your help.
I've been trying to solve the problem with stroke scaling and I'm not capable.
I followed the advice, seemingly simple, of the friend Gordyr but I have not reached any result.
Some other advice??
That's my code: https://jsfiddle.net/jmlop/t03ohewm/2/

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

No branches or pull requests

5 participants