erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(render): Phase U.3 — scope gl_ClipDistance enable to world-geometry draws

Browse source 
Code review caught a portability hazard: GL_CLIP_DISTANCE0..7 was enabled globally
at init, but sky/particle/ui/debug vertex shaders don't write gl_ClipDistance —
undefined behavior that could clip them away on some drivers (benign on the dev
driver, which is why the offline check passed). Bracket the enable/disable around
only the terrain+entity (mesh_modern/terrain_modern) draws; sky/particles/UI/debug
render with clipping off. U.4's EnvCellRenderer.Render belongs inside the bracket.
Also: ClipFrame is long-lived (??= NoClip()), so Dispose now deletes its GL buffers;
fix the stale per-frame-transient comments.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-30 17:42:27 +02:00
parent bf2e559369
commit 864fc5f94e
2 changed files with 54 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

22
src/AcDream.App/Rendering/ClipFrame.cs
View file

@ -85,6 +85,11 @@ public sealed class ClipFrame : IDisposable
private bool _glInitialized;
private bool _disposed;
// GL reference captured on the first UploadShared so Dispose can delete the two
// buffers. ClipFrame is long-lived in U.3 (GameWindow holds one via ??= NoClip()
// and reuses it every frame), so we DO own buffer teardown — see Dispose.
private GL? _gl;
private ClipFrame(byte[] regionBytes, int slotCount)
{
_regionBytes = regionBytes;
@ -197,6 +202,7 @@ public sealed class ClipFrame : IDisposable
if (!_glInitialized)
{
_gl = gl; // captured for Dispose (single context for the frame's lifetime)
_regionSsbo = gl.GenBuffer();
_terrainUbo = gl.GenBuffer();
_glInitialized = true;
@ -224,11 +230,17 @@ public sealed class ClipFrame : IDisposable
{
if (_disposed) return;
_disposed = true;
// GL buffers are deleted by the owner's GL context teardown; ClipFrame
// is a per-frame transient in U.3 (NoClip() each frame). We do not hold a
// GL handle to delete here because UploadShared may not have run. If a
// future phase makes ClipFrame long-lived, add buffer deletion guarded by
// _glInitialized + a captured GL reference.
// ClipFrame is long-lived in U.3 (GameWindow holds one via ??= NoClip() and
// reuses it every frame), so we own the two GL buffers and delete them here.
// _glInitialized guards the case where UploadShared never ran (no buffers to
// delete, and _gl was never captured).
if (_glInitialized && _gl is not null)
{
_gl.DeleteBuffer(_regionSsbo);
_gl.DeleteBuffer(_terrainUbo);
_regionSsbo = 0;
_terrainUbo = 0;
}
}
// ---- byte helpers (little-endian; matches x86/x64 GPU upload) ------------

52
src/AcDream.App/Rendering/GameWindow.cs
View file

@ -167,11 +167,12 @@ public sealed class GameWindow : IDisposable
private AcDream.App.Rendering.Wb.WbFrustum? _envCellFrustum;
// Phase U.3: the shared per-frame clip data (binding=2 mesh SSBO + terrain
// UBO). In U.3 this is rebuilt as ClipFrame.NoClip() every frame and uploaded
// once before terrain/entities draw, so the whole scene renders ungated
// (identical to pre-U.3). The buffer ids are handed to the three renderers so
// each re-binds binding=2 immediately before its own draw. U.4 replaces the
// NoClip() frame with one built from the portal-visibility result.
// UBO). In U.3 a single ClipFrame.NoClip() instance is created lazily (??=) and
// REUSED across frames — its GL buffers persist; only the cheap CPU-side no-clip
// state is re-uploaded each frame before terrain/entities draw, so the whole
// scene renders ungated (identical to pre-U.3). The buffer ids are handed to the
// three renderers so each re-binds binding=2 immediately before its own draw.
// U.4 replaces the NoClip() frame with one built from the portal-visibility result.
private ClipFrame? _clipFrame;
/// <summary>
@ -1087,16 +1088,16 @@ public sealed class GameWindow : IDisposable
_gl.ClearColor(0.05f, 0.10f, 0.18f, 1.0f);
_gl.Enable(EnableCap.DepthTest);
// Phase U.3: enable all 8 hardware clip planes once at startup. The mesh
// and terrain vertex shaders write gl_ClipDistance[0..7] every frame;
// unused planes are set to +1.0 (pass-all) when a region's plane count is
// below 8, so leaving all 8 enabled permanently is correct and avoids
// per-draw glEnable/glDisable thrash. In U.3 every region is no-clip, so
// nothing is actually clipped — the running game renders identically. U.4
// populates real clip regions; the enables stay as-is.
// (EnableCap.ClipDistance0 == GL_CLIP_DISTANCE0 0x3000; +i selects plane i.)
for (int _cp = 0; _cp < ClipFrame.MaxPlanes; _cp++)
_gl.Enable(EnableCap.ClipDistance0 + _cp);
// Phase U.3: the 8 hardware clip planes (GL_CLIP_DISTANCE0..7) are NOT
// enabled here. Only the mesh_modern / terrain_modern vertex shaders write
// gl_ClipDistance[0..7]; the sky / particle / weather / UI / debug-line
// shaders write gl_Position but no gl_ClipDistance. Enabling a clip plane
// for a draw whose vertex shader doesn't write that plane's distance is
// UNDEFINED per the GL/GLSL spec (a driver may read the unwritten value as
// negative and clip the primitive away). So instead of a permanent global
// enable, OnRender brackets glEnable/glDisable(GL_CLIP_DISTANCE0..7) around
// ONLY the clip-writing world-geometry draws (terrain + entities, plus
// U.4's EnvCellRenderer.Render); everything else draws with clipping off.
string shadersDir = Path.Combine(AppContext.BaseDirectory, "Rendering", "Shaders");
@ -7295,6 +7296,19 @@ public sealed class GameWindow : IDisposable
_envCellRenderer?.SetClipRegionSsbo(_clipFrame.RegionSsbo);
_terrain?.SetClipUbo(_clipFrame.TerrainUbo);
// Phase U.3: enable the 8 hardware clip planes for the world-geometry
// block ONLY. All gl_ClipDistance-writing draws (terrain, entities, and
// U.4's EnvCellRenderer.Render) MUST be inside this enable/disable
// bracket; everything else (sky, particles, weather, debug, UI) renders
// with clip DISABLED. Sky already drew above (must not be clipped);
// particles/weather/debug/UI draw below the matching glDisable. Scoping
// the enable here (instead of a permanent init-time enable) avoids the
// undefined behavior of leaving GL_CLIP_DISTANCE_i on for shaders that
// never write gl_ClipDistance[i] — a driver is free to clip those away.
// (EnableCap.ClipDistance0 == GL_CLIP_DISTANCE0 0x3000; +i selects plane i.)
for (int _cp = 0; _cp < ClipFrame.MaxPlanes; _cp++)
_gl.Enable(EnableCap.ClipDistance0 + _cp);
// Phase N.5b: wrap Draw in CPU stopwatch for [TERRAIN-DIAG] rollup
// (gated on ACDREAM_WB_DIAG=1, same env var as [WB-DIAG]). Stopwatch
// is cheap; only the periodic Console.WriteLine is gated.
@ -7334,6 +7348,14 @@ public sealed class GameWindow : IDisposable
visibleCellIds: visibility?.VisibleCellIds,
animatedEntityIds: animatedIds);
// Phase U.3: close the world-geometry clip bracket opened above. From
// here down (particles, weather, debug lines, UI) the vertex shaders do
// NOT write gl_ClipDistance, so the planes must be OFF to avoid the
// undefined-behavior clip. U.4's EnvCellRenderer.Render, when added,
// belongs ABOVE this line (it writes gl_ClipDistance like the others).
for (int _cp = 0; _cp < ClipFrame.MaxPlanes; _cp++)
_gl.Disable(EnableCap.ClipDistance0 + _cp);
// Phase G.1 / E.3: draw all live particles after opaque
// scene geometry so alpha blending composites correctly.
// Runs with depth test on (particles occluded by walls)