Skip to content

Refactor glow layer management to use exclusion instead of inclusion - #455

Merged
tracygardner merged 1 commit into
mainfrom
claude/fix-glow-rendering-order-GUYnZ
Mar 22, 2026
Merged

Refactor glow layer management to use exclusion instead of inclusion#455
tracygardner merged 1 commit into
mainfrom
claude/fix-glow-rendering-order-GUYnZ

Conversation

@tracygardner

@tracygardnertracygardner commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR refactors the glow layer system to use an exclusion-based approach instead of an inclusion-based approach. The sky mesh is now explicitly excluded from the glow layer, and glow effects are applied to all meshes by default rather than maintaining a list of included meshes.

Key Changes

  • Glow layer initialization: Changed from using addIncludedOnlyMesh() to using addExcludedMesh() for the sky sphere, allowing the glow layer to affect all meshes except the sky by default
  • Glow application: Removed the addIncludedOnlyMesh() call when applying glow to individual meshes, simplifying the glow effect application logic
  • Glow removal: Replaced the check for included meshes with a more robust check that scans all scene meshes to determine if any other meshes still have the glow metadata flag set
  • Sky mesh exclusion: Added consistent exclusion of the sky mesh from the glow layer whenever a sky is created (in all three sky creation methods in scene.js)

Implementation Details

  • The new approach is more maintainable as it doesn't require tracking which meshes are included in the glow layer
  • The glow layer is only disposed when no meshes in the scene have the glow metadata flag set, providing better lifecycle management
  • Sky mesh exclusion is now handled at creation time rather than at glow layer initialization, ensuring the sky is always excluded regardless of initialization order

https://claude.ai/code/session_01XjT3aKdwLYiYfEiYRWwt26

Summary by CodeRabbit

  • Bug Fixes
    • Fixed glow effect rendering to properly exclude the sky from being affected by glowing light effects, ensuring improved visual consistency
    • Optimized the registration and management of glowing effects across multiple mesh elements for better scene performance
    • Improved cleanup logic for glow layer effects to ensure proper resource management in complex scenes

Replace addIncludedOnlyMesh() approach with all-meshes emissive mode to
restore correct depth-based rendering. Exclude the sky sphere explicitly
via addExcludedMesh() to prevent it from being affected by the glow layer.
Also update setSky() to register new sky spheres with any active glow
layer, and fix clearEffects() to scan scene metadata instead of the now-
unused includedOnlyMeshes list when deciding whether to dispose the layer.
https://claude.ai/code/session_01XjT3aKdwLYiYfEiYRWwt26
@coderabbitai

coderabbitaiBot commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a50524ed-9f47-4588-b84d-049cd7b99d34

📥 Commits

Reviewing files that changed from the base of the PR and between 29910b9 and 2fb7fd7.

📒 Files selected for processing (2)
  • api/material.js
  • api/scene.js

📝 Walkthrough

Walkthrough

Modified glow layer management in material and scene modules. The glow layer now excludes the sky mesh during initialization and when setting the sky, and removes per-mesh inclusion registration in favor of scene-wide glow metadata scanning during cleanup.

Changes

Cohort / File(s)Summary
Glow Layer Initialization & Cleanup
api/material.js
Refactored glow layer setup to exclude flock.sky at initialization. Removed per-mesh addIncludedOnlyMesh() calls from glowMesh(). Updated clearEffects() to dispose glow layer based on scene-wide mesh glow metadata scan instead of included-only list length.
Sky Assignment Integration
api/scene.js
Extended setSky() to exclude the sky mesh from the existing glow layer via addExcludedMesh() after sky creation and assignment.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A sky once glowed with misty sheen,
But now it hides from glow's keen gleam—
Excluded bright, the heaven stays,
While meshes dance through glowing rays! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Refactor glow layer management to use exclusion instead of inclusion' directly and clearly summarizes the main architectural change across both modified files.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-glow-rendering-order-GUYnZ

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying flockdev with Cloudflare Pages Cloudflare Pages

Latest commit:2fb7fd7
Status: ✅ Deploy successful!
Preview URL:https://3b7f5c60.flockdev.pages.dev
Branch Preview URL:https://claude-fix-glow-rendering-or.flockdev.pages.dev

View logs

@tracygardner
tracygardner merged commit 514da92 into mainMar 22, 2026
8 checks passed
@tracygardner
tracygardner deleted the claude/fix-glow-rendering-order-GUYnZ branch March 22, 2026 13:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tracygardner@claude