microcharts-dotnet / Microcharts

Create cross-platform (Xamarin, Windows, ...) simple charts.
MIT License
2.03k stars 360 forks source link

Chart presumes that properties are always being changed on the UI thread. Exceptions are lost in async void PlanifyInvalidate. #245

Open twofingerrightclick opened 3 years ago

twofingerrightclick commented 3 years ago

So the offending structural set up is based around the fact that the ChartView presumes properties on a chart are always being changed on the main UI thread. This is of course not always the case. Especially as in the example of data coming from a polling task in issue 30 https://github.com/dotnet-ad/Microcharts/issues/30 Whenever a chart is going to be redrawn such as at: view.handler = view.chart.ObserveInvalidate(view, (v) => v.InvalidateSurface()); at line 63 in ChartView.cs there should be check as to what thread is being run and if not on the UI thread for Xamarin.Forms it should be Device.BeginInvokeOnMainThread(( => v.InvalidateSurface())) to make sure that the drawing occurs. The offending problem is that PlanifyInvalidate in Chart.cs is async void!!!! So you will never know why your app is crashing, when a property on a chart is changed from a thread other than the UI thread when it reaches Invalidate().

The simplest work around is make a "custom" chart by deriving from Chart, and just overriding OnPropertyChanged like so as observed by my colleague @Keboo :

/// <summary>
/// Invoked whenever a property changed.
/// </summary>
/// <param name="sender">Sender.</param>
/// <param name="e">E.</param>
protected override void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
    switch (e.PropertyName)
    {
        case nameof(AnimationProgress):
        case nameof(LabelTextSize):
        case nameof(MaxValue):
        case nameof(MinValue):
        case nameof(BackgroundColor):
            Device.BeginInvokeOnMainThread(() => base.OnPropertyChanged(sender, e));
            break;
        default:
            Device.BeginInvokeOnMainThread(() => base.OnPropertyChanged(sender, e));
            break;
    }
}

So that OnPropertyChanged Is ran on on the UI thread so that after the await in the async void PlanifyInvalidate lets the Invalidate() be called from the UI thread.

This is not production code, as it hogs the UI thread, but as we can't get at Invalidate calls such as the likes of view.handler = view.chart.ObserveInvalidate(view, (v) => v.InvalidateSurface()); at line 63 in ChartView.cs if you are using the nuget package and not the source code, this is a working option.

Originally posted by @twofingerrightclick in https://github.com/dotnet-ad/Microcharts/issues/30#issuecomment-780173662