CanvasRenderer: null is not an object (evaluating 't._ghostContext.imageSmoothingEnabled=!1') after 5.1.1

amcharts / amcharts5

The newest, fastest, and most advanced amCharts charting library for JavaScript and TypeScript apps.

Other

351 stars 94 forks source link

CanvasRenderer: null is not an object (evaluating 't._ghostContext.imageSmoothingEnabled=!1') after 5.1.1 #254

Closed ghost closed 2 years ago

ghost commented 2 years ago

https://github.com/amcharts/amcharts5/issues/235 still persists after 5.1.1

React App crashes

Would something like this work?

martynasma commented 2 years ago

Would something like this work?

No. I mean it would alleviate the error, but the chart would still be broken. Possibly in some wonky non-apparent way. In my opinion that's even worse.

Does the same chart break outside of React?

I'm now thinking that maybe React is doing some disposing/recreating of charts which piles up canvas elements, hitting that damn new iOS Safari canvas GPU limit, causing this.

martynasma commented 2 years ago

Heads up, we've just released 5.1.2 which adds some workarounds for iOS canvas GC bug.

If the theory I described above pans out, this should help.

Let me know how it goes.

ghost commented 2 years ago

Heads up, we've just released 5.1.2 which adds some workarounds for iOS canvas GC bug.

If the theory I described above pans out, this should help.

Let me know how it goes.

Thank you for patching and releasing in such short notice. This fit right into deploy window today.

It's fixed! We can now render 25 gauge charts in a modal, and spam open/close the modal and the charts render each time.

I wonder now if it's necessary for the lower DPI since I understand the solution was related to dispose. It would be nice to have an option to use (unsafe?) device DPI or use the default recommended DPI. Chart text is blurry, but it's an acceptable trade-off for having charts successfully render and our app not crash. I'm sure you have other problems to solve in the interim.

We look forward to iOS improvements in the future as your team is doing an awesome job with v5. Thanks again for your time!

martynasma commented 2 years ago

I wonder now if it's necessary for the lower DPI since I understand the solution was related to dispose. It would be nice to have an option to use (unsafe?)

It's necessary. Without it, our tests have been erroring out on larger charts/screens even without any dispose going on.

Though, introducing a threshold setting is a good idea. For example number of pixels per Root. If chart is smaller than the threshold, resolution reduction does not kick in.

We'll give it some though. It breaks my heart to see reduced rez on otherwise sharp screens. I wonder why Apple is forcing this on us. The stuff must've affected thousands of libs and websites that rely on canvas rendering :(

martynasma commented 2 years ago

Implemented in 5.1.3.

[5.1.3] - 2022-02-03

Added

Added second parameter to Root constructor which contains settings object. For now, there's only one setting: useSafeResolution (default: true). More info.

Fixed

Dynamic data updates on MapPolygonSeries were not working in some cases.

Full change log.

Download options.

Make sure you clear your browser cache after upgrading. And feel free to contact us again if you are still experiencing this issue.

martynasma commented 2 years ago

So it's a boolean setting for now.