Add cell shape variants (9 selectable meshes with thumbnail previews)#9
Merged
Conversation
Adds the PLAN-cell-shapes.md spec at the repo root (matching the existing PLAN-*.md convention) and a step-by-step implementation plan under docs/superpowers/plans/. The feature generalizes the existing beveled-cube toggle into a CellShape enum covering nine 3D primitives (cube, beveled cube, tetrahedron, octahedron, square pyramid, icosahedron, dodecahedron, sphere, capsule) selected via an ImGui combo with mesh thumbnails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Document MeshBuilder.AddVertex's pass-through (no winding correction) - Use named tuple fields (.X/.Y/.Z) instead of .Item1/.Item2/.Item3 in the cross-product math for easier auditing - Note in IInstancedMesh's doc that the existing mesh classes are adapted to this interface in the next commit Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ia MeshBuilder CubeMesh becomes an IInstancedMesh implementer (one-word change — it already exposes the right surface). BeveledCubeMesh adopts the same interface and delegates its geometry helpers to the shared static MeshBuilder, removing the now-duplicated private AddQuad / AddTriangle / AddVertex methods. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The forward-reference comment described work that has now landed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces RenderSettings.UseBeveledCubes with a CellShape enum. Renames InstancedCubeRenderer to InstancedCellRenderer and uses a shape->mesh dictionary instead of two fixed fields. SessionManager keeps the legacy UseBeveledCubes nullable for one release as a load-time fallback. The UI checkbox is replaced with a 2-option text combo (Cube, Rounded Cube). Subsequent tasks add the remaining seven shapes and thumbnails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- SessionManager now uses Enum.IsDefined instead of clamping the persisted shape value. An unknown value (e.g. saved by a newer build) falls back to the renderer default explicitly rather than silently saturating to the highest known enum member. Removes the need to widen a magic-number upper bound each time a shape is added. - ImGuiUI clamps the loaded _shape index into the ShapeNames range so a future-shape session doesn't push ImGui.Combo into an out-of-range state. Matches the defensive pattern used for FloorMode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces internal-static UploadAndBind helper on OctahedronMesh that the remaining mesh classes (pyramid, icosahedron, dodecahedron, icosphere, capsule) reuse to avoid duplicating GL setup boilerplate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Internal-static BaseVertices and BaseFaces tables are reused by the upcoming Dodecahedron (built as the icosahedron's dual) and Icosphere (subdivided icosahedron) meshes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Built as the dual of the icosahedron: each pentagon connects the centroids of the 5 icosa faces meeting at one icosa vertex. The dodec vertices and connectivity are derived from IcosahedronMesh's verified BaseVertices and BaseFaces, then uniformly rescaled to fit the unit cell footprint (±0.5). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One subdivision of the icosahedron from Task 7 yields 80 triangles with smooth radial normals. Midpoint deduplication via cache so shared edges between adjacent faces don't get duplicated vertices. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Y-axis capsule: 8 segments × 3 latitude rings per hemisphere cap. The strip between the top cap's equator ring and the bottom cap's equator ring forms the cylindrical body section — both equator rings have the same radius, so the band is a true uniform-radius cylinder. Smooth normals: each vertex's normal points radially out from its nearest cap center; at the equator rings that direction is purely in the XZ plane, matching the cylinder body's normal, so shading is continuous across the cap/body joint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renders each registered mesh once into a 32x32 RGBA texture at startup using a minimal dedicated shader pair (thumb.vert/thumb.frag). The cell shape combo now uses BeginCombo/Selectable rows so each entry shows the rendered thumbnail next to the shape name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Save the prior GL_DEPTH_TEST enable bit alongside viewport and FBO binding, and restore it after rendering the thumbnails. Without this, calling Render outside the current single-call-at-startup contract could silently flip a caller's depth-test state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Move the shared VAO/VBO/EBO upload helper from OctahedronMesh to MeshBuilder, where it belongs alongside AddQuad/AddTriangle. All call sites updated. - Refactor TetrahedronMesh to call the shared helper instead of inlining the same 22 lines of GL setup. Drops 'unsafe' from Generate(). - Document the IcosahedronMesh.BaseFaces ordering contract — the array is consumed by IcosphereMesh and DodecahedronMesh and must not be reordered. - Note ShapeThumbnailRenderer.Render's startup-only contract: it does not save/restore the active shader or the bound texture unit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds selectable cell shape rendering to the 3D Game of Life renderer, replacing the beveled-cube toggle with a persisted CellShape setting and thumbnail-backed ImGui picker.
Changes:
- Introduces
CellShape,IInstancedMesh, shared mesh helpers, and seven new mesh implementations. - Refactors instanced rendering to select meshes dynamically while preserving beveled-cube LOD fallback.
- Adds thumbnail rendering shaders/FBO and session persistence compatibility for legacy
UseBeveledCubes.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/GameOfLife3D.NET/UI/ImGuiUI.cs |
Replaces rounded-cube checkbox with shape combo using thumbnails. |
src/GameOfLife3D.NET/Shaders/thumb.vert |
Adds thumbnail vertex shader. |
src/GameOfLife3D.NET/Shaders/thumb.frag |
Adds thumbnail fragment shader. |
src/GameOfLife3D.NET/Rendering/ShapeThumbnailRenderer.cs |
Renders per-shape offscreen thumbnail textures. |
src/GameOfLife3D.NET/Rendering/RenderSettings.cs |
Replaces beveled-cube boolean with CellShape. |
src/GameOfLife3D.NET/Rendering/Renderer3D.cs |
Initializes shape renderer and thumbnail renderer. |
src/GameOfLife3D.NET/Rendering/Meshes/TetrahedronMesh.cs |
Adds tetrahedron mesh. |
src/GameOfLife3D.NET/Rendering/Meshes/SquarePyramidMesh.cs |
Adds square pyramid mesh. |
src/GameOfLife3D.NET/Rendering/Meshes/OctahedronMesh.cs |
Adds octahedron mesh. |
src/GameOfLife3D.NET/Rendering/Meshes/IcosphereMesh.cs |
Adds sphere/icosphere mesh. |
src/GameOfLife3D.NET/Rendering/Meshes/IcosahedronMesh.cs |
Adds icosahedron mesh and shared base geometry. |
src/GameOfLife3D.NET/Rendering/Meshes/DodecahedronMesh.cs |
Adds dodecahedron mesh from icosahedron dual. |
src/GameOfLife3D.NET/Rendering/Meshes/CapsuleMesh.cs |
Adds capsule mesh. |
src/GameOfLife3D.NET/Rendering/MeshBuilder.cs |
Adds shared mesh construction/upload helpers. |
src/GameOfLife3D.NET/Rendering/InstancedCellRenderer.cs |
Refactors instanced renderer to select registered meshes. |
src/GameOfLife3D.NET/Rendering/IInstancedMesh.cs |
Adds common mesh interface. |
src/GameOfLife3D.NET/Rendering/CubeMesh.cs |
Makes cube mesh implement IInstancedMesh. |
src/GameOfLife3D.NET/Rendering/CellShape.cs |
Adds persisted shape enum. |
src/GameOfLife3D.NET/Rendering/BeveledCubeMesh.cs |
Makes beveled cube mesh implement IInstancedMesh and use shared helpers. |
src/GameOfLife3D.NET/IO/SessionManager.cs |
Persists Shape and supports legacy beveled-cube fallback. |
PLAN-cell-shapes.md |
Adds high-level design plan. |
docs/superpowers/plans/2026-05-15-cell-shapes.md |
Adds detailed implementation plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,123 @@ | |||
| # PLAN — Cell Shape Variants | |||
|
|
|||
| **Status:** design approved, not yet implemented | |||
| // UseBeveledCubes is removed. | ||
| ``` | ||
|
|
||
| `CellShape` enum members: `Cube, BeveledCube, Octahedron, Tetrahedron, SquarePyramid, Dodecahedron, Icosahedron, Sphere, Capsule`. |
Comment on lines
+80
to
+115
| foreach (var kv in meshes) | ||
| { | ||
| uint tex = _gl.GenTexture(); | ||
| _gl.BindTexture(TextureTarget.Texture2D, tex); | ||
| _gl.TexImage2D(TextureTarget.Texture2D, 0, (int)InternalFormat.Rgba8, | ||
| Size, Size, 0, PixelFormat.Rgba, PixelType.UnsignedByte, null); | ||
| _gl.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureMinFilter, | ||
| (int)TextureMinFilter.Linear); | ||
| _gl.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureMagFilter, | ||
| (int)TextureMagFilter.Linear); | ||
| _gl.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureWrapS, | ||
| (int)TextureWrapMode.ClampToEdge); | ||
| _gl.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureWrapT, | ||
| (int)TextureWrapMode.ClampToEdge); | ||
|
|
||
| _gl.FramebufferTexture2D(FramebufferTarget.Framebuffer, | ||
| FramebufferAttachment.ColorAttachment0, TextureTarget.Texture2D, tex, 0); | ||
|
|
||
| _gl.ClearColor(0.10f, 0.12f, 0.16f, 1f); // dark slate | ||
| _gl.Clear(ClearBufferMask.ColorBufferBit | ClearBufferMask.DepthBufferBit); | ||
|
|
||
| _gl.BindVertexArray(kv.Value.Vao); | ||
| _gl.DrawElements(PrimitiveType.Triangles, kv.Value.IndexCount, | ||
| DrawElementsType.UnsignedInt, null); | ||
|
|
||
| _textures[kv.Key] = tex; | ||
| } | ||
|
|
||
| _gl.BindVertexArray(0); | ||
| _gl.BindFramebuffer(FramebufferTarget.Framebuffer, (uint)prevFbo); | ||
| _gl.Viewport(prevViewport[0], prevViewport[1], | ||
| (uint)prevViewport[2], (uint)prevViewport[3]); | ||
| if (!prevDepthTest) _gl.Disable(EnableCap.DepthTest); | ||
|
|
||
| _gl.DeleteFramebuffer(fbo); | ||
| _gl.DeleteRenderbuffer(depthRbo); |
- PLAN-cell-shapes.md: mark Status as implemented (#9) since the implementation now ships in this PR. - PLAN-cell-shapes.md: fix the documented enum order to match the actual CellShape values (Tetrahedron=2 before Octahedron=3, Icosahedron=5 before Dodecahedron=6). The line documents persisted JSON ordering, so it must agree with the code. - ShapeThumbnailRenderer: add a one-time CheckFramebufferStatus call after the first color attachment, matching the diagnostic pattern already used in PostProcessPipeline and ReflectiveFloorRenderer. Logs to stderr on incomplete; the FBO structure is identical across iterations so one check covers every shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UseBeveledCubesboolean toggle into aCellShapeenum threaded throughRenderSettings, the renamedInstancedCellRenderer,SessionManager, andImGuiUI.Rendering/Meshes/and share their GL setup viaMeshBuilder.UploadAndBind. The dodecahedron is built as the dual of the icosahedron (face centroids + angular sort) for provable geometric correctness.Design docs
PLAN-cell-shapes.md— specdocs/superpowers/plans/2026-05-15-cell-shapes.md— implementation planTest plan
dotnet build— 0 errors, 0 warningsUseBeveledCubesboolean) falls back toCube/BeveledCubeas appropriate — please verify with an old session fileBackward compatibility
RenderSessionData.UseBeveledCubes(nowbool?) is kept for one release as a legacy load-time fallback. New sessions only writeShape. Can be removed in a follow-up PR.Enum.IsDefined, so a session saved by a future build with an unknown shape number falls back toCellShape.BeveledCube(the renderer default) instead of failing.Out of scope (deliberately deferred)
🤖 Generated with Claude Code