feat(render): add integer vertex attributes and the terrain tiling binding

A scouting pass over V4d stopped before writing code and reported three gaps between terrain and the pinned contract. All three verified against source.

The load-bearing one: terrain_modern.vert declares locations 2-5 as uvec4 and TerrainModernRenderer feeds them with glVertexAttribIPointer, but GpuVertexFormat had no integer format and the encoder only issued glVertexAttribPointer. GL leaves an integer shader input undefined if it arrives through the float path, and Vulkan needs the format named as R8G8B8A8_UINT rather than _UNORM, so UByte4Normalized cannot stand in for it. Those packed bytes carry terrain-type, road and split-direction codes that drive every blend decision, so normalising them would have produced garbage rather than an approximation. Adds GpuVertexFormat.UByte4UInt and an integer branch in the encoder.

Also adds a uniform binding for terrain's 36-float per-layer tiling array, which at 144 bytes cannot ride in the 96-byte push-constant block or Vulkan's guaranteed 128-byte ceiling, and has no uniform-array verb to reach it otherwise.

Corrects two V4d plan rows: TerrainAtlas belongs to V4t with the rest of the texture stack, and terrain has no GPU timer to port since its diagnostics use a CPU stopwatch. The uView/uProjection convergence gets its own pixel-gated sub-commit because it moves a matrix product from per-vertex GPU evaluation to a CPU multiply, and that rounding effect should be attributable on its own.

Files #250: two zero-allocation tests fail about one run in three on an unchanged tree, independent of this campaign. That noise trains everyone to re-run until green, which is how a real regression gets waved through.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-27 21:11:48 +02:00
parent cb0182a06c
commit c7f5f251f8
7 changed files with 136 additions and 9 deletions

View file

@ -97,6 +97,49 @@ Copy this block when adding a new issue:
---
## #250 — Zero-allocation tests fail intermittently, roughly 1 run in 3
**Status:** OPEN
**Severity:** MEDIUM (undermines every "tests green" gate)
**Filed:** 2026-07-27
**Component:** tests / allocation assertions
**Description:** Two tests fail non-deterministically on an otherwise unchanged
tree:
- `AcDream.App.Tests.UI.UiDatFontTests.InstanceMeasureWidth_ReusesGlyphTableWithoutAllocating`
- `AcDream.App.Tests.Rendering.RenderFrameProductTests.WarmProductBuildAndBorrowAllocateNothing`
Measured over six consecutive Release runs of the App suite on an unmodified
tree: four passed 3,846/3, and two failed with exactly one failure — a different
one of the pair each time. So the observed rate is about one run in three, and
it is not specific to either test.
Both assert that a warmed code path allocates zero managed bytes. That
measurement is inherently sensitive to anything else the runtime does on the
thread — tiered JIT recompilation and background GC bookkeeping can both attribute
bytes to the measured window.
**Why it matters now:** Campaign V's acceptance criteria include 0 B/frame
steady-state managed allocation, and these are the tests that guard it. A gate
that fails a third of the time for unrelated reasons trains everyone to re-run
until green, which is exactly how a real regression gets waved through — see the
V4a revert, where a failing gate was rationalised rather than investigated.
**Provenance:** first observed during Campaign V slice V2 and dismissed as
unrelated flakiness; seen again independently by the V4c and V4d agents; then
reproduced deliberately here. Not caused by the campaign.
**Fix direction:** warm the path harder before measuring (force tiered
promotion), take the best of N samples rather than a single one, or measure with
`GC.TryStartNoGCRegion`. Whichever is chosen, the assertion should stay strict —
the goal is to remove the measurement noise, not to loosen the bound.
**Acceptance:** twenty consecutive Release runs of the App suite with zero
failures.
---
## #249 — Bindless handles stay resident after their table slot is released
**Status:** OPEN

View file

@ -517,7 +517,7 @@ because sample positions are not specified across implementations.
| **V4b** | `GlobalMeshBuffer` + `ObjectMeshManager` onto `IGpuBuffer`; arena, LRU and ledger logic untouched. | pixel gate |
| **V4c** | **The large one.** `WbDrawDispatcher` + `EnvCellRenderer`: per-frame uploads → rings, MDI brackets → pipelines + `MultiDrawIndexedIndirect`, loose uniforms → push constants, timer scopes. `RetailAlphaQueue` and all bucketing untouched. **Narrowed after the V4c scouting report — see §5.3.** | pixel gate at several checkpoints + connected lifecycle |
| **V4t** | **World texture stack** (added 2026-07-27, see §5.3): `TextureCache`, `CompositeTextureArrayCache`, `ManagedGLTextureArray`, `TerrainAtlas` and `ObjectMeshManager`'s material path onto `IGpuTexture`/`IGpuSampler`; retype `GroupKey`, `CachedBatch` and `ObjectRenderBatch` from `ulong` bindless handle to `GpuTextureSlot`; retire the interim per-renderer handle tables for V4c, V4d and V4e at once. | pixel gate |
| **V4d** | `TerrainModernRenderer` + `TerrainAtlas`. | pixel gate |
| **V4d** | `TerrainModernRenderer` only — **`TerrainAtlas` belongs to V4t** with the rest of the texture stack. Two sub-commits: first the `uView`/`uProjection``uViewProjection` shader convergence on its own pixel gate (it moves a matrix product from per-vertex GPU to a CPU multiply, so its rounding effect must be attributable alone), then the plumbing. Terrain has no GPU timer to port — its diagnostics use a CPU `Stopwatch`. | pixel gate per sub-commit |
| **V4e** | `ParticleRenderer` (after V4c — shared alpha-queue contract). | pixel gate (particle-heavy checkpoint) |
| **V4f** | `SkyRenderer` + weather. | pixel gate (dawn/dusk, day group pinned) |
| **V4g** | `PrivateEntityViewportRenderer``IGpuRenderTarget`; `PortalDepthMaskRenderer` + `PortalTunnelPresentation` → stencil/depth-mask pipelines. | pixel gate incl. paperdoll and portal transit |

View file

@ -3,7 +3,17 @@ using Silk.NET.OpenGL;
namespace AcDream.App.Rendering.Gpu.Gl;
/// <summary>One vertex attribute's GL shape: component count, element type, and whether integer values normalize to [0,1]/[-1,1].</summary>
internal readonly record struct GlVertexAttributeShape(int ComponentCount, VertexAttribPointerType Type, bool Normalized);
/// <summary>
/// How one vertex attribute reaches GL. <paramref name="Integer"/> selects
/// <c>glVertexAttribIPointer</c> over <c>glVertexAttribPointer</c>: GL requires
/// the integer entry point for an integer shader input (<c>uvec4</c> and friends)
/// and leaves the value undefined otherwise.
/// </summary>
internal readonly record struct GlVertexAttributeShape(
int ComponentCount,
VertexAttribPointerType Type,
bool Normalized,
bool Integer = false);
/// <summary>
/// Pure, GL-context-free mappings from the RHI's backend-neutral enums to
@ -21,6 +31,9 @@ internal static class GlEnumMapping
GpuVertexFormat.Float3 => new GlVertexAttributeShape(3, VertexAttribPointerType.Float, false),
GpuVertexFormat.Float4 => new GlVertexAttributeShape(4, VertexAttribPointerType.Float, false),
GpuVertexFormat.UByte4Normalized => new GlVertexAttributeShape(4, VertexAttribPointerType.UnsignedByte, true),
// Integer attributes carry Integer = true; the encoder must route them
// through glVertexAttribIPointer, not the normalized float path.
GpuVertexFormat.UByte4UInt => new GlVertexAttributeShape(4, VertexAttribPointerType.UnsignedByte, false, Integer: true),
_ => throw new NotSupportedException($"No GL vertex attribute shape for {format}."),
};

View file

@ -110,13 +110,27 @@ internal sealed class GlGpuPassEncoder : IGpuPassEncoder
{
GlVertexAttributeShape shape = GlEnumMapping.VertexShapeOf(attribute.Format);
nint attributeOffset = (nint)(offsetBytes + attribute.OffsetBytes);
_gl.VertexAttribPointer(
attribute.Location,
shape.ComponentCount,
shape.Type,
shape.Normalized,
layout.StrideBytes,
(void*)attributeOffset);
if (shape.Integer)
{
// An integer shader input (uvec4) must come through the I-form.
// Supplying it via glVertexAttribPointer leaves the value undefined.
_gl.VertexAttribIPointer(
attribute.Location,
shape.ComponentCount,
(VertexAttribIType)shape.Type,
layout.StrideBytes,
(void*)attributeOffset);
}
else
{
_gl.VertexAttribPointer(
attribute.Location,
shape.ComponentCount,
shape.Type,
shape.Normalized,
layout.StrideBytes,
(void*)attributeOffset);
}
}
GLHelpers.ThrowOnResourceError(_gl, $"bind vertex buffer '{buffer.Name}'");
_gl.BindBuffer(GLEnum.ArrayBuffer, 0);

View file

@ -78,6 +78,17 @@ internal static class GpuBindingModel
/// </summary>
public const uint UniformSceneLighting = 1;
/// <summary>
/// Terrain per-layer texture tiling factors — 36 floats.
///
/// Added at slice V4d. These live in a <c>uniform float[36]</c> today, which
/// is 144 bytes: too large for the 96-byte push-constant block (and for
/// Vulkan's guaranteed 128-byte ceiling), and there is no RHI verb for setting
/// a uniform array. A small uniform buffer is the Vulkan-legal home. Binding 2
/// is taken by the terrain clip block, so this is 3.
/// </summary>
public const uint UniformTerrainTiling = 3;
/// <summary>Set index carrying every uniform buffer.</summary>
public const uint UniformSet = 1;

View file

@ -9,7 +9,25 @@ internal enum GpuVertexFormat
Float2,
Float3,
Float4,
/// <summary>Four unsigned bytes scaled to [0,1] floats — a shader <c>vec4</c> input.</summary>
UByte4Normalized,
/// <summary>
/// Four unsigned bytes delivered as INTEGERS — a shader <c>uvec4</c> input.
///
/// Distinct from <see cref="UByte4Normalized"/> in kind, not just in scaling:
/// GL requires <c>glVertexAttribIPointer</c> for an integer shader input and
/// leaves the value undefined if it arrives through the float path, and Vulkan
/// needs the format named as <c>R8G8B8A8_UINT</c> rather than <c>_UNORM</c>.
///
/// Added at slice V4d, which found `terrain_modern.vert` declares locations 25
/// as <c>uvec4</c> and feeds them with <c>glVertexAttribIPointer</c>. Those
/// packed bytes carry terrain-type, road and split-direction codes that drive
/// every blend decision, so normalising them would not be an approximation —
/// it would be garbage.
/// </summary>
UByte4UInt,
}
/// <summary>One vertex attribute, matching a <c>layout(location = N) in</c> declaration.</summary>

View file

@ -86,6 +86,34 @@ public sealed class GpuContractTests
Assert.Contains(GpuBlendMode.InverseAlpha, Enum.GetValues<GpuBlendMode>());
}
[Fact]
public void IntegerVertexAttributesAreRepresentableDistinctlyFromNormalizedOnes()
{
// terrain_modern.vert declares locations 2-5 as uvec4 and the CPU feeds
// them with glVertexAttribIPointer. GL leaves an integer shader input
// undefined if it arrives through the float path, and Vulkan needs the
// format named as R8G8B8A8_UINT rather than _UNORM — so the two cannot be
// the same contract value. Those packed bytes carry terrain-type, road and
// split-direction codes, so normalising them would produce garbage, not an
// approximation.
Assert.NotEqual(GpuVertexFormat.UByte4Normalized, GpuVertexFormat.UByte4UInt);
Assert.Contains(GpuVertexFormat.UByte4UInt, Enum.GetValues<GpuVertexFormat>());
}
[Fact]
public void UniformBindingsDoNotCollide()
{
uint[] uniformBindings =
[
GpuBindingModel.UniformSceneLighting,
GpuBindingModel.UniformTerrainTiling,
];
Assert.Equal(uniformBindings.Length, uniformBindings.Distinct().Count());
// Binding 2 is the terrain clip block; the tiling array must not take it.
Assert.NotEqual(2u, GpuBindingModel.UniformTerrainTiling);
}
[Fact]
public void UnassignedTextureSlotIsNeverAValidIndex()
{