Fix slow performance of polygon generation tool

[minetoblend]

Reusing the hitobjects now instead of removing them and regenerating everything from scratch each time.
Also fixes the flashing whenever the polygon updates.

I put tryCreatePolygon() behind Scheduler.AddOnce for good measure, not sure if this is actually required as it's plenty fast either way now.

Before:

2024-10-11.11-29-04.mp4

After:

2024-10-11.11-44-56.mp4

[bdach]

How much of the "performance gain" here is the "reuse of objects"? I'd wager the .AddOnce() is doing the heavy lifting here if anything.

[minetoblend]

The majority of it comes from reusing the objects.

For comparison: (All the videos are recorded on release builds btw)

Index: osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs b/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs
--- a/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs	(revision 7e439be9eca9e626a080defc0cd24da933072710)
+++ b/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs	(date 1728640773096)
@@ -125,7 +125,7 @@
             tryCreatePolygon();
         }
 
-        private void scheduleRefresh() => Scheduler.AddOnce(tryCreatePolygon);
+        private void scheduleRefresh() => tryCreatePolygon();
 
         private void tryCreatePolygon()
         {

2024-10-11.12-00-23.mp4

[minetoblend]

Ran it one more time with just Scheduler.AddOnce to double check, and it looks like it actually does have a noticeable effect, albeit much less than with just reusing the objects.

2024-10-11.12-07-36.mp4

[bdach]

Nit but I wouldn't bother. Just inline this in all usages.

[bdach]

This is off by one.

To illustrate with an example: Say you have 10 points and want to leave 5. Thus, you want to keep 5, starting from index 0 through 4, and delete 5, starting from index 5 through 9. So the + 1 everywhere is gratuitous / wrong.

Suggested change

	editorBeatmap.RemoveRange(insertedCircles.GetRange(totalPoints + 1, insertedCircles.Count - totalPoints - 1));
	insertedCircles.RemoveRange(totalPoints + 1, insertedCircles.Count - totalPoints - 1);
	editorBeatmap.RemoveRange(insertedCircles.GetRange(totalPoints, insertedCircles.Count - totalPoints));
	insertedCircles.RemoveRange(totalPoints, insertedCircles.Count - totalPoints);

You can actually see this fail from the default settings if you increase the number of vertices by 1 and then decrease it again - 4 objects will get created, not 3.

(This is partially why I was asking as to how much performance this was actually gaining, because this sort of logic is very easy to get wrong and it's just simpler to not have to think about it if it's not necessary.)

[minetoblend]

Oh, yeah that's a silly mistake on my part.

[bdach]

You can't be changing stuff on hitobjects that already exist in EditorBeatmap without calling EditorBeatmap.Update() at the end. While yes, stuff like changing start time will automatically call it for you, stuff like changing new combo flags does not. And in fact this behaviour with the combo toggle state is completely broken with this change if the first object is reused.

(Note though that it is half-broken already on master if you open the polygon tool without an object selected, because EditorSelectionHandler will reset new combo state to off on deselect. You can do

diff --git a/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs b/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs
index dbbf767a7d..dfda93cedd 100644
--- a/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/EditorSelectionHandler.cs
@@ -176,7 +176,8 @@ private void createStateBindables()
         /// </summary>
         protected virtual void UpdateTernaryStates()
         {
-            SelectionNewComboState.Value = GetStateFromSelection(SelectedItems.OfType<IHasComboInformation>(), h => h.NewCombo);
+            if (SelectedItems.Any())
+                SelectionNewComboState.Value = GetStateFromSelection(SelectedItems.OfType<IHasComboInformation>(), h => h.NewCombo);
 
             var samplesInSelection = SelectedItems.SelectMany(enumerateAllSamples).ToArray();
 

to change that, and we should probably do that at some point.)

[minetoblend]

Oh, I wasn't aware of needing to call EditorBeatmap.Update, I'll keep that in mind for the future.
I restructured the loop body a bit to clearly split the two cases of adding/updating an object.
I also made sure the polygon gets regenerated when the newComboState changes.

The diff above works for preserving the newCombo state in most cases, however when a new object gets added to the beatmap it still resets to false (because of ComposeBlueprintContainer.hitObjectAdded). I added an extra bit at the end to restore the previous newcombo state in that case.