Skip to content

Commit

Permalink
Reduce complexity overhead (and avoid one copy) when obtaining charac…
Browse files Browse the repository at this point in the history 
…ter SSDQs in `SpriteTextDrawNode`

Of note, this doesn't show a huge improvement in benchmarks, but in an
edge case noticed in osu!, the `AddRange` operation can add a
perceivable overhead.

We believe this may be a dotnet runtime quirk.

Even though this change can't be shown in isolated benchmarks, I'd argue
the code quality improvement is worth it.

Of note, the caching of the SSDQs at `SpriteText` was a bit redundant as
it was being invalidated by basically everything. So it makes sense to
just fetch it every time, regardless, in the draw node itself.

The only potential saving we could obtain with the previous logic would
be to shift the load to the `Update` frame. But this wasn't being done.
Rather than investigating whether that has any benefits, I'd rather
focus on getting `SpriteText` rewritten to use an optimised shader.
  • Loading branch information
@peppy
peppy committed Jul 3, 2023
1 parent 00c570f commit 2967626
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 59 deletions.
54 changes: 0 additions & 54 deletions osu.Framework/Graphics/Sprites/SpriteText.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ namespace osu.Framework.Graphics.Sprites
/// </summary>
public partial class SpriteText : Drawable, IHasLineBaseHeight, ITexturedShaderDrawable, IHasText, IHasFilterTerms, IFillFlowContainer, IHasCurrentValue<string>
{
private const float default_text_size = 20;

/// <remarks>
/// <c>U+00A0</c> is the Unicode NON-BREAKING SPACE character (distinct from the standard ASCII space).
/// <c>U+202F</c> is the Unicode NARROW NO-BREAK SPACE character.
Expand Down Expand Up @@ -56,8 +54,6 @@ public SpriteText()
});

AddLayout(charactersCache);
AddLayout(parentScreenSpaceCache);
AddLayout(localScreenSpaceCache);
AddLayout(shadowOffsetCache);
AddLayout(textBuilderCache);
}
Expand Down Expand Up @@ -501,53 +497,6 @@ private void computeCharacters()
}
}

private readonly LayoutValue parentScreenSpaceCache = new LayoutValue(Invalidation.DrawSize | Invalidation.Presence | Invalidation.DrawInfo, InvalidationSource.Parent);
private readonly LayoutValue localScreenSpaceCache = new LayoutValue(Invalidation.MiscGeometry, InvalidationSource.Self);

private readonly List<ScreenSpaceCharacterPart> screenSpaceCharactersBacking = new List<ScreenSpaceCharacterPart>();

/// <summary>
/// The characters in screen space. These are ready to be drawn.
/// </summary>
private List<ScreenSpaceCharacterPart> screenSpaceCharacters
{
get
{
computeScreenSpaceCharacters();
return screenSpaceCharactersBacking;
}
}

private void computeScreenSpaceCharacters()
{
if (!parentScreenSpaceCache.IsValid)
{
localScreenSpaceCache.Invalidate();
parentScreenSpaceCache.Validate();
}

if (localScreenSpaceCache.IsValid)
return;

screenSpaceCharactersBacking.Clear();

Vector2 inflationAmount = DrawInfo.MatrixInverse.ExtractScale().Xy;

foreach (var character in characters)
{
screenSpaceCharactersBacking.Add(new ScreenSpaceCharacterPart
{
DrawQuad = ToScreenSpace(character.DrawRectangle.Inflate(inflationAmount)),
InflationPercentage = new Vector2(
character.DrawRectangle.Size.X == 0 ? 0 : inflationAmount.X / character.DrawRectangle.Size.X,
character.DrawRectangle.Size.Y == 0 ? 0 : inflationAmount.Y / character.DrawRectangle.Size.Y),
Texture = character.Texture
});
}

localScreenSpaceCache.Validate();
}

private readonly LayoutValue<Vector2> shadowOffsetCache = new LayoutValue<Vector2>(Invalidation.DrawInfo, InvalidationSource.Parent);

private Vector2 premultipliedShadowOffset =>
Expand All @@ -565,9 +514,6 @@ private void invalidate(bool characters = false, bool textBuilder = false)
if (textBuilder)
InvalidateTextBuilder();

parentScreenSpaceCache.Invalidate();
localScreenSpaceCache.Invalidate();

Invalidate(Invalidation.RequiredParentSizeToFit);
}

Expand Down
52 changes: 47 additions & 5 deletions osu.Framework/Graphics/Sprites/SpriteText_DrawNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Collections.Generic;
using System.Buffers;
using System.Diagnostics;
using osu.Framework.Graphics.Colour;
using osu.Framework.Graphics.Primitives;
using osu.Framework.Graphics.Rendering;
Expand All @@ -22,7 +23,9 @@ internal class SpriteTextDrawNode : TexturedShaderDrawNode
private ColourInfo shadowColour;
private Vector2 shadowOffset;

private readonly List<ScreenSpaceCharacterPart> parts = new List<ScreenSpaceCharacterPart>();
private ScreenSpaceCharacterPart[]? parts;

private int partCount;

public SpriteTextDrawNode(SpriteText source)
: base(source)
Expand All @@ -33,8 +36,7 @@ public override void ApplyState()
{
base.ApplyState();

parts.Clear();
parts.AddRange(Source.screenSpaceCharacters);
updateScreenSpaceCharacters();
shadow = Source.Shadow;

if (shadow)
Expand All @@ -46,6 +48,8 @@ public override void ApplyState()

public override void Draw(IRenderer renderer)
{
Debug.Assert(parts != null);

base.Draw(renderer);

BindTextureShader(renderer);
Expand All @@ -58,7 +62,7 @@ public override void Draw(IRenderer renderer)
var finalShadowColour = DrawColourInfo.Colour;
finalShadowColour.ApplyChild(shadowColour.MultiplyAlpha(shadowAlpha));

for (int i = 0; i < parts.Count; i++)
for (int i = 0; i < partCount; i++)
{
if (shadow)
{
Expand All @@ -78,6 +82,44 @@ public override void Draw(IRenderer renderer)

UnbindTextureShader(renderer);
}

/// <summary>
/// The characters in screen space. These are ready to be drawn.
/// </summary>
private void updateScreenSpaceCharacters()
{
partCount = Source.characters.Count;

if (parts == null || parts.Length < partCount)
{
if (parts != null)
ArrayPool<ScreenSpaceCharacterPart>.Shared.Return(parts);
parts = ArrayPool<ScreenSpaceCharacterPart>.Shared.Rent(partCount);
}

Vector2 inflationAmount = DrawInfo.MatrixInverse.ExtractScale().Xy;

for (int i = 0; i < partCount; i++)
{
var character = Source.characters[i];
parts[i] = new ScreenSpaceCharacterPart
{
DrawQuad = Source.ToScreenSpace(character.DrawRectangle.Inflate(inflationAmount)),
InflationPercentage = new Vector2(
character.DrawRectangle.Size.X == 0 ? 0 : inflationAmount.X / character.DrawRectangle.Size.X,
character.DrawRectangle.Size.Y == 0 ? 0 : inflationAmount.Y / character.DrawRectangle.Size.Y),
Texture = character.Texture
};
}
}

protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);

if (parts != null)
ArrayPool<ScreenSpaceCharacterPart>.Shared.Return(parts);
}
}

/// <summary>
Expand Down

0 comments on commit 2967626

Please sign in to comment.