From b602f0cfd5391153a0635857046e30b393a237ce Mon Sep 17 00:00:00 2001 From: 7aGiven Date: Sat, 18 Jul 2026 14:16:56 +0800 Subject: [PATCH 1/4] fix Duplicate DrawString --- src/Graphics/SpriteBatch.cs | 178 ++++++------------------------------ src/Graphics/SpriteFont.cs | 150 ++++++++++++------------------ 2 files changed, 88 insertions(+), 240 deletions(-) diff --git a/src/Graphics/SpriteBatch.cs b/src/Graphics/SpriteBatch.cs index 5b3b251ed..427a6e66c 100644 --- a/src/Graphics/SpriteBatch.cs +++ b/src/Graphics/SpriteBatch.cs @@ -708,149 +708,12 @@ public void DrawString( SpriteEffects effects, float layerDepth ) { - /* FIXME: This method is a duplicate of DrawString(string)! - * The only difference is how we iterate through the StringBuilder. - * We don't use ToString() since it generates garbage. - * -flibit - */ - CheckBegin("DrawString"); if (text == null) { throw new ArgumentNullException("text"); } - if (text.Length == 0) - { - return; - } - effects &= (SpriteEffects) 0x03; - - /* We pull all these internal variables in at once so - * anyone who wants to use this file to make their own - * SpriteBatch can easily replace these with reflection. - * -flibit - */ - Texture2D textureValue = spriteFont.textureValue; - List glyphData = spriteFont.glyphData; - List croppingData = spriteFont.croppingData; - List kerning = spriteFont.kerning; - Dictionary characterIndexMap = spriteFont.characterIndexMap; - - // FIXME: This needs an accuracy check! -flibit - - // Calculate offsets/axes, using the string size for flipped text - Vector2 baseOffset = origin; - float axisDirX = axisDirectionX[(int) effects]; - float axisDirY = axisDirectionY[(int) effects]; - float axisDirMirrorX = 0.0f; - float axisDirMirrorY = 0.0f; - if (effects != SpriteEffects.None) - { - Vector2 size = spriteFont.MeasureString(text); - baseOffset.X -= size.X * axisIsMirroredX[(int) effects]; - baseOffset.Y -= size.Y * axisIsMirroredY[(int) effects]; - axisDirMirrorX = axisIsMirroredX[(int) effects]; - axisDirMirrorY = axisIsMirroredY[(int) effects]; - } - - Vector2 curOffset = Vector2.Zero; - bool firstInLine = true; - for (int i = 0; i < text.Length; i += 1) - { - char c = text[i]; - - // Special characters - if (c == '\r') - { - continue; - } - if (c == '\n') - { - curOffset.X = 0.0f; - curOffset.Y += spriteFont.LineSpacing; - firstInLine = true; - continue; - } - - /* Get the List index from the character map, defaulting to the - * DefaultCharacter if it's set. - */ - int index; - if (!characterIndexMap.TryGetValue(c, out index)) - { - if (!spriteFont.DefaultCharacter.HasValue) - { - throw new ArgumentException( - "Text contains characters that cannot be" + - " resolved by this SpriteFont.", - "text" - ); - } - index = characterIndexMap[spriteFont.DefaultCharacter.Value]; - } - - /* For the first character in a line, always push the width - * rightward, even if the kerning pushes the character to the - * left. - */ - Vector3 cKern = kerning[index]; - if (firstInLine) - { - curOffset.X += Math.Abs(cKern.X); - firstInLine = false; - } - else - { - curOffset.X += spriteFont.Spacing + cKern.X; - } - - // Calculate the character origin - Rectangle cCrop = croppingData[index]; - Rectangle cGlyph = glyphData[index]; - float offsetX = baseOffset.X + ( - curOffset.X + cCrop.X - ) * axisDirX; - float offsetY = baseOffset.Y + ( - curOffset.Y + cCrop.Y - ) * axisDirY; - if (effects != SpriteEffects.None) - { - offsetX += cGlyph.Width * axisDirMirrorX; - offsetY += cGlyph.Height * axisDirMirrorY; - } - - // Draw! - float sourceW = Math.Sign(cGlyph.Width) * Math.Max( - Math.Abs(cGlyph.Width), - MathHelper.MachineEpsilonFloat - ) / (float) textureValue.Width; - float sourceH = Math.Sign(cGlyph.Height) * Math.Max( - Math.Abs(cGlyph.Height), - MathHelper.MachineEpsilonFloat - ) / (float) textureValue.Height; - PushSprite( - textureValue, - cGlyph.X / (float) textureValue.Width, - cGlyph.Y / (float) textureValue.Height, - sourceW, - sourceH, - position.X, - position.Y, - cGlyph.Width * scale.X, - cGlyph.Height * scale.Y, - color, - offsetX / sourceW / (float) textureValue.Width, - offsetY / sourceH / (float) textureValue.Height, - (float) Math.Sin(rotation), - (float) Math.Cos(rotation), - layerDepth, - (int) effects - ); - - /* Add the character width and right-side - * bearing to the line width. - */ - curOffset.X += cKern.Y + cKern.Z; - } + SpriteFont.IReadOnlyCharList view = new SpriteFont.StringBuilderView(text); + DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); } public void DrawString( @@ -907,15 +770,31 @@ public void DrawString( SpriteEffects effects, float layerDepth ) { - /* FIXME: This method is a duplicate of DrawString(StringBuilder)! - * The only difference is how we iterate through the string. - * -flibit - */ - CheckBegin("DrawString"); if (text == null) { throw new ArgumentNullException("text"); } + SpriteFont.IReadOnlyCharList view = new SpriteFont.StringView(text); + DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); + } + + #endregion + + #region Private Methods + + private void DrawString( + SpriteFont spriteFont, + ref SpriteFont.IReadOnlyCharList text, + Vector2 position, + Color color, + float rotation, + Vector2 origin, + Vector2 scale, + SpriteEffects effects, + float layerDepth + ) + { + CheckBegin("DrawString"); if (text.Length == 0) { return; @@ -943,7 +822,7 @@ float layerDepth float axisDirMirrorY = 0.0f; if (effects != SpriteEffects.None) { - Vector2 size = spriteFont.MeasureString(text); + Vector2 size = spriteFont.MeasureString(ref text); baseOffset.X -= size.X * axisIsMirroredX[(int) effects]; baseOffset.Y -= size.Y * axisIsMirroredY[(int) effects]; axisDirMirrorX = axisIsMirroredX[(int) effects]; @@ -952,8 +831,10 @@ float layerDepth Vector2 curOffset = Vector2.Zero; bool firstInLine = true; - foreach (char c in text) + for (int i = 0; i < text.Length; i += 1) { + char c = text[i]; + // Special characters if (c == '\r') { @@ -1048,11 +929,6 @@ float layerDepth curOffset.X += cKern.Y + cKern.Z; } } - - #endregion - - #region Private Methods - private unsafe void PushSprite( Texture2D texture, float sourceX, diff --git a/src/Graphics/SpriteFont.cs b/src/Graphics/SpriteFont.cs index d34ef38a5..a8a299b5a 100644 --- a/src/Graphics/SpriteFont.cs +++ b/src/Graphics/SpriteFont.cs @@ -122,18 +122,9 @@ internal SpriteFont( #endregion - #region Public MeasureString Methods - - public Vector2 MeasureString(string text) - { - /* FIXME: This method is a duplicate of MeasureString(StringBuilder)! - * The only difference is how we iterate through the string. - * -flibit - */ - if (text == null) - { - throw new ArgumentNullException("text"); - } + #region Internal MeasureString Method + internal Vector2 MeasureString(ref IReadOnlyCharList text) { + // FIXME: change argument to `(in IReadOnlyCharList text)`. that only allow above C# 7.2 if (text.Length == 0) { return Vector2.Zero; @@ -146,8 +137,10 @@ public Vector2 MeasureString(string text) float finalLineHeight = LineSpacing; bool firstInLine = true; - foreach (char c in text) + for (int i = 0; i < text.Length; i += 1) { + char c = text[i]; + // Special characters if (c == '\r') { @@ -216,103 +209,82 @@ public Vector2 MeasureString(string text) return result; } + #endregion - public Vector2 MeasureString(StringBuilder text) + #region Public MeasureString Methods + + public Vector2 MeasureString(string text) { - /* FIXME: This method is a duplicate of MeasureString(string)! - * The only difference is how we iterate through the StringBuilder. - * We don't use ToString() since it generates garbage. - * -flibit - */ if (text == null) { throw new ArgumentNullException("text"); } - if (text.Length == 0) + IReadOnlyCharList view = new StringView(text); + return MeasureString(ref view); + } + + public Vector2 MeasureString(StringBuilder text) + { + if (text == null) { - return Vector2.Zero; + throw new ArgumentNullException("text"); } + IReadOnlyCharList view = new StringBuilderView(text); + return MeasureString(ref view); + } - // FIXME: This needs an accuracy check! -flibit - - Vector2 result = Vector2.Zero; - float curLineWidth = 0.0f; - float finalLineHeight = LineSpacing; - bool firstInLine = true; + #endregion - for (int i = 0; i < text.Length; i += 1) + #region Internal IReadOnlyCharList Structs + internal interface IReadOnlyCharList + { + int Length { get; } + char this[int index] { get; } + } + internal struct StringView : IReadOnlyCharList + { + private readonly string view; + internal StringView(string view) { - char c = text[i]; - - // Special characters - if (c == '\r') - { - continue; - } - if (c == '\n') - { - result.X = Math.Max(result.X, curLineWidth); - result.Y += LineSpacing; - curLineWidth = 0.0f; - finalLineHeight = LineSpacing; - firstInLine = true; - continue; - } - - /* Get the List index from the character map, defaulting to the - * DefaultCharacter if it's set. - */ - int index; - if (!characterIndexMap.TryGetValue(c, out index)) + this.view = view; + } + public int Length + { + get { - if (!DefaultCharacter.HasValue) - { - throw new ArgumentException( - "Text contains characters that cannot be" + - " resolved by this SpriteFont.", - "text" - ); - } - index = characterIndexMap[DefaultCharacter.Value]; + return view.Length; } - - /* For the first character in a line, always push the width - * rightward, even if the kerning pushes the character to the - * left. - */ - Vector3 cKern = kerning[index]; - if (firstInLine) + } + public char this[int index] + { + get { - curLineWidth += Math.Abs(cKern.X); - firstInLine = false; + return view[index]; } - else + } + } + internal struct StringBuilderView : IReadOnlyCharList + { + private readonly StringBuilder view; + internal StringBuilderView(StringBuilder view) + { + this.view = view; + } + public int Length + { + get { - curLineWidth += Spacing + cKern.X; + return view.Length; } - - /* Add the character width and right-side bearing to the line - * width. - */ - curLineWidth += cKern.Y + cKern.Z; - - /* If a character is taller than the default line height, - * increase the height to that of the line's tallest character. - */ - int cCropHeight = croppingData[index].Height; - if (cCropHeight > finalLineHeight) + } + public char this[int index] + { + get { - finalLineHeight = cCropHeight; + return view[index]; } } - - // Calculate the final width/height of the text box - result.X = Math.Max(result.X, curLineWidth); - result.Y += finalLineHeight; - - return result; } - #endregion } } From 0606cd7ee1259f2d8f8602bc82b2c988f5a457fd Mon Sep 17 00:00:00 2001 From: 7aGiven <77519196+7aGiven@users.noreply.github.com> Date: Sun, 19 Jul 2026 07:46:49 +0800 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Ethan Lee --- src/Graphics/SpriteFont.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Graphics/SpriteFont.cs b/src/Graphics/SpriteFont.cs index a8a299b5a..8a6c67813 100644 --- a/src/Graphics/SpriteFont.cs +++ b/src/Graphics/SpriteFont.cs @@ -123,7 +123,9 @@ internal SpriteFont( #endregion #region Internal MeasureString Method - internal Vector2 MeasureString(ref IReadOnlyCharList text) { + + internal Vector2 MeasureString(ref IReadOnlyCharList text) + { // FIXME: change argument to `(in IReadOnlyCharList text)`. that only allow above C# 7.2 if (text.Length == 0) { @@ -209,6 +211,7 @@ internal Vector2 MeasureString(ref IReadOnlyCharList text) { return result; } + #endregion #region Public MeasureString Methods From 218fe0a70143406b2d8caf1abaa05cacf64b7a54 Mon Sep 17 00:00:00 2001 From: 7aGiven Date: Sun, 19 Jul 2026 08:31:58 +0800 Subject: [PATCH 3/4] change interface to generic to avoid boxing --- src/Graphics/SpriteBatch.cs | 10 +++++----- src/Graphics/SpriteFont.cs | 7 +++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Graphics/SpriteBatch.cs b/src/Graphics/SpriteBatch.cs index 427a6e66c..b7c6050fc 100644 --- a/src/Graphics/SpriteBatch.cs +++ b/src/Graphics/SpriteBatch.cs @@ -712,7 +712,7 @@ float layerDepth { throw new ArgumentNullException("text"); } - SpriteFont.IReadOnlyCharList view = new SpriteFont.StringBuilderView(text); + SpriteFont.StringBuilderView view = new SpriteFont.StringBuilderView(text); DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); } @@ -774,7 +774,7 @@ float layerDepth { throw new ArgumentNullException("text"); } - SpriteFont.IReadOnlyCharList view = new SpriteFont.StringView(text); + SpriteFont.StringView view = new SpriteFont.StringView(text); DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); } @@ -782,9 +782,9 @@ float layerDepth #region Private Methods - private void DrawString( + private void DrawString( SpriteFont spriteFont, - ref SpriteFont.IReadOnlyCharList text, + ref T text, Vector2 position, Color color, float rotation, @@ -792,7 +792,7 @@ private void DrawString( Vector2 scale, SpriteEffects effects, float layerDepth - ) + ) where T : SpriteFont.IReadOnlyCharList { CheckBegin("DrawString"); if (text.Length == 0) diff --git a/src/Graphics/SpriteFont.cs b/src/Graphics/SpriteFont.cs index 8a6c67813..078e2acbe 100644 --- a/src/Graphics/SpriteFont.cs +++ b/src/Graphics/SpriteFont.cs @@ -124,9 +124,8 @@ internal SpriteFont( #region Internal MeasureString Method - internal Vector2 MeasureString(ref IReadOnlyCharList text) + internal Vector2 MeasureString(ref T text) where T : IReadOnlyCharList { - // FIXME: change argument to `(in IReadOnlyCharList text)`. that only allow above C# 7.2 if (text.Length == 0) { return Vector2.Zero; @@ -222,7 +221,7 @@ public Vector2 MeasureString(string text) { throw new ArgumentNullException("text"); } - IReadOnlyCharList view = new StringView(text); + StringView view = new StringView(text); return MeasureString(ref view); } @@ -232,7 +231,7 @@ public Vector2 MeasureString(StringBuilder text) { throw new ArgumentNullException("text"); } - IReadOnlyCharList view = new StringBuilderView(text); + StringBuilderView view = new StringBuilderView(text); return MeasureString(ref view); } From fb9eaa1b3ff82435ab375205c86de3739eade55b Mon Sep 17 00:00:00 2001 From: 7aGiven Date: Mon, 20 Jul 2026 23:48:09 +0800 Subject: [PATCH 4/4] Don't use ref for small struct --- src/Graphics/SpriteBatch.cs | 12 +++++------- src/Graphics/SpriteFont.cs | 8 +++----- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Graphics/SpriteBatch.cs b/src/Graphics/SpriteBatch.cs index b7c6050fc..44947b642 100644 --- a/src/Graphics/SpriteBatch.cs +++ b/src/Graphics/SpriteBatch.cs @@ -712,8 +712,7 @@ float layerDepth { throw new ArgumentNullException("text"); } - SpriteFont.StringBuilderView view = new SpriteFont.StringBuilderView(text); - DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); + DrawString(spriteFont, new SpriteFont.StringBuilderView(text), position, color, rotation, origin, scale, effects, layerDepth); } public void DrawString( @@ -774,8 +773,7 @@ float layerDepth { throw new ArgumentNullException("text"); } - SpriteFont.StringView view = new SpriteFont.StringView(text); - DrawString(spriteFont, ref view, position, color, rotation, origin, scale, effects, layerDepth); + DrawString(spriteFont, new SpriteFont.StringView(text), position, color, rotation, origin, scale, effects, layerDepth); } #endregion @@ -784,7 +782,7 @@ float layerDepth private void DrawString( SpriteFont spriteFont, - ref T text, + T text, Vector2 position, Color color, float rotation, @@ -792,7 +790,7 @@ private void DrawString( Vector2 scale, SpriteEffects effects, float layerDepth - ) where T : SpriteFont.IReadOnlyCharList + ) where T : struct, SpriteFont.IReadOnlyCharList { CheckBegin("DrawString"); if (text.Length == 0) @@ -822,7 +820,7 @@ float layerDepth float axisDirMirrorY = 0.0f; if (effects != SpriteEffects.None) { - Vector2 size = spriteFont.MeasureString(ref text); + Vector2 size = spriteFont.MeasureString(text); baseOffset.X -= size.X * axisIsMirroredX[(int) effects]; baseOffset.Y -= size.Y * axisIsMirroredY[(int) effects]; axisDirMirrorX = axisIsMirroredX[(int) effects]; diff --git a/src/Graphics/SpriteFont.cs b/src/Graphics/SpriteFont.cs index 078e2acbe..70cdc1134 100644 --- a/src/Graphics/SpriteFont.cs +++ b/src/Graphics/SpriteFont.cs @@ -124,7 +124,7 @@ internal SpriteFont( #region Internal MeasureString Method - internal Vector2 MeasureString(ref T text) where T : IReadOnlyCharList + internal Vector2 MeasureString(T text) where T : struct, IReadOnlyCharList { if (text.Length == 0) { @@ -221,8 +221,7 @@ public Vector2 MeasureString(string text) { throw new ArgumentNullException("text"); } - StringView view = new StringView(text); - return MeasureString(ref view); + return MeasureString(new StringView(text)); } public Vector2 MeasureString(StringBuilder text) @@ -231,8 +230,7 @@ public Vector2 MeasureString(StringBuilder text) { throw new ArgumentNullException("text"); } - StringBuilderView view = new StringBuilderView(text); - return MeasureString(ref view); + return MeasureString(new StringBuilderView(text)); } #endregion