phase(N.5) Task 9 fixup: layout assertion + DrawCommandStride const
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<BatchData, BatchDataPublic> 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) <noreply@anthropic.com>
This commit is contained in:
parent
9a7a250b62
commit
b163c53622
3 changed files with 30 additions and 5 deletions
|
|
@ -1364,7 +1364,8 @@ public readonly record struct IndirectGroupInput(
|
||||||
TranslucencyKind Translucency);
|
TranslucencyKind Translucency);
|
||||||
|
|
||||||
/// <summary>Public mirror of the per-group BatchData laid into the SSBO. Tests verify alignment.</summary>
|
/// <summary>Public mirror of the per-group BatchData laid into the SSBO. Tests verify alignment.</summary>
|
||||||
[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 struct BatchDataPublic
|
||||||
{
|
{
|
||||||
public ulong TextureHandle;
|
public ulong TextureHandle;
|
||||||
|
|
@ -1431,8 +1432,7 @@ public static IndirectLayoutResult BuildIndirectArrays(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
int sizeofDEIC = 20; // matches struct layout
|
return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * DrawCommandStride);
|
||||||
return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * sizeofDEIC);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static bool IsOpaque(TranslucencyKind t)
|
private static bool IsOpaque(TranslucencyKind t)
|
||||||
|
|
|
||||||
|
|
@ -534,6 +534,13 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
// These are public so the pure-CPU unit tests in AcDream.Core.Tests can
|
// These are public so the pure-CPU unit tests in AcDream.Core.Tests can
|
||||||
// exercise BuildIndirectArrays without needing a GL context.
|
// exercise BuildIndirectArrays without needing a GL context.
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Stride in bytes of <c>DrawElementsIndirectCommand</c> in the indirect buffer.
|
||||||
|
/// 5 × <c>uint</c> = 20 bytes. Tests and callers reference this symbolically
|
||||||
|
/// rather than hard-coding <c>20</c> so a layout change produces a compile error.
|
||||||
|
/// </summary>
|
||||||
|
public const int DrawCommandStride = 20; // sizeof(DrawElementsIndirectCommand): 5 × uint
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Public view of the per-group inputs to <see cref="BuildIndirectArrays"/> — used in tests.
|
/// Public view of the per-group inputs to <see cref="BuildIndirectArrays"/> — used in tests.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|
@ -622,8 +629,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const int SizeofDEIC = 20; // sizeof(DrawElementsIndirectCommand) — 5 × uint
|
return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * DrawCommandStride);
|
||||||
return new IndirectLayoutResult(opaqueCount, transparentCount, opaqueCount * SizeofDEIC);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static bool IsOpaque(TranslucencyKind t)
|
private static bool IsOpaque(TranslucencyKind t)
|
||||||
|
|
|
||||||
|
|
@ -91,4 +91,23 @@ public sealed class WbDrawDispatcherIndirectBuilderTests
|
||||||
Assert.Equal(1, result.OpaqueCount);
|
Assert.Equal(1, result.OpaqueCount);
|
||||||
Assert.Equal(0, result.TransparentCount);
|
Assert.Equal(0, result.TransparentCount);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BatchDataPublic_LayoutMatchesPrivateBatchData()
|
||||||
|
{
|
||||||
|
// Task 10 will use MemoryMarshal.Cast<BatchData, BatchDataPublic> 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<WbDrawDispatcher.BatchDataPublic>());
|
||||||
|
Assert.Equal(0, (int)System.Runtime.InteropServices.Marshal.OffsetOf<WbDrawDispatcher.BatchDataPublic>(nameof(WbDrawDispatcher.BatchDataPublic.TextureHandle)));
|
||||||
|
Assert.Equal(8, (int)System.Runtime.InteropServices.Marshal.OffsetOf<WbDrawDispatcher.BatchDataPublic>(nameof(WbDrawDispatcher.BatchDataPublic.TextureLayer)));
|
||||||
|
Assert.Equal(12, (int)System.Runtime.InteropServices.Marshal.OffsetOf<WbDrawDispatcher.BatchDataPublic>(nameof(WbDrawDispatcher.BatchDataPublic.Flags)));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DrawCommandStride_MatchesStructSize()
|
||||||
|
{
|
||||||
|
Assert.Equal(WbDrawDispatcher.DrawCommandStride, System.Runtime.CompilerServices.Unsafe.SizeOf<DrawElementsIndirectCommand>());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue