Fix CreateImGuiRangesFrom to omit null char (#1709)

* Fix CreateImGuiRangesFrom to omit null char

UnicodeRanges.BasicLatin is [0, 127], but ImGui stops reading the glyph
range list on encountering a zero. Fixed that by ensuring that 0 never
appears in the glyph range list.

* Make problems explicit

---------

Co-authored-by: goat <16760685+goaaats@users.noreply.github.com>
This commit is contained in:
srkizer 2024-03-17 01:02:36 +09:00 committed by GitHub
parent 87b9edb448
commit e52c2755cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 51 additions and 9 deletions

View file

@ -85,6 +85,10 @@ public interface IFontAtlas : IDisposable
/// <summary>Creates a new <see cref="IFontHandle"/> from game's built-in fonts.</summary> /// <summary>Creates a new <see cref="IFontHandle"/> from game's built-in fonts.</summary>
/// <param name="style">Font to use.</param> /// <param name="style">Font to use.</param>
/// <returns>Handle to a font that may or may not be ready yet.</returns> /// <returns>Handle to a font that may or may not be ready yet.</returns>
/// <exception cref="InvalidOperationException">When called during <see cref="BuildStepChange"/>,
/// <see cref="UiBuilder.BuildFonts"/>, <see cref="UiBuilder.AfterBuildFonts"/>, and alike. Move the font handle
/// creating code outside those handlers, and only initialize them once. Call <see cref="IDisposable.Dispose"/>
/// on a previous font handle if you're replacing one.</exception>
/// <remarks>This function does not throw. <see cref="IFontHandle.LoadException"/> will be populated instead, if /// <remarks>This function does not throw. <see cref="IFontHandle.LoadException"/> will be populated instead, if
/// the build procedure has failed. <see cref="IFontHandle.Push"/> can be used regardless of the state of the font /// the build procedure has failed. <see cref="IFontHandle.Push"/> can be used regardless of the state of the font
/// handle.</remarks> /// handle.</remarks>
@ -93,6 +97,13 @@ public interface IFontAtlas : IDisposable
/// <summary>Creates a new IFontHandle using your own callbacks.</summary> /// <summary>Creates a new IFontHandle using your own callbacks.</summary>
/// <param name="buildStepDelegate">Callback for <see cref="IFontAtlas.BuildStepChange"/>.</param> /// <param name="buildStepDelegate">Callback for <see cref="IFontAtlas.BuildStepChange"/>.</param>
/// <returns>Handle to a font that may or may not be ready yet.</returns> /// <returns>Handle to a font that may or may not be ready yet.</returns>
/// <exception cref="InvalidOperationException">When called during <see cref="BuildStepChange"/>,
/// <see cref="UiBuilder.BuildFonts"/>, <see cref="UiBuilder.AfterBuildFonts"/>, and alike. Move the font handle
/// creating code outside those handlers, and only initialize them once. Call <see cref="IDisposable.Dispose"/>
/// on a previous font handle if you're replacing one.</exception>
/// <remarks>Consider calling <see cref="IFontAtlasBuildToolkitPreBuild.AttachExtraGlyphsForDalamudLanguage"/> to
/// support glyphs that are not supplied by the game by default; this mostly affects Chinese and Korean language
/// users.</remarks>
/// <remarks> /// <remarks>
/// <para>Consider calling <see cref="IFontAtlasBuildToolkitPreBuild.AttachExtraGlyphsForDalamudLanguage"/> to /// <para>Consider calling <see cref="IFontAtlasBuildToolkitPreBuild.AttachExtraGlyphsForDalamudLanguage"/> to
/// support glyphs that are not supplied by the game by default; this mostly affects Chinese and Korean language /// support glyphs that are not supplied by the game by default; this mostly affects Chinese and Korean language

View file

@ -35,6 +35,9 @@ internal sealed partial class FontAtlasFactory
/// </summary> /// </summary>
public const string EllipsisCodepoints = "\u2026\u0085"; public const string EllipsisCodepoints = "\u2026\u0085";
/// <summary>Marker for tasks on whether it's being called inside a font build cycle.</summary>
public static readonly AsyncLocal<bool> IsBuildInProgressForTask = new();
/// <summary> /// <summary>
/// If set, disables concurrent font build operation. /// If set, disables concurrent font build operation.
/// </summary> /// </summary>
@ -427,11 +430,28 @@ internal sealed partial class FontAtlasFactory
} }
/// <inheritdoc/> /// <inheritdoc/>
public IFontHandle NewGameFontHandle(GameFontStyle style) => this.gameFontHandleManager.NewFontHandle(style); public IFontHandle NewGameFontHandle(GameFontStyle style)
{
if (IsBuildInProgressForTask.Value)
{
throw new InvalidOperationException(
$"{nameof(this.NewGameFontHandle)} may not be called during {nameof(this.BuildStepChange)}, the callback of {nameof(this.NewDelegateFontHandle)}, {nameof(UiBuilder.BuildFonts)} or {nameof(UiBuilder.AfterBuildFonts)}.");
}
return this.gameFontHandleManager.NewFontHandle(style);
}
/// <inheritdoc/> /// <inheritdoc/>
public IFontHandle NewDelegateFontHandle(FontAtlasBuildStepDelegate buildStepDelegate) => public IFontHandle NewDelegateFontHandle(FontAtlasBuildStepDelegate buildStepDelegate)
this.delegateFontHandleManager.NewFontHandle(buildStepDelegate); {
if (IsBuildInProgressForTask.Value)
{
throw new InvalidOperationException(
$"{nameof(this.NewDelegateFontHandle)} may not be called during {nameof(this.BuildStepChange)} or the callback of {nameof(this.NewDelegateFontHandle)}, {nameof(UiBuilder.BuildFonts)} or {nameof(UiBuilder.AfterBuildFonts)}.");
}
return this.delegateFontHandleManager.NewFontHandle(buildStepDelegate);
}
/// <inheritdoc/> /// <inheritdoc/>
public void BuildFontsOnNextFrame() public void BuildFontsOnNextFrame()
@ -630,6 +650,8 @@ internal sealed partial class FontAtlasFactory
FontAtlasBuiltData? res = null; FontAtlasBuiltData? res = null;
nint atlasPtr = 0; nint atlasPtr = 0;
BuildToolkit? toolkit = null; BuildToolkit? toolkit = null;
IsBuildInProgressForTask.Value = true;
try try
{ {
res = new(this, scale); res = new(this, scale);
@ -754,6 +776,7 @@ internal sealed partial class FontAtlasFactory
// ReSharper disable once ConstantConditionalAccessQualifier // ReSharper disable once ConstantConditionalAccessQualifier
toolkit?.Dispose(); toolkit?.Dispose();
this.buildQueued = false; this.buildQueued = false;
IsBuildInProgressForTask.Value = false;
} }
unsafe bool ValidateMergeFontReferences(ImFontPtr replacementDstFont) unsafe bool ValidateMergeFontReferences(ImFontPtr replacementDstFont)

View file

@ -516,9 +516,16 @@ public sealed class UiBuilder : IDisposable
/// <returns>Handle to the game font which may or may not be available for use yet.</returns> /// <returns>Handle to the game font which may or may not be available for use yet.</returns>
[Obsolete($"Use {nameof(this.FontAtlas)}.{nameof(IFontAtlas.NewGameFontHandle)} instead.", false)] [Obsolete($"Use {nameof(this.FontAtlas)}.{nameof(IFontAtlas.NewGameFontHandle)} instead.", false)]
[Api10ToDo(Api10ToDoAttribute.DeleteCompatBehavior)] [Api10ToDo(Api10ToDoAttribute.DeleteCompatBehavior)]
public GameFontHandle GetGameFontHandle(GameFontStyle style) => new( public GameFontHandle GetGameFontHandle(GameFontStyle style)
(GamePrebakedFontHandle)this.FontAtlas.NewGameFontHandle(style), {
Service<FontAtlasFactory>.Get()); var prevValue = FontAtlasFactory.IsBuildInProgressForTask.Value;
FontAtlasFactory.IsBuildInProgressForTask.Value = false;
var v = new GameFontHandle(
(GamePrebakedFontHandle)this.FontAtlas.NewGameFontHandle(style),
Service<FontAtlasFactory>.Get());
FontAtlasFactory.IsBuildInProgressForTask.Value = prevValue;
return v;
}
/// <summary> /// <summary>
/// Call this to queue a rebuild of the font atlas.<br/> /// Call this to queue a rebuild of the font atlas.<br/>

View file

@ -493,12 +493,13 @@ public static class ImGuiHelpers
/// <returns>The range array that can be used for <see cref="SafeFontConfig.GlyphRanges"/>.</returns> /// <returns>The range array that can be used for <see cref="SafeFontConfig.GlyphRanges"/>.</returns>
public static ushort[] CreateImGuiRangesFrom(IEnumerable<UnicodeRange> ranges) => public static ushort[] CreateImGuiRangesFrom(IEnumerable<UnicodeRange> ranges) =>
ranges ranges
.Where(x => x.FirstCodePoint <= ushort.MaxValue) .Select(x => (First: Math.Max(x.FirstCodePoint, 1), Last: x.FirstCodePoint + x.Length))
.Where(x => x.First <= ushort.MaxValue && x.First <= x.Last)
.SelectMany( .SelectMany(
x => new[] x => new[]
{ {
(ushort)Math.Min(x.FirstCodePoint, ushort.MaxValue), (ushort)Math.Min(x.First, ushort.MaxValue),
(ushort)Math.Min(x.FirstCodePoint + x.Length, ushort.MaxValue), (ushort)Math.Min(x.Last, ushort.MaxValue),
}) })
.Append((ushort)0) .Append((ushort)0)
.ToArray(); .ToArray();