Merge pull request ppy#4249 from bdach/text-builder-not-invalidated

Fix text builder not invalidating on indirect relative size changes
karaoke-dev · Mar 4, 2021 · 7f5f186 · 7f5f186
2 parents 7087cf0 + b5ec9aa
commit 7f5f186
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/osu.Framework.Tests/Visual/Sprites/TestSceneSpriteTextMaxWidth.cs b/osu.Framework.Tests/Visual/Sprites/TestSceneSpriteTextMaxWidth.cs
@@ -7,6 +7,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Text;
 using osuTK.Graphics;
 
 namespace osu.Framework.Tests.Visual.Sprites
@@ -70,16 +71,20 @@ public void TestFixedSizeMoreThanMaxWidth()
         [Test]
         public void TestMaxWidthWithRelativeSize()
         {
+            float textWidth = 0;
+
             createTest(s =>
             {
                 s.RelativeSizeAxes = Axes.X;
                 s.MaxWidth = 0.5f;
                 s.Text = "some very long text that should exceed the max width";
                 s.Truncate = true;
             }, Axes.Y);
+            AddStep("store text width", () => textWidth = display.Text.TextBuilder.Bounds.X);
 
             AddStep("set parent size", () => display.Width = 100);
             AddAssert("size <= max", () => display.Text.DrawWidth <= 50);
+            AddAssert("width increased", () => display.Text.TextBuilder.Bounds.X > textWidth);
         }
 
         private void createTest(Action<SpriteText> initFunc, Axes autoSizeAxes = Axes.Both)
@@ -93,7 +98,7 @@ private void createTest(Action<SpriteText> initFunc, Axes autoSizeAxes = Axes.Bo
 
         private class VisualDisplay : CompositeDrawable
         {
-            public readonly SpriteText Text;
+            public readonly TestSpriteText Text;
 
             public VisualDisplay(Action<SpriteText> initFunc, Axes autoSizeAxes = Axes.Both)
             {
@@ -109,11 +114,19 @@ public VisualDisplay(Action<SpriteText> initFunc, Axes autoSizeAxes = Axes.Both)
                         Alpha = 0.2f,
                         Colour = Color4.Pink
                     },
-                    Text = new SpriteText { AllowMultiline = false }
+                    Text = new TestSpriteText { AllowMultiline = false }
                 };
 
                 initFunc?.Invoke(Text);
             }
         }
+
+        private class TestSpriteText : SpriteText
+        {
+            public TextBuilder TextBuilder { get; private set; }
+
+            protected override TextBuilder CreateTextBuilder(ITexturedGlyphLookupStore store)
+                => TextBuilder = base.CreateTextBuilder(store);
+        }
     }
 }
diff --git a/osu.Framework/Graphics/Sprites/SpriteText.cs b/osu.Framework/Graphics/Sprites/SpriteText.cs
@@ -6,7 +6,6 @@
 using System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
-using osu.Framework.Caching;
 using osu.Framework.Development;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics.Containers;
@@ -55,6 +54,7 @@ public SpriteText()
             AddLayout(parentScreenSpaceCache);
             AddLayout(localScreenSpaceCache);
             AddLayout(shadowOffsetCache);
+            AddLayout(textBuilderCache);
         }
 
         [BackgroundDependencyLoader]
@@ -575,12 +575,12 @@ private void invalidate(bool characters = false, bool textBuilder = false)
         /// </summary>
         protected virtual char FallbackCharacter => '?';
 
-        private readonly Cached<TextBuilder> textBuilderBacking = new Cached<TextBuilder>();
+        private readonly LayoutValue<TextBuilder> textBuilderCache = new LayoutValue<TextBuilder>(Invalidation.DrawSize, InvalidationSource.Parent);
 
         /// <summary>
         /// Invalidates the current <see cref="TextBuilder"/>, causing a new one to be created next time it's required via <see cref="CreateTextBuilder"/>.
         /// </summary>
-        protected void InvalidateTextBuilder() => textBuilderBacking.Invalidate();
+        protected void InvalidateTextBuilder() => textBuilderCache.Invalidate();
 
         /// <summary>
         /// Creates a <see cref="TextBuilder"/> to generate the character layout for this <see cref="SpriteText"/>.
@@ -613,10 +613,10 @@ protected virtual TextBuilder CreateTextBuilder(ITexturedGlyphLookupStore store)
 
         private TextBuilder getTextBuilder()
         {
-            if (!textBuilderBacking.IsValid)
-                textBuilderBacking.Value = CreateTextBuilder(store);
+            if (!textBuilderCache.IsValid)
+                textBuilderCache.Value = CreateTextBuilder(store);
 
-            return textBuilderBacking.Value;
+            return textBuilderCache.Value;
         }
 
         public override string ToString() => $@"""{displayedText}"" " + base.ToString();