From a922ea9b0163a805c5b2108ddecd22cb6d412c4a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 20 Jun 2022 15:28:36 +0900 Subject: [PATCH] Fix selection by directly comparing control points Previously, all control points would get replaced, which led to performance issues that was worked around in this PR. By comparing control points, we're able to get good performance without requiring the workaround. --- .../Edit/LegacyEditorBeatmapPatcher.cs | 28 +++++++++++++++---- osu.Game/Screens/Edit/Timing/TimingScreen.cs | 8 ++---- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index 6b18855396..01bc01bf5b 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -6,12 +6,14 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using DiffPlex; using DiffPlex.Model; using osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Formats; using osu.Game.IO; using osu.Game.Skinning; @@ -50,14 +52,28 @@ namespace osu.Game.Screens.Edit if (removedIndices.Count == 0 && addedIndices.Count == 0) return; - // Due to conversion from legacy to non-legacy control points, it becomes difficult to diff control points correctly. - // So instead _all_ control points are reloaded if _any_ control point is changed. + ControlPointInfo newControlPoints = EditorBeatmap.ConvertControlPoints(getNewBeatmap().ControlPointInfo); - var newControlPoints = EditorBeatmap.ConvertControlPoints(getNewBeatmap().ControlPointInfo); + // Remove all groups from the current beatmap which don't have a corresponding equal group in the new beatmap. + foreach (var oldGroup in editorBeatmap.ControlPointInfo.Groups.ToArray()) + { + var newGroup = newControlPoints.GroupAt(oldGroup.Time); - editorBeatmap.ControlPointInfo.Clear(); - foreach (var point in newControlPoints.AllControlPoints) - editorBeatmap.ControlPointInfo.Add(point.Time, point); + if (!oldGroup.Equals(newGroup)) + editorBeatmap.ControlPointInfo.RemoveGroup(oldGroup); + } + + // Add all groups from the new beatmap which don't have a corresponding equal group in the old beatmap. + foreach (var newGroup in newControlPoints.Groups) + { + var oldGroup = editorBeatmap.ControlPointInfo.GroupAt(newGroup.Time); + + if (!newGroup.Equals(oldGroup)) + { + foreach (var point in newGroup.ControlPoints) + editorBeatmap.ControlPointInfo.Add(newGroup.Time, point); + } + } } private void processHitObjects(DiffResult result, Func getNewBeatmap) diff --git a/osu.Game/Screens/Edit/Timing/TimingScreen.cs b/osu.Game/Screens/Edit/Timing/TimingScreen.cs index 27b62f1104..60cb263a79 100644 --- a/osu.Game/Screens/Edit/Timing/TimingScreen.cs +++ b/osu.Game/Screens/Edit/Timing/TimingScreen.cs @@ -139,12 +139,8 @@ namespace osu.Game.Screens.Edit.Timing controlPointGroups.BindTo(Beatmap.ControlPointInfo.Groups); controlPointGroups.BindCollectionChanged((sender, args) => { - // This AddOnce() works around performance issues from the LegacyEditorBeatmapPatcher re-initialising all control points every undo & redo. - Scheduler.AddOnce(() => - { - table.ControlGroups = controlPointGroups; - changeHandler?.SaveState(); - }); + table.ControlGroups = controlPointGroups; + changeHandler?.SaveState(); }, true); }