beto-rodriguez / LiveCharts2

Simple, flexible, interactive & powerful charts, maps and gauges for .Net, LiveCharts2 can now practically run everywhere Maui, Uno Platform, Blazor-wasm, WPF, WinForms, Xamarin, Avalonia, WinUI, UWP.
https://livecharts.dev
MIT License
4.38k stars 572 forks source link

Setting the fill on a series disposes of the old paint, meaning you can't use a singleton paint object even if it makes sense #1614

Closed ryanmolden closed 4 weeks ago

ryanmolden commented 1 month ago

My situation is this:

I have a livecharts chart hosted in an app that has its own theming.

I have several radial gauge pie series that have their fill calculated based on usage percentage of the thing they are reprresenting. When the theme changes I need to (potentially) change the fill color of these items.

I had (psuedo) code to do this that looked like this (roughly)

foreach ISeries s in collectionOfGaugeSeriesFillsToUpdate s.Fill = CalculateFillColorForCurrentTheme(somePercentageValue);

Now, the interesting thing for me was in all themes my green color was the same. The other two colors could change with the theme.

What CalculateFillColorForCurrentTheme did was select amongst the three colors for 'ok', 'warning', and 'bad' based on the percentage passed in.

I naively had a singleton SolidColorPaint that I would use for Green ('ok'), and the other two colors would get changed over on theme change.

The problem is since it was possible for CalculateFillColorForCurrentTheme to return the singleton Green color sometimes the color change for a fill on theme change was basically a NOP as the old value for Fill and new value for Fill were literally the same SolidColorPaint.

I noticed a bug on theme change where the gauges would update but suddenly stop fill painting, but only if they were using the Green color.

I debugged into it, and it is because when you set the fill it adds the 'old' color to a queue of items to get deleted/disposed. It doesn't validate that old != new like most property setting systems with change notifications do. So now it sets a 'new' brush to use that it is about to delete/dispose of.

To Reproduce Steps to reproduce the behavior:

Create a radial gauge pie chart Set the fill to a SolidColorPaint (any will do) At some point (event/theme change/button press) do a NOP reset of the fill (i.e. set the Fill to the same SolidColorPaint object it is currently set to)

Expected behavior The set is basically a NOP, there is no change so nothing occurs, no events are raised, no paint objects are disposed of, nothing. Rendering keeps working as expected.

Desktop (please complete the following information): Windows 11, desktop, WPF app

beto-rodriguez commented 1 month ago

Fixed with the referenced commit and will be included in the next version of the library,

Thanks for the report!