Skip to content

feat: billboard/cross-model rendering for vegetation - #395

Merged
MichaelFisher1997 merged 2 commits into
devfrom
feature/cross-billboard-vegetation
Apr 1, 2026
Merged

feat: billboard/cross-model rendering for vegetation#395
MichaelFisher1997 merged 2 commits into
devfrom
feature/cross-billboard-vegetation

Conversation

@MichaelFisher1997

Copy link
Copy Markdown
Collaborator

Summary

Closes #373

  • Add RenderShape enum (.cube / .cross) to BlockDefinition in block registry
  • Create cross_mesher.zig module that emits 2 diagonal quads (X-shaped billboard) per cross block with double-sided faces, proper lighting, and biome tint support
  • Skip cross blocks in greedy mesher face mask so they don't generate cube faces
  • Register cross shape for: flower_red, flower_yellow, tall_grass, dead_bush, acacia_sapling, bamboo, torch

Files Changed

File Change
src/world/block_registry.zig Add RenderShape enum, render_shape field, populate per block
src/world/meshing/cross_mesher.zig New — cross-pattern vertex generation with biome tint
src/world/meshing/greedy_mesher.zig Skip cross blocks in face mask building
src/world/chunk_mesh.zig Call cross mesher after greedy mesher per subchunk

Testing

  • zig build — compiles clean
  • zig build test — all unit tests + shader validation pass
  • zig fmt — formatting clean
  • Pre-push hooks passed (format + full test suite)

@github-actions github-actions Bot added documentation Improvements or additions to documentation world labels Apr 1, 2026
@opencode-agent

opencode-agent Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

📋 Summary

Linked Issue: Closes #373

This PR adds billboard/cross-model rendering for vegetation blocks (flowers, tall grass, saplings, etc.) by:

  • Adding a RenderShape enum with .cube and .cross variants to BlockDefinition
  • Creating a new cross_mesher.zig module that generates X-shaped diagonal quads for cross-shaped blocks
  • Modifying the greedy mesher to skip cross blocks during face mask building
  • Registering cross shape for 7 vegetation blocks: flower_red, flower_yellow, tall_grass, dead_bush, acacia_sapling, bamboo, torch

The implementation is well-structured and follows existing code patterns. All testing criteria have been met (builds cleanly, tests pass, formatting clean).

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

⚠️ High Priority Issues (Should Fix)

None identified.

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] src/world/meshing/cross_mesher.zig:47-66 - Redundant biome color sampling per block
Confidence: High
Description: The biome color averaging computes a 3x3 neighborhood (9 samples) for every cross block. Since adjacent blocks in the same chunk will sample overlapping neighborhoods, this results in redundant work. For example, blocks at (x,z) and (x+1,z) share 6 of 9 biome samples but recompute them independently.
Impact: Mesh generation performance degradation, especially in vegetation-heavy chunks with many cross blocks (tall grass, flowers).
Suggested Fix: Consider caching biome colors per XZ column or using a simpler single-biome lookup since cross blocks are typically small and biome boundaries within a 3x3 region rarely cause visible artifacts for small vegetation.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/world/block_registry.zig:327 - Torch registered as cross shape but emits light
Confidence: Medium
Description: Torch is registered as a cross-shaped block, which renders correctly as a vertical pole. However, torches emit light (.{ 15, 11, 6 }). Ensure the cross mesh geometry doesn't cause unexpected lighting artifacts with the point light emission. The current implementation should work fine, but worth verifying visually.
Impact: Potential visual inconsistency if light emission doesn't align with the thin cross geometry.
Suggested Fix: No code change needed, just visual verification that torch lighting looks correct with the cross mesh.

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 Cross mesher has single focus; greedy mesher isn't polluted with cross logic
Open/Closed 8 Easy to add new shapes via RenderShape enum without modifying existing meshers
Liskov Substitution N/A No inheritance hierarchy in use
Interface Segregation 9 Clean separation between meshing stages via function calls
Dependency Inversion 8 Depends on abstractions (TextureAtlas, BlockDefinition)
Average 8.5

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

  • Code Quality: 90% (Clean, well-documented, follows Zig conventions)
  • Completeness: 95% (All requirements from PR description implemented)
  • Risk Level: 75% (New meshing path, but isolated and well-tested)
  • Test Coverage: 85% (Builds and tests pass, though no unit tests for cross mesher specifically)

Merge Readiness:

  • All critical issues resolved
  • SOLID average score >= 6.0
  • Overall confidence >= 60%
  • No security concerns
  • Tests present and passing (if applicable)

Verdict:

MERGE

The implementation is solid, follows project conventions, and successfully adds cross-model rendering for vegetation. The medium-priority performance concern can be addressed in a future optimization PR if profiling shows it's a bottleneck.

New%20session%20-%202026-04-01T17%3A13%3A46.028Z
opencode session  |  github run

Add RenderShape enum (cube/cross) to block definitions. Cross blocks
(flowers, tall grass, saplings, bamboo, dead bush, torch) now render as
2 diagonal quads instead of full cubes, reducing vertex count by ~70%
for vegetation. Cross blocks are skipped in greedy meshing and handled
by a dedicated cross_mesher module with double-sided quads, proper
lighting, and biome tint support.
@MichaelFisher1997
MichaelFisher1997 force-pushed the feature/cross-billboard-vegetation branch from 0360df8 to 7462965 Compare April 1, 2026 17:23
@opencode-agent

opencode-agent Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Tests pass. Now let me provide my comprehensive review:


📋 Summary

Linked Issue: Closes #373

This PR successfully implements billboard/cross-model rendering for vegetation blocks. The implementation:

  • Adds a clean RenderShape enum with .cube and .cross variants
  • Creates a new cross_mesher.zig module that generates X-shaped diagonal quads (2 quads × 2 sides = 12 triangles per block)
  • Properly skips cross blocks in the greedy mesher to avoid duplicate cube faces
  • Registers 7 vegetation blocks with cross shape: flower_red, flower_yellow, tall_grass, dead_bush, acacia_sapling, bamboo, torch

Verification against issue #373: The implementation fully satisfies the requirements for vegetation billboard rendering.

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved.

None identified.

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high priority issues have been resolved.

None identified.

💡 Medium Priority Issues (Nice to Fix)

[FIXED] Previous issue: Redundant biome color sampling per block

Note: The previous review (2026-04-01T17:18:00Z) incorrectly stated there was redundant 3x3 neighborhood sampling at cross_mesher.zig:47-66. Upon review of the actual implementation, the code at lines 49-51 performs a single biome lookup per XZ column (chunk.getBiome(x, z)), which is efficient and appropriate. No redundant sampling exists.

None identified.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/world/meshing/cross_mesher.zig:31 - Unused neighbors parameter
Confidence: Low
Description: The neighbors parameter is captured with _ = neighbors; but never used. While this is acceptable for API consistency with other meshing functions, consider either:

  1. Using it for cross blocks at chunk boundaries (if light/biome sampling needs neighbor data)
  2. Removing the parameter if truly not needed
    Impact: Minimal - code clarity only
    Suggested Fix: Either implement neighbor-aware sampling or simplify the API

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 Cross mesher has single, clear responsibility; greedy mesher modification is minimal
Open/Closed 9 Easy to extend with new render shapes; no modification to core meshing logic needed
Liskov Substitution N/A No inheritance hierarchy
Interface Segregation 9 Clean API: meshCrossBlocks follows same pattern as other meshing functions
Dependency Inversion 8 Depends on abstractions (TextureAtlas, BlockDefinition)
Average 8.8

🎯 Final Assessment

Overall Confidence Score: 92%

Confidence Breakdown:

  • Code Quality: 95% (Clean Zig code, proper memory management with defer, good comments)
  • Completeness: 95% (All requirements from [Batch 1] Billboard/cross-model rendering for vegetation #373 implemented, tests passing)
  • Risk Level: 85% (Low risk - new module isolated from existing code paths)
  • Test Coverage: 85% (Builds and tests pass, shader validation included)

Merge Readiness:

  • All critical issues resolved
  • SOLID average score >= 6.0
  • Overall confidence >= 60%
  • No security concerns
  • Tests present and passing (zig build test passes, exit code 0)

Verdict:

MERGE

The implementation is clean, follows project conventions, and successfully adds cross-model rendering for vegetation. The code is production-ready with excellent separation of concerns between the greedy mesher (cube faces) and cross mesher (billboards). All tests pass including shader validation.

New%20session%20-%202026-04-01T17%3A24%3A08.769Z
opencode session  |  github run

@opencode-agent

opencode-agent Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

📋 Summary

Linked Issue: Closes #373

This PR successfully implements billboard/cross-model rendering for vegetation blocks. The implementation:

  • Adds a clean RenderShape enum with .cube and .cross variants
  • Creates a new cross_mesher.zig module (115 lines) that generates X-shaped diagonal quads
  • Properly skips cross blocks in greedy mesher via b1_cross/b2_cross checks (lines 85-86, 88, 92)
  • Registers 7 vegetation blocks with cross shape: flower_red, flower_yellow, tall_grass, dead_bush, acacia_sapling, bamboo, torch
  • All blocks correctly assigned to .cutout render pass with appropriate is_tintable flags

Verification: All tests pass (exit code 0), including shader validation for 31 GLSL shaders.

🔴 Critical Issues (Must Fix - Blocks Merge)

✅ All previously reported critical issues have been resolved.

None identified.

⚠️ High Priority Issues (Should Fix)

✅ All previously reported high priority issues have been resolved.

None identified.

💡 Medium Priority Issues (Nice to Fix)

None identified.

Note on previous review: The previous concern about redundant biome sampling was confirmed to be a false positive - the code correctly uses single chunk.getBiome(x, z) lookup per XZ column at line 49.

ℹ️ Low Priority Suggestions (Optional)

[LOW] src/world/meshing/cross_mesher.zig:31 - Unused neighbors parameter
Confidence: Low
Description: The neighbors parameter is captured with _ = neighbors; but never used. For API consistency with other meshing functions this is acceptable, but the parameter could be removed if neighbor data is never needed for cross blocks.
Impact: Minimal - code clarity only
Suggested Fix: Either document why neighbors may be needed in the future, or remove the parameter to simplify the API

[LOW] src/world/block_registry.zig:327 - Long line
Confidence: Low
Description: The render_shape switch statement spans a long line. While functional, it could be formatted for better readability.
Impact: Minimal - style only
Suggested Fix: Break into multiple lines if the project style guide prefers it

📊 SOLID Principles Score

Principle Score Notes
Single Responsibility 9 Cross mesher has single focus; greedy mesher modification is minimal (4 lines)
Open/Closed 9 Easy to extend with new render shapes; no modification to core meshing logic needed
Liskov Substitution N/A No inheritance hierarchy
Interface Segregation 9 Clean API: meshCrossBlocks follows same pattern as other meshing functions
Dependency Inversion 8 Depends on abstractions (TextureAtlas, BlockDefinition)
Average 8.8

🎯 Final Assessment

Overall Confidence Score: 94%

Confidence Breakdown:

  • Code Quality: 95% (Clean Zig code, proper memory management with defer/errdefer, good documentation)
  • Completeness: 95% (All requirements from [Batch 1] Billboard/cross-model rendering for vegetation #373 implemented, tests passing)
  • Risk Level: 90% (Very low risk - isolated new module, well-tested integration)
  • Test Coverage: 90% (Builds and tests pass, shader validation included)

Merge Readiness:

  • All critical issues resolved
  • SOLID average score >= 6.0
  • Overall confidence >= 60%
  • No security concerns
  • Tests present and passing (exit code 0)

Verdict:

MERGE

The implementation is clean, follows project conventions, and successfully adds cross-model rendering for vegetation. The code is production-ready with excellent separation of concerns. The only outstanding item is a trivial unused parameter that doesn't affect functionality.

New%20session%20-%202026-04-01T17%3A53%3A00.166Z
opencode session  |  github run

@MichaelFisher1997
MichaelFisher1997 merged commit beec53d into dev Apr 1, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation world

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Batch 1] Billboard/cross-model rendering for vegetation

1 participant