Merge pull request #200 from andy840119/fix-ruby-romaji-text-position…

…-calculation Fix the ruby/romaji position x position calculation in the `PositionTextBuilder`.
karaoke-dev · May 19, 2022 · 1d0b7cc · 1d0b7cc
2 parents cb483d9 + 526569d
commit 1d0b7cc
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 42 deletions.
diff --git a/osu.Framework.Font.Tests/Text/PositionTextBuilderTest.cs b/osu.Framework.Font.Tests/Text/PositionTextBuilderTest.cs
@@ -99,8 +99,8 @@ public void TestAddPositionTextHasChar(char c, bool equal)
             }
         }
 
-        [TestCase('か', 5.5f, 6.5f)]
-        [TestCase('ら', 9.0f, 6.5f)]
+        [TestCase('か', 5.5f, 12.5f)]
+        [TestCase('ら', 9.0f, 12.5f)]
         public void TestAddPositionTextPosition(char c, float x, float y)
         {
             var builder = new PositionTextBuilder(fontStore, normal_font, characterList: characterList);

diff --git a/osu.Framework.Font.Tests/Visual/Sprites/TestSceneLyricSpriteTextCharacterPosition.cs b/osu.Framework.Font.Tests/Visual/Sprites/TestSceneLyricSpriteTextCharacterPosition.cs
@@ -39,7 +39,7 @@ public TestSceneLyricSpriteTextCharacterPosition()
                     {
                         Anchor = Anchor.Centre,
                         Origin = Anchor.Centre,
-                        Text = "カラオケ－",
+                        Text = "カラオケyo－",
                         Rubies = TestCaseTagHelper.ParseParsePositionTexts(new[] { "[0,1]:か", "[1,2]:ら", "[2,3]:お", "[3,4]:け", "[4,5]:－" }),
                         Romajies = TestCaseTagHelper.ParseParsePositionTexts(new[] { "[0,1]:ka", "[1,2]:ra", "[2,3]:o", "[3,4]:ke", "[4,5]:yo" }),
                     },
@@ -90,9 +90,9 @@ public TestSceneLyricSpriteTextCharacterPosition()
         }
 
         [TestCase(0, TextIndex.IndexState.Start, true)]
-        [TestCase(3, TextIndex.IndexState.End, true)]
+        [TestCase(6, TextIndex.IndexState.End, true)]
         [TestCase(-1, TextIndex.IndexState.End, false)]
-        [TestCase(5, TextIndex.IndexState.Start, false)]
+        [TestCase(7, TextIndex.IndexState.Start, false)]
         public void TestGetTextIndexPosition(int index, TextIndex.IndexState state, bool valid)
         {
             prepareTestCase(() =>
@@ -103,9 +103,9 @@ public void TestGetTextIndexPosition(int index, TextIndex.IndexState state, bool
         }
 
         [TestCase(0, true)]
-        [TestCase(3, true)]
+        [TestCase(6, true)]
         [TestCase(-1, false)]
-        [TestCase(5, false)]
+        [TestCase(7, false)]
         public void TestGetCharacterRectangle(int index, bool valid)
         {
             prepareTestCase(() => lyricSpriteText.GetCharacterRectangle(index), valid);

diff --git a/osu.Framework.Font/Graphics/Sprites/LyricSpriteText_Characters.cs b/osu.Framework.Font/Graphics/Sprites/LyricSpriteText_Characters.cs
@@ -8,6 +8,7 @@
 using osu.Framework.Graphics.Shaders;
 using osu.Framework.Layout;
 using osu.Framework.Text;
+using osu.Framework.Utils;
 using osuTK;
 
 namespace osu.Framework.Graphics.Sprites
@@ -169,7 +170,7 @@ public RectangleF GetCharacterRectangle(int index, bool drawSizeOnly = false)
                 throw new ArgumentOutOfRangeException(nameof(index));
 
             var character = characters[charIndex];
-            var drawRectangle = getCharacterRectangle(character, drawSizeOnly);
+            var drawRectangle = TextBuilderGlyphUtils.GetCharacterRectangle(character, drawSizeOnly);
             return getComputeCharacterDrawRectangle(drawRectangle);
         }
 
@@ -183,7 +184,7 @@ public RectangleF GetRubyTagPosition(PositionText rubyTag, bool drawSizeOnly = f
             int count = rubyTag.Text.Length;
             var drawRectangle = characters.ToList()
                                           .GetRange(startCharacterIndex, count)
-                                          .Select(x => getCharacterRectangle(x, drawSizeOnly))
+                                          .Select(x => TextBuilderGlyphUtils.GetCharacterRectangle(x, drawSizeOnly))
                                           .Aggregate(RectangleF.Union);
             return getComputeCharacterDrawRectangle(drawRectangle);
         }
@@ -198,26 +199,11 @@ public RectangleF GetRomajiTagPosition(PositionText romajiTag, bool drawSizeOnly
             int count = romajiTag.Text.Length;
             var drawRectangle = characters.ToList()
                                           .GetRange(startCharacterIndex, count)
-                                          .Select(x => getCharacterRectangle(x, drawSizeOnly))
+                                          .Select(x => TextBuilderGlyphUtils.GetCharacterRectangle(x, drawSizeOnly))
                                           .Aggregate(RectangleF.Union);
             return getComputeCharacterDrawRectangle(drawRectangle);
         }
 
-        private static RectangleF getCharacterRectangle(TextBuilderGlyph character, bool drawSizeOnly)
-        {
-            if (drawSizeOnly)
-                return character.DrawRectangle;
-
-            // todo: should get the real value.
-            var topReduce = character.Baseline * 0.3f;
-            var bottomIncrease = character.Baseline * 0.2f;
-            return character.DrawRectangle.Inflate(new MarginPadding
-            {
-                Top = character.YOffset - topReduce,
-                Bottom = character.Baseline - character.Height - character.YOffset + bottomIncrease,
-            });
-        }
-
         private int skinIndex(IEnumerable<PositionText> positionTexts, int endIndex)
             => positionTexts.Where((_, i) => i < endIndex).Sum(x => x.Text.Length);
 

diff --git a/osu.Framework.Font/Text/PositionTextBuilder.cs b/osu.Framework.Font/Text/PositionTextBuilder.cs
@@ -5,7 +5,9 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
+using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Utils;
 using osuTK;
 
 namespace osu.Framework.Text
@@ -64,14 +66,12 @@ public void AddText(PositionText positionText)
                 return;
 
             // get some position relative params.
-            var canterPosition = getCenterPosition(positionText.StartIndex, positionText.EndIndex);
-            var mainFontHeight = getMainTextHeight();
+            var mainCharacterRect = getMainCharacterRectangleF(positionText.StartIndex, positionText.EndIndex);
             var subTextWidth = getSubTextWidth(text);
             var subTextHeight = getSubTextHeight(text);
 
-            // calculate the start text position.
-            var yOffset = relativePosition == RelativePosition.Top ? -mainFontHeight / 2 - subTextHeight : mainFontHeight / 2;
-            var position = new Vector2(canterPosition.X - subTextWidth / 2, canterPosition.Y + yOffset);
+            // calculate the start draw position.
+            var position = getPositionTextDrawPosition(mainCharacterRect, new Vector2(subTextWidth, subTextHeight), relativePosition);
 
             // set start render position
             setCurrentPosition(position + startOffset);
@@ -94,12 +94,27 @@ private void setCurrentPosition(Vector2 position)
             prop.SetValue(this, position);
         }
 
-        private Vector2 getCenterPosition(int startCharIndex, int endCharIndex)
+        private static Vector2 getPositionTextDrawPosition(RectangleF mainTextPosition, Vector2 positionTextSize, RelativePosition relativePosition)
+        {
+            var subTextWidth = positionTextSize.X;
+            var subTextHeight = positionTextSize.Y;
+
+            return relativePosition switch
+            {
+                RelativePosition.Top => new Vector2(mainTextPosition.Centre.X, mainTextPosition.Top) + new Vector2(-subTextWidth / 2, -subTextHeight),
+                RelativePosition.Bottom => new Vector2(mainTextPosition.Centre.X, mainTextPosition.Bottom) + new Vector2(-subTextWidth / 2, 0),
+                _ => throw new ArgumentOutOfRangeException(nameof(relativePosition), relativePosition, null)
+            };
+        }
+
+        private RectangleF getMainCharacterRectangleF(int startCharIndex, int endCharIndex)
         {
             var starCharacter = Characters[startCharIndex];
             var endCharacter = Characters[endCharIndex - 1];
-            var startCharacterRectangle = starCharacter.DrawRectangle;
-            var endCharacterRectangle = endCharacter.DrawRectangle;
+            var startCharacterRectangle = TextBuilderGlyphUtils.GetCharacterRectangle(starCharacter, false);
+            var endCharacterRectangle = TextBuilderGlyphUtils.GetCharacterRectangle(endCharacter, false);
+
+            var position = startCharacterRectangle.TopLeft;
 
             // if center position is between two lines, then should let canter position in the first line.
             var leftX = startCharacterRectangle.Left;
@@ -108,17 +123,11 @@ private Vector2 getCenterPosition(int startCharIndex, int endCharIndex)
                 : Characters.Max(c => c.DrawRectangle.Right);
 
             // because each character has different height, so we need to get base text height from here.
-            var x = (leftX + rightX) / 2;
-            var y = startCharacterRectangle.Centre.Y;
+            var width = rightX - leftX;
+            var height = Math.Max(startCharacterRectangle.Height, endCharacterRectangle.Height);
 
             // return center position.
-            return new Vector2(x, y);
-        }
-
-        private float getMainTextHeight()
-        {
-            // note: base line in here already include font size. see TextBuilderGlyph.
-            return Characters.Max(x => x.Baseline);
+            return new RectangleF(position, new Vector2(width, height));
         }
 
         private float getSubTextWidth(string text)

diff --git a/osu.Framework.Font/Utils/TextBuilderGlyphUtils.cs b/osu.Framework.Font/Utils/TextBuilderGlyphUtils.cs
@@ -0,0 +1,27 @@
+// Copyright (c) karaoke.dev <contact@karaoke.dev>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Primitives;
+using osu.Framework.Text;
+
+namespace osu.Framework.Utils
+{
+    public class TextBuilderGlyphUtils
+    {
+        public static RectangleF GetCharacterRectangle(TextBuilderGlyph character, bool drawSizeOnly)
+        {
+            if (drawSizeOnly)
+                return character.DrawRectangle;
+
+            // todo: should get the real value.
+            var topReduce = character.Baseline * 0.3f;
+            var bottomIncrease = character.Baseline * 0.2f;
+            return character.DrawRectangle.Inflate(new MarginPadding
+            {
+                Top = character.YOffset - topReduce,
+                Bottom = character.Baseline - character.Height - character.YOffset + bottomIncrease,
+            });
+        }
+    }
+}