Sunlight artifacts, and a double door's open leaf - #224
Merged
Conversation
A gradient's transform scales its semi-axes, and the beam's falloff was being handed the gap's whole width for the across one. So the ellipse ran nearly twice as wide as the polygon carrying it: at the flank the light was still at about half strength, and the polygon's straight edge cut it off there. That is the hard diagonal boundary in #206 — not an artifact of two beams meeting or of the reach running out, just the outline slicing through light the falloff had not finished with. Measured on the plan from the report: a 160-wide window, ellipse semi-axis 142.8 across, polygon flank at 75.2 — the cut landing at 0.53 of the ellipse's radius, where it is still ~47% opaque. Halving it puts the flanks where the comment always said they were: the light dies just inside the gap's own edges, so the patch has soft sides as well as a soft tip, and nothing but the falloff bounds it. The existing "runs its outline past the falloff" test only checked the two far corners, which is why this survived. It now checks all four. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`wallsLightPassesThrough` cut a gap of the right size in the wrong place: `length * fraction`, always centred. For a double door with a sensor per leaf that is off by a quarter of the opening — one leaf open clears its own half, and the card lit half of the leaf that was still shut and only half of the one that was open (#219). So the callback may now answer with a `[start, end]` span instead of a bare width. A number still centres, which is what a caller with no better answer wants and what keeps every existing call — sunlight included — rendering exactly as it did. `openingClearSpan` works out where, and by construction agrees with `openingClearFraction` about how much; a test holds the two together, since a span wider or narrower than the fraction would leak light or lose it. Only the hinged double moves. A slider's centred gap is just as approximate — a single panel really clears the side it slid away from — but those want more than one interval per opening, which this function's shape still cannot say, and moving them would change every existing plan's lighting for a case nobody has reported. Placement has to be projected onto the wall, not read off the opening: a wall drawn right-to-left runs backwards under the same doorway, and taken as given the span's ends cross over and the gap is dropped entirely — a door that stops passing light because of the direction its wall was drawn. There is a test for that; it caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fixes are narrowly scoped, maintain backward-compatible behavior for existing callers, and are protected by new regression tests covering both reported issues and key edge cases.
Pull request overview
This PR fixes two rendering/geometry bugs in the lighting system: (1) a sunlight falloff ellipse that was sized using the beam’s full width instead of its semi-axis width (causing hard diagonal clipping artifacts), and (2) wall “gaps” for light transmission that were always centered, which misplaces light passing through a hinged double door when only one leaf is open.
Changes:
- Correct sunlight falloff sizing by using half the beam width when computing the ellipse “across” semi-axis, preventing polygon edges from clipping non-zero falloff (#206).
- Extend
wallsLightPassesThroughto accept either a centered width (number) or an explicit[start, end]span, enabling correct placement for per-leaf double doors (#219). - Add targeted tests and documentation updates to lock in the corrected invariants and behavior.
File summaries
| File | Description |
|---|---|
| src/render.ts | Adds span-based opening placement (openingClearSpan, glowClearSpan), updates wall-gap cutting to respect spans, and fixes sunlight falloff semi-axis scaling. |
| src/render.test.ts | Adds unit tests for span placement invariants and wall-gap cutting behavior for double doors and reversed wall direction. |
| src/render.opening.test.ts | Strengthens sunlight regression tests to ensure the beam polygon never clips non-zero ellipse falloff and validates semi-axis sizing. |
| src/floorplan-card.ts | Switches lamp/wall clearing logic from fraction-only to span-aware (glowClearSpan) for correct double-door behavior. |
| src/editor.ts | Mirrors the card’s span-aware wall clearing logic in the editor preview. |
| README.md | Documents that lamp pools follow per-leaf behavior for hinged doubles (gap placement matches the open leaf). |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Closes #206, closes #219.
Two separate bugs in the light geometry, one commit each.
#206 — the hard diagonal edge on a sun patch
A gradient's
gradientTransformscales its semi-axes. The beam's falloff was being handed the gap's whole width for the across one, so the ellipse ran nearly twice as wide as the polygon carrying it. At the flank the light was still at about half strength and the polygon's straight edge cut it off there — the hard boundary circled in the issue.Measured on a plan like the report's, a 160-wide window:
Halving it puts the flanks where the constant's own comment always said they were: the light dies just inside the gap's own edges, so the patch has soft sides as well as a soft tip and nothing but the falloff bounds it.
It survived because the existing "runs its outline past the falloff" test only checked the two far corners. It now checks all four, which is the invariant that matters: the polygon may only clip the ellipse where the ellipse has already reached zero.
#219 — light through a double door with one leaf open
wallsLightPassesThroughcut a gap of the right size in the wrong place —length * fraction, always centred. For a double door with a sensor per leaf that is off by a quarter of the opening: with the left leaf open the card lit the middle, which is half of the leaf still shut and half of the one that is open.The callback may now answer with a
[start, end]span instead of a bare width. A number still centres, so every existing call — sunlight included — renders exactly as it did.On a 40-wide double door centred at 500, first leaf open:
0..490,510..10000..480,500..1000openingClearSpanworks out where, and by construction agrees withopeningClearFractionabout how much — there's a test holding the two together, since a span wider or narrower than the fraction would leak light or lose it.Only the hinged double moves. A slider's centred gap is just as approximate, but those want more than one interval per opening, which this function's shape still cannot say, and moving them would change every existing plan's lighting for a case nobody has reported.
Writing the placement turned up one bug the tests caught: a wall drawn right-to-left runs backwards under the same doorway, so a span taken as given has its ends cross over and the gap is dropped entirely — a door that stops passing light because of the direction its wall was drawn. Projected and ordered now, with a test.
The other half of #219
The reporter also asked for
glazedto work for a lamp and not only the sun. That already landed in #215, so this PR leaves it alone — a glazed door passes a lamp's pool onmaintoday. Worth saying in the issue when it closes.Verification
tsc --noEmitclean. Both fixes fail their new tests when the source change is stashed — I checked, with the exact numbers above.Sunlight still centres its own beam within the opening, so a partly open double door in direct sun keeps today's approximation. Moving that means moving the beam and the wall gap together, and the beam ids are position-stable per opening for a reason — a separate change, not a drive-by.
🤖 Generated with Claude Code