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]

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.

[bdach]

I know that I proposed this originally but now I see that it's not correct in all instances. This change means that if you select an object that starts a combo, the new combo toggle will enable. Worse yet, if you select a combination of new-combo and non-new-combo objects, the toggle will enter indeterminate state and not even deselection will get rid of that state.

Not sure what the solution here is. Maybe the interaction of this popover with the new combo toggle should be removed completely. @peppy maybe you will have a UX opinion on this

[peppy]

@bdach correct me if i'm wrong, but it seems the only change here is that the check for selected items has been added. so the fail case would be that "new combo" may not be reset when the selection is empty?

[bdach]

Correct - in the worst case it will stay in indeterminate state after selecting and deselecting a group of objects with mixed new combo state.

[peppy]

I think this is okay. The next toggle fixes the indeterminate state anyways (and in this state it will seemingly default to placement without new combo, which makes sense to me).

That said, @bdach if you're not happy with this interaction then I'd propose making the generate dialog not set new combo at all, and leave that up to the user to fix as a post operation.

[bdach]

I've just made an alternative change to just revert the indeterminate state to full false on deselect (88aea70). Should hopefully be uncontroversial.

My only bother has ever been it visually looking like in the "indeterminate" state after a deselect, nothing more.

[peppy]

Code looks okay to me. Will leave to @bdach's discretion for a final decision.