From b163c5362236936b0d7b953477e68c27b2a3bcf4 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 20:42:49 +0200 Subject: [PATCH] phase(N.5) Task 9 fixup: layout assertion + DrawCommandStride const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code quality review caught: - sizeofDEIC was a local; promoted to public const DrawCommandStride so tests can reference it symbolically. - BatchDataPublic layout invariant (size + field offsets) wasn't asserted in tests. Added BatchDataPublic_LayoutMatchesPrivateBatchData + DrawCommandStride_MatchesStructSize tests to gate Task 10's MemoryMarshal.Cast safety. - Plan doc updated: BatchDataPublic spec was Pack=4 (wrong — must match private BatchData's Pack=8 for the cast to work). Implementation was already correct; plan now matches. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-08-phase-n5-modern-rendering.md | 6 +++--- .../Rendering/Wb/WbDrawDispatcher.cs | 10 ++++++++-- .../WbDrawDispatcherIndirectBuilderTests.cs | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md b/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md index 471da6b..d9269a7 100644 --- a/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md +++ b/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md @@ -1364,7 +1364,8 @@ public readonly record struct IndirectGroupInput( TranslucencyKind Translucency); /// Public mirror of the per-group BatchData laid into the SSBO. Tests verify alignment. -[StructLayout(LayoutKind.Sequential, Pack = 4)] +// Pack=8 (not 4) — must stay layout-identical to private BatchData for Task 10's MemoryMarshal.Cast. +[StructLayout(LayoutKind.Sequential, Pack = 8)] public struct BatchDataPublic { public ulong TextureHandle; @@ -1431,8 +1432,7 @@ public static IndirectLayoutResult BuildIndirectArrays( } } - int sizeofDEIC = 20; // matches struct layout - return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * sizeofDEIC); + return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * DrawCommandStride); } private static bool IsOpaque(TranslucencyKind t) diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 2912472..6d33293 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -534,6 +534,13 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // These are public so the pure-CPU unit tests in AcDream.Core.Tests can // exercise BuildIndirectArrays without needing a GL context. + /// + /// Stride in bytes of DrawElementsIndirectCommand in the indirect buffer. + /// 5 × uint = 20 bytes. Tests and callers reference this symbolically + /// rather than hard-coding 20 so a layout change produces a compile error. + /// + public const int DrawCommandStride = 20; // sizeof(DrawElementsIndirectCommand): 5 × uint + /// /// Public view of the per-group inputs to — used in tests. /// @@ -622,8 +629,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable } } - const int SizeofDEIC = 20; // sizeof(DrawElementsIndirectCommand) — 5 × uint - return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * SizeofDEIC); + return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * DrawCommandStride); } private static bool IsOpaque(TranslucencyKind t) diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherIndirectBuilderTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherIndirectBuilderTests.cs index 1f2e552..855a2ef 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherIndirectBuilderTests.cs +++ b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherIndirectBuilderTests.cs @@ -91,4 +91,23 @@ public sealed class WbDrawDispatcherIndirectBuilderTests Assert.Equal(1, result.OpaqueCount); Assert.Equal(0, result.TransparentCount); } + + [Fact] + public void BatchDataPublic_LayoutMatchesPrivateBatchData() + { + // Task 10 will use MemoryMarshal.Cast to + // expose the dispatcher's per-frame BatchData[] scratch to BuildIndirectArrays + // without copying. The cast is only safe if the structs have identical + // layout (size, field offsets). Both use [StructLayout(Sequential, Pack=8)]. + Assert.Equal(16, System.Runtime.CompilerServices.Unsafe.SizeOf()); + Assert.Equal(0, (int)System.Runtime.InteropServices.Marshal.OffsetOf(nameof(WbDrawDispatcher.BatchDataPublic.TextureHandle))); + Assert.Equal(8, (int)System.Runtime.InteropServices.Marshal.OffsetOf(nameof(WbDrawDispatcher.BatchDataPublic.TextureLayer))); + Assert.Equal(12, (int)System.Runtime.InteropServices.Marshal.OffsetOf(nameof(WbDrawDispatcher.BatchDataPublic.Flags))); + } + + [Fact] + public void DrawCommandStride_MatchesStructSize() + { + Assert.Equal(WbDrawDispatcher.DrawCommandStride, System.Runtime.CompilerServices.Unsafe.SizeOf()); + } }