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

toDataURL doubles shadow offset on objects #5577

Closed
anthify opened this issue Mar 16, 2019 · 5 comments · Fixed by #5593
Closed

toDataURL doubles shadow offset on objects #5577

anthify opened this issue Mar 16, 2019 · 5 comments · Fixed by #5593

Comments

@anthify
Copy link
Contributor

anthify commented Mar 16, 2019

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
Copy link
Member

asturur commented Mar 16, 2019

https://jsfiddle.net/9s3rn2Lc/

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

@asturur asturur added the bug label Mar 16, 2019
@anthify
Copy link
Contributor Author

anthify commented Mar 16, 2019

@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
Copy link
Member

asturur commented Mar 18, 2019

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
Copy link
Member

asturur commented Mar 23, 2019

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
Copy link
Contributor Author

anthify commented Mar 23, 2019

@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.

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

Successfully merging a pull request may close this issue.

2 participants