toDataURL doubles shadow offset on objects

fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser

http://fabricjs.com

Other

29.11k stars 3.52k forks source link

toDataURL doubles shadow offset on objects #5577

Closed anthify closed 5 years ago

anthify commented 5 years ago

Version

2.7.0

Test Case

https://jsfiddle.net/5Lgw20dj/

Information about environment

Nodejs or browser?

Both Which browsers?
Chrome

Steps to reproduce

Create canvas with object which has a shadow and then call toDataURL() with any options.

Expected Behavior

Shadow's offset to be the same as the document

Actual Behavior

Values are doubled (see jsfiddle example)

asturur commented 5 years ago

https://jsfiddle.net/9s3rn2Lc/

Looks like a retina scaling regression. Needs a fix and a visual test.

anthify commented 5 years ago

@asturur thanks that works. I stepped through the functions when toDataURL is called but I found no culprit. If there is a bit of code you want me to look at, let me know!

asturur commented 5 years ago

I'm not exactly sure where the problem is. This wasn't a bug before, but recently i modifie the dataUrl functionalities and this broke because there was not a visual test ready to warn me that something changed.

I guess that also moving to a different test environment with mocks ( like jest ) would help to write better unit tests.

is something in the toCanvasElement function in the dataurl export mixin in mixins folder.

asturur commented 5 years ago

if you still have time and want to fix this, i think the problem is in the toCanvasElement function.

We need to read the value of the canvas enableRetinaScaling, set it to false, and after the render operation we need to set it back to the original value.

Let me know if you can do it, or i'll fix it.

anthify commented 5 years ago

@asturur I should have time to fix it next week, but go ahead if have time to do it sooner. Feel free to tag me in the PR so I can look.