From 40da4f029cd5e17810108b4992ce1ebfeb0c539f Mon Sep 17 00:00:00 2001 From: gamer147 Date: Thu, 30 Jul 2026 20:29:39 -0400 Subject: [PATCH] fix retained glyph bottom clipping --- docs/engine-re.md | 10 ++++ docs/phase-a-slice-plan.md | 9 ++++ docs/remake-architecture-and-roadmap.md | 8 ++- .../GlyphMaskCompositorTests.cs | 34 ++++++++++++ .../RetainedAdvTextLayoutPresentationTests.cs | 23 ++++++++ .../Model/AdvRetainedTextContract.cs | 15 ++++++ .../Text/RetainedAdvTextLayoutPresentation.cs | 41 ++++++++++---- .../Text/RetainedGlyphLayoutEngine.cs | 53 ++++++++++++++++++- godot/GodotAdvHost.cs | 5 +- 9 files changed, 185 insertions(+), 13 deletions(-) diff --git a/docs/engine-re.md b/docs/engine-re.md index 8336c3c..a2259f2 100644 --- a/docs/engine-re.md +++ b/docs/engine-re.md @@ -2894,6 +2894,16 @@ The shipped effect profiles use only modes 1 and 3: `360 / (sqrt(effect_x²+effect_y²)*8)`, blits the effect-colored glyph at every rounded offset, then draws the primary glyph. All 164 paired sites use `(1,1)`, producing the normal one-pixel outline. +**Measured cell versus raster ink (2026-07-30).** `adv_text_build_glyph_records@0x4576c0` stores +the `GetTextExtentPoint32A` measured cell edges in each retained record, and +`adv_text_publish_next_glyph@0x451220` uses those edges when it binds the native object. +`text_raster_string_cached@0x45b600`, however, composites the complete `GetGlyphOutlineA` black-box +bitmap and effect copies into the shared layout surface. Direct 24-pixel bold Mincho probes show several +ordinary Japanese glyphs reaching below the 24-pixel measured cell even before the mode-3 `(1,1)` outline. +Thus the record is a layout/publication cell, not a guaranteed ink bounding box. The port preserves that +native record and derives a separate transient vertical publication crop from mask origin/height and effect +offsets; its horizontal crop remains the measured cell to avoid exposing the next glyph during timed reveal. + The ordinary ADV preset is therefore 24-pixel `ＭＳ 明朝`, weight 700, white fill, mode-3 `(1,1)` outline in `0x606060`, with 8 pixels of line leading. The small menu-description preset is 16-pixel `ＭＳ ゴシック`, weight 0, white fill, mode 1 with zero displacement, and 9 pixels of leading. diff --git a/docs/phase-a-slice-plan.md b/docs/phase-a-slice-plan.md index 82627b8..c2abcf8 100644 --- a/docs/phase-a-slice-plan.md +++ b/docs/phase-a-slice-plan.md @@ -3674,6 +3674,15 @@ surface's z-order, alpha, tint, fade, transform, clipping, capture, and lifetime Validation: engine **580/580**, zero-warning Godot build, and threaded `SELFTEST OK` in both forced `portable-godot-textserver` and forced `windows-gdi-gray4` modes. +**Bottom-edge clipping follow-up (2026-07-30):** retained publication originally reused the native +measured-cell rectangle as its exact source crop. Direct GDI probing confirmed that the glyph black-box +bitmap, plus the mode-3 outline, can extend below that cell even though the pixels are present in the +layout surface. The native `AdvRetainedGlyphRecord` remains unchanged for layout and reconstruction; each +transient placement now carries a separate vertically ink-safe publication rectangle. Horizontal edges +remain cell-bounded so timed reveal cannot expose a neighboring glyph. Synthetic compositor/publication +regressions pass, the Godot build is clean, and forced exact and portable self-tests both report +`SELFTEST OK`. + ## Persistence native-format reconnaissance complete (2026-07-24) The deferred save/profile ownership question now has a compatibility-mode answer. The remaining native diff --git a/docs/remake-architecture-and-roadmap.md b/docs/remake-architecture-and-roadmap.md index 194f5e1..0b4943f 100644 --- a/docs/remake-architecture-and-roadmap.md +++ b/docs/remake-architecture-and-roadmap.md @@ -627,7 +627,10 @@ primitive, which cannot be made equivalent by choosing another embolden constant per-run origins, partial erase/republication, suspension, measured cursor handoff, reset, and reconstruction. The synthesized Godot VM self-test exercises timed exact reveal and reports `live-adv-text=retained-glyphs`; real GPU captures of SC0000 pages 1 and 7 confirm ordinary and voiced - dialogue placement/wrapping alongside the immediate speaker-name surface. + dialogue placement/wrapping alongside the immediate speaker-name surface. A 2026-07-30 clipping + follow-up keeps those native measured-cell records intact but gives transient placements a separate + vertically ink-safe source crop, because backend glyph black boxes and mode-3 outlines can extend below + the measured cell; horizontal crops remain cell-bounded to preserve reveal isolation. 6. **Migrate History and the ADV wait indicator through the same retained path. — Completed 2026-07-30.** Op `0x1d1` now rasterizes each HISTORY row into its target layout surface and publishes its native glyph interval through ordinary `GfxState` bindings. Row reset, wheel redraw, the broad native exit @@ -645,7 +648,8 @@ primitive, which cannot be made equivalent by choosing another embolden constant sizes. `auto` selects exact GDI when its ACP-932 gate passes and otherwise selects the explicitly non-pixel-exact Unicode backend; development runs can force either with `--text-backend`. Both modes pass the same immediate/live/History retained-glyph self-test. Gameplay `Label` pools and surface-text overlay - projection are removed, so text always participates in the ordinary surface compositor. + projection are removed, so text always participates in the ordinary surface compositor. Publication uses + the transient ink-safe crop established in step 5 in both backends. **Acceptance gates:** diff --git a/engine/Age.Engine.Tests/GlyphMaskCompositorTests.cs b/engine/Age.Engine.Tests/GlyphMaskCompositorTests.cs index c2a25fc..0f14b71 100644 --- a/engine/Age.Engine.Tests/GlyphMaskCompositorTests.cs +++ b/engine/Age.Engine.Tests/GlyphMaskCompositorTests.cs @@ -193,6 +193,40 @@ public class GlyphMaskCompositorTests Assert.Equal(255, Pixel(destination, 2, 7).A); } + [Fact] + public void RetainedPresentationCropIncludesInkBelowTheNativeCell() + { + var mask = new GlyphMask( + width: 1, height: 4, stride: 1, + originX: 0, originY: 3, + cellAdvanceX: 4, cellAdvanceY: 0, + cellWidth: 4, cellHeight: 5, + coverage: [16, 16, 16, 16]); + var destination = Image(16, 16); + var engine = new RetainedGlyphLayoutEngine( + new CountingRasterizer(_ => mask)); + + GlyphTextLayoutResult result = engine.Render( + destination, + new GlyphTextLayoutOptions( + CursorX: 2, CursorY: 1, LineOriginX: 2, + RightBound: 16, BottomBound: 16, WrapHorizontally: true, + Style(renderMode: 3) with + { + EffectOffsetX = 1, + EffectOffsetY = 1, + }), + [NativeRequest(0x82a0, 0x3042)]); + + Assert.Equal( + new AdvRetainedGlyphRecord(0, 2, 1, 6, 6), + Assert.Single(result.Records)); + Assert.Equal( + new AdvRetainedGlyphPresentationRect(2, 1, 6, 8), + Assert.Single(result.PresentationRects)); + Assert.NotEqual(0, Pixel(destination, 2, 7).A); + } + [Fact] public void ClosingPunctuationStaysOnOverflowingPrecedingLine() { diff --git a/engine/Age.Engine.Tests/RetainedAdvTextLayoutPresentationTests.cs b/engine/Age.Engine.Tests/RetainedAdvTextLayoutPresentationTests.cs index 5301ac8..9eb030a 100644 --- a/engine/Age.Engine.Tests/RetainedAdvTextLayoutPresentationTests.cs +++ b/engine/Age.Engine.Tests/RetainedAdvTextLayoutPresentationTests.cs @@ -79,6 +79,29 @@ public class RetainedAdvTextLayoutPresentationTests gfx.SnapshotVisibleObjects().Select(item => (item.DstX, item.DstY))); } + [Fact] + public void PublicationUsesInkSafeCropWithoutChangingNativeRecord() + { + var presentation = new RetainedAdvTextLayoutPresentation( + new AdvTextLayoutPresentationBinding( + 1, 21, 100, 4, -1, 10, 20)); + var record = new AdvRetainedGlyphRecord(0, 3, 4, 7, 10); + presentation.Append( + [record], + [new AdvRetainedGlyphPresentationRect(3, 3, 7, 12)], + layoutOriginX: 30, + layoutOriginY: 40); + var gfx = new GfxState(); + gfx.CreateSurface(21); + + presentation.PublishThrough(gfx, 1); + + RenderObject item = Assert.Single(gfx.SnapshotVisibleObjects()); + Assert.Equal((3, 3, 4, 9, 33, 43), + (item.SrcX, item.SrcY, item.W, item.H, item.DstX, item.DstY)); + Assert.Equal(record, Assert.Single(presentation.Glyphs).Record); + } + [Fact] public void SavedFrameStyleReconstructionRebuildsTheSameBindings() { diff --git a/engine/Age.Engine/Model/AdvRetainedTextContract.cs b/engine/Age.Engine/Model/AdvRetainedTextContract.cs index 17cb75f..5bb7520 100644 --- a/engine/Age.Engine/Model/AdvRetainedTextContract.cs +++ b/engine/Age.Engine/Model/AdvRetainedTextContract.cs @@ -34,8 +34,23 @@ public readonly record struct AdvTextLayoutPresentationBinding( int ResetCursorX, int ResetCursorY); +/// +/// Transient publication crop. Native records retain measured cell edges, but a backend mask/effect can +/// carry ink outside that cell. Keeping the expanded crop separate preserves the native layout ABI. +/// +public readonly record struct AdvRetainedGlyphPresentationRect( + int Left, + int Top, + int Right, + int Bottom) +{ + public int Width => Right - Left; + public int Height => Bottom - Top; +} + public readonly record struct AdvRetainedGlyphPlacement( AdvRetainedGlyphRecord Record, + AdvRetainedGlyphPresentationRect PresentationRect, int LayoutOriginX, int LayoutOriginY); diff --git a/engine/Age.Engine/Text/RetainedAdvTextLayoutPresentation.cs b/engine/Age.Engine/Text/RetainedAdvTextLayoutPresentation.cs index 27a2062..d5d7bce 100644 --- a/engine/Age.Engine/Text/RetainedAdvTextLayoutPresentation.cs +++ b/engine/Age.Engine/Text/RetainedAdvTextLayoutPresentation.cs @@ -38,7 +38,30 @@ public sealed class RetainedAdvTextLayoutPresentation int first = _glyphs.Count; foreach (AdvRetainedGlyphRecord record in records) _glyphs.Add(new AdvRetainedGlyphPlacement( - record, layoutOriginX, layoutOriginY)); + record, + new AdvRetainedGlyphPresentationRect( + record.Left, record.Top, record.Right, record.Bottom), + layoutOriginX, + layoutOriginY)); + return first; + } + + public int Append( + IReadOnlyList records, + IReadOnlyList presentationRects, + int layoutOriginX, + int layoutOriginY) + { + ArgumentNullException.ThrowIfNull(records); + ArgumentNullException.ThrowIfNull(presentationRects); + if (presentationRects.Count != records.Count) + throw new ArgumentException( + "Every retained glyph record requires one presentation rectangle.", + nameof(presentationRects)); + int first = _glyphs.Count; + for (int index = 0; index < records.Count; index++) + _glyphs.Add(new AdvRetainedGlyphPlacement( + records[index], presentationRects[index], layoutOriginX, layoutOriginY)); return first; } @@ -71,16 +94,16 @@ public sealed class RetainedAdvTextLayoutPresentation private void Bind(GfxState gfx, int index) { AdvRetainedGlyphPlacement placement = _glyphs[index]; - AdvRetainedGlyphRecord record = placement.Record; - if (record.Width <= 0 || record.Height <= 0) return; + AdvRetainedGlyphPresentationRect rect = placement.PresentationRect; + if (rect.Width <= 0 || rect.Height <= 0) return; gfx.BindDraw( checked(Binding.FirstObjectHandle + index), Binding.SourceSurfaceSlot, - record.Left, - record.Top, - record.Width, - record.Height, - checked(placement.LayoutOriginX + record.Left), - checked(placement.LayoutOriginY + record.Top)); + rect.Left, + rect.Top, + rect.Width, + rect.Height, + checked(placement.LayoutOriginX + rect.Left), + checked(placement.LayoutOriginY + rect.Top)); } } diff --git a/engine/Age.Engine/Text/RetainedGlyphLayoutEngine.cs b/engine/Age.Engine/Text/RetainedGlyphLayoutEngine.cs index 5d032dc..4bb824e 100644 --- a/engine/Age.Engine/Text/RetainedGlyphLayoutEngine.cs +++ b/engine/Age.Engine/Text/RetainedGlyphLayoutEngine.cs @@ -14,6 +14,7 @@ public readonly record struct GlyphTextLayoutOptions( public sealed record GlyphTextLayoutResult( IReadOnlyList Records, + IReadOnlyList PresentationRects, int CursorX, int CursorY, int ConsumedGlyphs, @@ -40,6 +41,8 @@ public sealed class RetainedGlyphLayoutEngine ArgumentNullException.ThrowIfNull(destination); ArgumentNullException.ThrowIfNull(glyphs); var records = new List(glyphs.Count); + var presentationRects = + new List(glyphs.Count); int cursorX = options.CursorX; int cursorY = options.CursorY; int wrappedLines = 0; @@ -81,11 +84,59 @@ public sealed class RetainedGlyphLayoutEngine AgeGlyphMaskCompositor.DrawGlyph( destination, mask, cursorX, cursorY, request.PixelHeight, options.Style); records.Add(new AdvRetainedGlyphRecord(0, cursorX, cursorY, right, bottom)); + presentationRects.Add(PresentationRect( + destination.Height, + mask, + cursorX, + cursorY, + right, + bottom, + request.PixelHeight, + options.Style)); cursorX = checked(cursorX + mask.CellAdvanceX); cursorY = checked(cursorY + mask.CellAdvanceY); } return new GlyphTextLayoutResult( - records, cursorX, cursorY, records.Count, wrappedLines, observed, stopped); + records, presentationRects, cursorX, cursorY, records.Count, + wrappedLines, observed, stopped); + } + + private static AdvRetainedGlyphPresentationRect PresentationRect( + int surfaceHeight, + GlyphMask mask, + int cellLeft, + int cellTop, + int cellRight, + int cellBottom, + int topToBaseline, + AdvTextStyle style) + { + int minimumEffectY = 0; + int maximumEffectY = 0; + if (style.RenderMode == 1) + { + minimumEffectY = Math.Min(0, style.EffectOffsetY); + maximumEffectY = Math.Max(0, style.EffectOffsetY); + } + else if (style.RenderMode == 3) + { + foreach ((int _, int y) in AgeGlyphMaskCompositor.GetMode3OutlineOffsets( + style.EffectOffsetX, style.EffectOffsetY)) + { + minimumEffectY = Math.Min(minimumEffectY, y); + maximumEffectY = Math.Max(maximumEffectY, y); + } + } + + int inkTop = checked( + cellTop + topToBaseline - mask.OriginY + minimumEffectY); + int inkBottom = checked( + cellTop + topToBaseline - mask.OriginY + mask.Height + maximumEffectY); + return new AdvRetainedGlyphPresentationRect( + cellLeft, + Math.Clamp(Math.Min(cellTop, inkTop), 0, surfaceHeight), + cellRight, + Math.Clamp(Math.Max(cellBottom, inkBottom), 0, surfaceHeight)); } } diff --git a/godot/GodotAdvHost.cs b/godot/GodotAdvHost.cs index 92fa213..c945d46 100644 --- a/godot/GodotAdvHost.cs +++ b/godot/GodotAdvHost.cs @@ -425,7 +425,10 @@ public sealed class GodotAdvHost : IHost throw new InvalidOperationException( $"ADV layout {binding.LayoutSlot} changed its retained binding without reset."); int first = presentation.Append( - rendered.Records, run.Layout.OriginX, run.Layout.OriginY); + rendered.Records, + rendered.PresentationRects, + run.Layout.OriginX, + run.Layout.OriginY); result = new AdvRetainedTextRunResult( binding.LayoutSlot, first,