Hi @bclehmann ,

if it remains in this form, then it is worth shortening to 1 line:

Unloaded += (s , e) => PlottableCountTimer.Stop();

Great catch @Orace.

If we do not allow the user to manually manipulate the Plottable list. We don't really need a timer. We can raise CountChange event manually, in all methods that can change it. But these are just thoughts of how one could do it.

And for this PR, is definitely a good fix. Who would have thought that in WPF controls can be loaded and unloaded multiple times?

I'm not sure that the proposed implementation fix all corner cases for the memory leak. If we remove the parent element (like a grid) from it's parent (like a window) does the Unloaded event is called?

The proposed implementation may cause an other issue in my case (where I detected the memory leak too). I use a wpf objects tree to create a XPS report file (later converted to pdf). The wpf tree is never connected to a window and so I'm not even sure that the Loaded / Unloaded events are ever fired.

The implementation should get rid of the DispatcherTimer. If throttling is mandatory, I can propose an implementation based on Task that doesn't stuck in memory.

Hi @bclehmann, @StendProg, and @Orace 👋 I've been busy traveling and working on some personal projects but I anticipate having more time to work on ScottPlot again.

Intended Behavior (and why I used a timer)

To get us all on the same page, the core problem this timer thing is trying to achieve is:

• Automatically render once when the control is first populated with data
• Automatically re-plot when the number of plottables changes

I'm not using a CountChange event to trigger a render when the list of plottables is changed because:

• If the user adds many things to their plot (e.g., 10 scatter plots) when their program starts, ten independent renders will occur, slowing their start-up time significantly.
• If the user scatter plots then customizes the title, the render will be triggered before the title is updated.

When should the timer be started?

I believe that the PlottableCountTimer should be started on the Loaded event, otherwise the timer will not be restarted after the first unload event

This makes sense. This PR could be modified to achieve this. Similar functionality may need to be added to the WinForms and Avalonia controls.

Alternative Solution (use the render queue?)

Maybe instead of using a timer in the control to trigger plottable count checks and renders, a change to the plottables list could fire an event that adds add a fast delayed render (10 ms) to the queue. @StendProg you may be the most experienced with this system, what do you think of this idea?

I think if we were to ever consider automatically updating when the user changes plot properties or data then the render queue would be the most extensible solution that's been brought up. However, you could also have a system where these updates invoke a render more directly, which would probably make removing unnecessary renders easier.

I'm thinking of a simple debouncer, where it executes some time after the last time it was invoked. A smart delay is probably ~16ms so it's only 1 frame. I don't think we'd need to manually flush as I'd be very impressed if someone managed to continuously lock up this function for even one frame.

In principle, you could have both, when a proactive render is pushed to the render queue it doesn't render directly but simply calls a debounced render method. This should simplify things as we don't have to manage two different queue-like things, they can be treated exactly the same save for the handler at the other end of the queue.

For what it's worth, I still maintain that the inconsistency introduced by having some updates be automatically rendered but not others serves to confuse the API. One still ought to call Render, however this will be hidden in most situations. Instead of removing the bug from the user's code we have hidden it, and thus made it much more difficult to diagnose.

And in my opinion, having it update on timer instead of using an observable collection will normally mean that adjusting properties will be reflected in the automatic render. But it also introduces a race condition where the render happens after adding the plottable but before adjusting the properties. While adjusting virtually all properties should be very fast and this bad path should be very rare, it's an unnecessary bug which is obscenely difficult to even reproduce.

And it's not unforeseeable that we have more complicated properties that can be slower to update, such as if we were to have properties that caused heatmaps to be renormalized. Such a thing would make racing the two codepaths much more likely.

Implementation Using the Render Queue

simple debouncer ... where it executes some time after the last time it was invoked

I think something similar is in place, and this could be extended to provide the 16ms functionality you recommend. EventsProcessor.cs has a "delayed render timer" and when it crosses a threshold a render is performed. Requests reset the timer so two requests in a row will just reset the timer twice and result in a single render. Right now the threshold is a fixed number (RenderDelayMilliseconds) but this system could be modified to support different delays.

Should Automatic Render() be Disabled Entirely?

For what it's worth, I still maintain that the inconsistency introduced by having some updates be automatically rendered but not others serves to confuse the API. One still ought to call Render, however this will be hidden in most situations. Instead of removing the bug from the user's code we have hidden it, and thus made it much more difficult to diagnose.

And in my opinion, having it update on timer instead of using an observable collection will normally mean that adjusting properties will be reflected in the automatic render. But it also introduces a race condition where the render happens after adding the plottable but before adjusting the properties. While adjusting virtually all properties should be very fast and this bad path should be very rare, it's an unnecessary bug which is obscenely difficult to even reproduce.

And it's not unforeseeable that we have more complicated properties that can be slower to update, such as if we were to have properties that caused heatmaps to be renormalized. Such a thing would make racing the two codepaths much more likely.

This is very well stated! There are multiple excellent points here. In an effort to simplify the basic case, the advanced case has become more confusing and harder to figure out. I'm struggling to figure out the best way to proceed...

Reviewing the Original Problem

formsPlot1.Plot.AddScatter(xs, ys);
formsPlot1.Plot.Title("Example");
formsPlot1.Render(); // <-- this line is required if a start-up render is not automatic

If an automatic render does not occur after the control is loaded, and the user does not know that render must be requested manually after setting-up the plot, the plot will appear empty when the application launches. When the user clicks or drags the plot data will suddenly appear. This is confusing and astonishing.

To achieve the intended behavior the user must manually request a render after setting-up the plot. This requires user knowledge: "When using ScottPlot controls you must call Render() after modifying the plot or its data." This fact is key to using ScottPlot in graphical environments, but doesn't apply to console applications and is not evident in the cookbook. If there were a way to ensure users know this, the render timer could be eliminated.

The quickstart website documentation could show this, but that page is easy to overlook. Another option could be to raise a pop-up readme file when users install the control alerting them of this fact. I find those a bit annoying, and the primary ScottPlot library already raises one, but it might be less annoying to users than trying to figure this out on their own.

Are there any other tricks I didn't think of we could use to maximize discoverability of the extra render step if the automatic render feature is eliminated?

NPM packages often give warnings or deprecation notices in the console? We could detect if they didn't call render and then log?

It's not particularly visible, but one could go further and draw text on the plottable or even throw if Debugger.IsAttached returned true. Maybe it could even be wrapped in with diagnostic mode.

one could go further and draw text on the plottable if Debugger.IsAttached

This is an interesting idea! I think I prefer this option over console debug messages and pop-up readmes 🤔

I don't want to make this decision quickly (it would be a breaking change), but I'm liking the idea of removing the complex/confusing automatic render system, and this API change and warning message may be the lesser of two evils.

The implementation should get rid of the DispatcherTimer

This PR is an improvement so I'll merge it now. Thank you everyone for your input!

Eventually I will remove this timer completely and mandate that the user call Render() at least once (#1165) which is a better long-term solution for the core problem the timer was originally intended to solve.