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

Textbox w/ stroke width toDataUrl has slightly different offset compared to canvas #2991

Closed
ghost opened this issue May 20, 2016 · 6 comments · Fixed by #2992
Closed

Textbox w/ stroke width toDataUrl has slightly different offset compared to canvas #2991

ghost opened this issue May 20, 2016 · 6 comments · Fixed by #2992
Labels
Milestone

Comments

@ghost
Copy link

ghost commented May 20, 2016

Version

1.6.2

Test Case

http://jsfiddle.net/7dq9ucyb/2/

Check the console of this jsfiddle to see the different calculated dimensions.

Steps to reproduce

  1. Create a textbox with stroke width of 0
  2. Call toDataURL on textbox
  3. The data URL image and the original textbox are positioned exactly the same relative to their top-left corners.
  4. Create the same textbox again but with stroke width of 8.
  5. Call toDataURL on textbox.

Expected Behavior

The data URL and the canvas textbox match position exactly, relative to their respective top left corners.

Actual Behavior

The data URL image and the textbox are positioned differently. The textbox appears to be "offset" by a few pixels when compared to the image.

In the below GIF, you can see the data URL image with a slightly darker background. When the textbox is selected, it has a slightly lighter background. This gif has the stroke width at 8.

text_stroke_offset

@asturur
Copy link
Member

asturur commented May 21, 2016

http://jsfiddle.net/7dq9ucyb/5/
This is an updated version of the fiddle with simplified code. The bug does not show up visually.
there is still a 0.5 pixel difference in the code (357 357.49 357.49 357.49) that comes from the image not being able to have less that one pixel increments.

When using padding there is a bigger difference anyway because the dat url of the object includes padding. That may be considered a bug or not.

@asturur
Copy link
Member

asturur commented May 21, 2016

ohh is see something. I was looking the gif long enough, before getting completely hypnotized i see that in your dataURL there is some scaling artifact. The image is not being applied as it is, but is getting some resize process.

@ghost
Copy link
Author

ghost commented May 21, 2016

Yes, you're right about the scaling.

This behaviour isn't visible when strokeWidth=0 and everything else is the same. The scaling doesn't cause this (I don't think).

I will spend some time trying to reproduce the problem more clearly.

@ghost
Copy link
Author

ghost commented May 21, 2016

I've tried making it a little clearer with further examples. In each jsfiddle below I overlay the captured image directly on top of the canvas that generates it.

  1. Text without stroke, jsfiddle: http://jsfiddle.net/7pkpbcm6/5/

without_stroke

  1. Text with stroke, jsfiddle: http://jsfiddle.net/7pkpbcm6/6/

with_stroke

What are your thoughts? I understand it's a very minor difference & if you don't think it's worth pursuing in fabric. I can try to compensate for this at the application layer instead.

@asturur
Copy link
Member

asturur commented May 21, 2016

bang!
http://jsfiddle.net/7pkpbcm6/7/

is a bug.

I mean, normally we calculate the bounding box of an object. The rect that is 100 width and has a stroke of 1, will be 101 width.

The line that is long 100 and has a stroke of 10, will be 100 or 110 depending of is strokeLineCap.

But the text gets a width, that is calculated by measure text already including strokes. So we should not touch it anymore. text classes needs an override of the basic dimensions function. ( and so needs line, so all the other are simpler )

@ghost
Copy link
Author

ghost commented May 21, 2016

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant