Changes pruning algorithm for audio visualizer. #7139
Conversation
- Selects paragraphs in the visible portion of the visualizer first to ensure you can see the current timeline. - Select paragraphs across the entire timeline before selecting multiple paragraphs that start at the same time. - After visible paragraphs are pruned, add some invisible ones to assist with scrolling (and prune again to limit rendered paragraphs). SubtitleEdit#7000
src/ui/Controls/AudioVisualizer.cs
Outdated
| * Group & select is done so that it draws paragraphs across the entire timeline before drawing overlapping paragraphs, up to the display limit. | ||
| * Materialize to list so that it can be counted below without pruning twice. | ||
| */ | ||
| displayableParagraphs = visibleParagraphs.Where(p => p.DurationTotalMilliseconds >= 0.01) |
There was a problem hiding this comment.
Too many LINQ. This can impact the performance!
Same for the rest of the code!
Every time you => an allocation will happen for action/func that is without considering the closure if you are using any variable from outside!
There was a problem hiding this comment.
Thanks for taking a look! I didn't think the performance impact was too large with LINQ but I don't mind rewriting it to use traditional loops.
After creating this PR I noticed there's a SecondsToXPosition method that looks better than grouping by the same number of milliseconds no matter the zoom level. So I'll get both of these changes in this PR.
There was a problem hiding this comment.
@niksedk is this something of your interest?
- Selects paragraphs in the visible portion of the visualizer first to ensure you can see the current timeline. - Select paragraphs across the entire timeline before selecting multiple paragraphs that start at the same time. - After visible paragraphs are pruned, add some invisible ones to assist with scrolling (and prune again to limit rendered paragraphs). SubtitleEdit#7000
…e coverage. Has a lot of debugging statements and is not optimized enough to be ready, but it's enough to save.
Caches coverage calculations to avoid iterating over the coverage records again. The results depend on the subtitles currently visible, but it sometimes cuts prune time from around 70 ms to around 35 ms (about 50% faster) and sometimes has little or no effect (66 ms to 50 ms) or even a bit slower (from 70 ms to 72 ms). It is extremely rare to see this, though. It almost always has a noticeable speed gain from caching. Also keep in mind that this is with the caching code executing first. If the runtime is caching the DisplaySubtitleHelper code, the non-caching version would already be at an advantage but it is still slower. This is still in progress and needs cleaning up.
Was incorrectly using the previous # of paragraphs and the start range, skipping processing the first record.
Instead of throwing away cache entries when adding new paragraphs, update them with the newly selected paragraph so they can be re-used. In testing, this improved selection speed by about 5-7 percent (but with a standard deviation of up to 19 percent so there is quite a bit of variation here, sometimes making performance worse).
This reduces the seek time when updating cache entries and can take the average prune time from around 45 ms to 29 or 36 ms on a debug build (depending on the number of partitions).
… portion of audio visualizer.
Finish documentation of methods, rename variables, refactor parameter order. Generally tries to be more consistent with itself and C# style guidlelines.
|
Hi, sorry for such a delay here! I had a lot of things to do and couldn't devote any time to this. Recently I've been able to work on this again. I started out with a simple rewrite of my first attempt but wasn't happy with the results. Since then, I've gone through a few revisions but this is an algorithm I'm really happy with. I've done a lot of profiling and optimizing to make sure it is fast even with a huge number of paragraphs, and I added some test cases to ensure it is choosing the 'best' paragraphs for display. |
After these changes, the timeline renders paragraphs even when there are many just outside the visible range.
#7000