Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WaveformGraph overhead when DrawPosition is changed #6009

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Fix WaveformGraph overhead when DrawPosition is changed #6009

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de75632
Fix `WaveformGraph` overhead when `DrawPosition` is changed
peppy Sep 29, 2023
10ece0b
Fix typo in inline comment
bdach Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions osu.Framework/Graphics/Audio/WaveformGraph.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
using osu.Framework.Graphics.Rendering.Vertices;
using osu.Framework.Graphics.Shaders;
using osu.Framework.Graphics.Textures;
using osu.Framework.Layout;
using osu.Framework.Logging;
using osu.Framework.Utils;
using osuTK;
Expand Down Expand Up @@ -158,18 +157,15 @@ public Color4? HighColour
}
}

protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source)
private Vector2? lastGeneratedDrawSize;

protected override void Update()
{
bool result = base.OnInvalidate(invalidation, source);
base.Update();

if ((invalidation & Invalidation.RequiredParentSizeToFit) > 0)
{
// We should regenerate when `Scale` changed, but not `Position`.
// Unfortunately both of these are grouped together in `MiscGeometry`.
// Can't use invalidation for this as RequiredParentSizeToFit is closes, but also triggers on DrawPosition changes.
if (lastGeneratedDrawSize != null && DrawSize != lastGeneratedDrawSize)
queueRegeneration();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment previously mentioned scale, but that's not included in DrawSize. Perhaps it makes more sense this way, though? That is - when we think of scale it's usually in the context of stretching data and not generating new data for the stretched bounds?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RequiredParentSizeToFit = MiscGeometry | DrawSize. MiscGeometry invalidates on Scale, so the comment made sense. I don't necessarily disagree with ignoring it though as you say.

}

return result;
}

private CancellationTokenSource? cancelSource = new CancellationTokenSource();
Expand All @@ -189,6 +185,8 @@ private void queueRegeneration() => Scheduler.AddOnce(() =>

cancelGeneration();

lastGeneratedDrawSize = DrawSize;

var originalWaveform = Waveform;

if (originalWaveform == null)
Expand Down