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

Add context check for __initRetinaScaling #7706

Closed

Conversation

andrejslogins
Copy link
Contributor

@andrejslogins andrejslogins commented Feb 19, 2022

Related to this #7705

In some cases, the context may not be defined.
Likely due to a function being called before the canvas is initialized, or another canvas instance is created in the old one's stead

@andrejslogins andrejslogins marked this pull request as draft February 19, 2022 12:41
@andrejslogins andrejslogins marked this pull request as ready for review February 19, 2022 12:55
@ShaMan123
Copy link
Contributor

ShaMan123 commented Feb 19, 2022

@andrejslogins I don't like this.
You don't know if the problem is at your end or not.
You'll be patching fabric with checks probably because you didn't find the source of the problem.
Investigate and return with insights and a repro or something so we can understand as well and give feedback.
This is bad practice.

Likely due to a function being called before the canvas is initialized, or another canvas instance is created in the old one's stead

Check your cleanup methods.
Check resizing events on canvas that may trigger this behavior.
Check initializing fabric more than once and throw an error if so (likely caused from some event you are listening to).

@asturur
Copy link
Member

asturur commented Feb 20, 2022

yes i would like to know how do we get to upperCanvas or lowerCanvas without the context.
When we do create them they are assigned immedialtey.

With the previous case of cacheContext that was a little bit less steady, because the cache gets created on the fly and during resizing or other events you may end up destroying and recreating it.

But this is less clear on how it happens

@andrejslogins
Copy link
Contributor Author

i'll close this for now, issue persists, but i'm not actively working on it atm

@ShaMan123
Copy link
Contributor

@andrejslogins try confirming your issue persists after patching fabric with #7775

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.

3 participants