feat(glyphs): add directional lighting to TexturedGlobeGlyph - #320
Merged
Conversation
Add a sun unit vector and ambient floor so the globe can be lit from a direction, shading a lambertian day/night terminator instead of reading as evenly illuminated. sun=None (the default) renders byte-identical to 0.33.0. - Accept sun/ambient on __init__ and override per call on draw/animate (mirroring spin); sun is normalized to unit length, ambient must be in [0, 1]. - Apply lighting per frame from the already-rotated vertices (one dot product over the _spun_mesh output, whose unit-sphere positions are the surface normals), scaling a copy of the cached facecolors by ambient + (1 - ambient) * clip(dot(normal, sun), 0, 1). The facecolors cache is never mutated and the texture is never re-sampled, so the sample-once/rotate-per-frame contract holds and a fixed sun sweeps the terminator across the surface as the globe spins. - Add lighting tests and a docs example. Closes #319
Address round-1 review findings on the new lighting path: - animate() validates sun/ambient eagerly at the call (matching draw) instead of deferring to matplotlib's per-frame render loop, and its Raises docstring now lists sun/ambient (M1). - Type the _INHERIT sentinel as Any and assert the facecolors cache is populated in _lit_facecolors, clearing the 7 new mypy errors the feature introduced (M2). - _normalize_sun rejects non-1-D inputs so a (1,3)/(3,1) array no longer slips through, matching the documented length-3 vector contract (L1). - Clarify ambient is always validated but only affects a lit render (N1).
Add round-1 regression/edge tests: non-1-D sun rejection, NaN ambient, ambient 0 (black night) and 1 (equals cache), animate's eager sun/ambient validation, and world-space sun honoured under tilt.
…y-space Assert the geographic north cap is dimmer than the brightest face under a +z sun with a 45deg tilt (north < peak - 0.1), which a body-space regression -- where the pole would be fully lit -- would fail. The prior north > south check passed for both frames.
sun=(1,0,0.3) is nearly parallel to the default camera eye, so it renders a mostly-lit disc, not the left/right split the comment described. Use sun=(0,1,0.3) (perpendicular to the view) for a real side-lit terminator.
Replace the two == 0.0 float-equality guards SonarCloud flags as bugs: the sun zero-vector check uses <= 0.0, and the per-face normal-magnitude guard floors the magnitude with np.maximum(..., 1e-12) instead of an == mask (same defensive intent, no float equality, and it doubles as a cleaner div-by-zero floor).
|
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
1 participant
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.
Description
Adds directional lighting to
TexturedGlobeGlyphso the sphere can be lit from a direction —shading a lambertian day/night terminator instead of reading as evenly illuminated. This is the
follow-up #311/0.33.0 did not cover, found while adopting the glyph downstream in earthlens (the
eclipse-geometry notebook had to keep a hand-rolled
textured_earthbecause the terminator is itsactual subject).
sun(a world-space(x, y, z)direction, auto-normalised to unit length) andambient(floor so the night side stays legible, default
0.13) on__init__, and overridable per callon
draw/animate— mirroring howspinworks, via a sentinel so an explicitsun=Nonecan turnlighting off for one call.
vertices
drawalready rotates (_spun_mesh(spin), whose unit-sphere positions are the surfacenormals): a copy of the cached
facecolorsis scaled byambient + (1 - ambient)·clip(dot(normal, sun), 0, 1). Thefacecolorscache is never mutated andthe texture is never re-sampled — so a fixed
sunwith a spinning globe sweeps the terminator acrossthe surface.
sun=None(the default) is byte-identical to 0.33.0 — every current caller is unchanged.facecolorsare per-face while mesh normals are per-vertex, so each quad'sfour corner normals are averaged to line the lit factor up with the faces.
Frame:
sunis in the same world frame the globe is drawn in —+zup/north,+xtoward theviewer at
spin=0. Scope: small, backward-compatible enhancement to the already-shipped globeglyph — NumPy in → matplotlib out, no new dependency (a dot product). No specular/atmosphere/cast
shadows (out of scope, per the issue).
No new runtime dependencies.
Issues
Type of change
Check relevant points.
How Has This Been Tested?
New
TestLightingcases intests/test_textured_globe_glyph.py(60 tests total; 100% line + branchcoverage of the module) covering the issue's Definition of Done:
sun=Nonebyte-identical — returns the cachedfacecolorsby identity (unlit regression).equals
ambient × cache(night side not black).spin— a fixed sun with the globe at spin 0 vs 180 gives different facecolors.facecolorscache is unchanged after a litdraw.sunon__init__and per-call override;draw(sun=None)disables an instance light;sunauto-normalised;
animate(sun=...)forwards per frame (rendered via the Pillow writer).sun(wrong length / zero / non-finite) andambientoutside[0, 1]raise.Reproduce (external uv env, worktree on
src):tests/test_textured_globe_glyph.py(60 passed, 100% coverage)pytest tests/ -q→ 2532 passed;ruff check+ruff format --checkclean;module doctests pass
Checklist: