feat(globe): add area sampling and a face_colors accessor to TexturedGlobeGlyph - #332
Open
MAfarrag wants to merge 6 commits into
Open
feat(globe): add area sampling and a face_colors accessor to TexturedGlobeGlyph#332MAfarrag wants to merge 6 commits into
MAfarrag wants to merge 6 commits into
Conversation
TexturedGlobeGlyph point-sampled one texture cell per mesh-face centre, so a feature narrower than one face fell between the sample points and vanished -- however fine the texture; only a finer mesh brought it back (#325). Two additions, each useful on its own: - sampling="point"|"area". "point" (default) is unchanged: the cheap sample-once lookup, right for photographic basemaps. "area" reduces the texture block each face covers, alpha-aware -- a face's colour is the mean of the opaque cells it spans, or transparent if it spans none -- so a small opaque feature survives. Faces finer than one texture cell fall back to point sampling, so a coarse texture never gains gaps. The reduction runs once in _prepare, keeping the sample-once/rotate-per-frame contract. - face_colors: a read-only property returning the per-face RGBA the globe will paint (before per-frame lighting), so a caller can check whether its data survived the sampling without a draw() or reaching into private state. Closes #325
… mesh-finer fallback Close the review's area-mode test gaps: exercise a 3-channel RGB texture (the common relief() basemap input) -- every face stays opaque and block-averages, differing from centre sampling on a high-frequency pattern -- and a fractional- alpha cell, which contributes and leaves the face partly transparent. Also give test_area_matches_point_when_mesh_finer_than_texture a non-uniform texture so it actually pins that area picks the same cell as point (a uniform fill could not).
…) semantics The area reduction includes every cell with alpha > 0, not only fully-opaque ones, and averages RGB un-premultiplied -- so a face over feathered edges keeps their mean colour and a mean alpha below 1. The docstrings/comments said 'opaque', which reads as fully-opaque. Reword to 'non-transparent (alpha > 0)', spell out the fractional-alpha behaviour, and rename the internal mask from 'opaque' to 'nontransparent' to match. Behaviour is unchanged.
… docstrings Cosmetic Round-2 review cleanups, no behaviour change: collapse the one-argument sampling ValueError onto a single line so the new hunk is ruff-format-canonical (L1); open face_colors with 'The base per-face RGBA colours...' so the headline agrees with the pre-lighting hedge (L2); rewrap the sampling docstring to remove an orphan line (N1); and align its 'fall back to point sampling' clause with the code's precise empty-block condition (N4).
… cells The area reduction averages RGB un-premultiplied: a face over two non-transparent cells of alpha 64 and 192 with distinct colours must reduce to the plain mean (red 0.5, not the alpha-weighted 0.25) and the mean alpha. Pins the documented 'each cell counts once' contract against a future premultiplied-mean regression.
The 'point' string literal appeared three times (default arg, validation tuple, mode comparison), which SonarCloud python:S1192 flags as a duplicated literal. Hoist the modes to module constants SAMPLING_POINT/SAMPLING_AREA/SAMPLING_MODES and reference them, leaving one definition of each. Behaviour unchanged.
|
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
TexturedGlobeGlyphpoint-samples one texture cell per mesh-face centre, which is what makes thesample-once/rotate-per-frame contract cheap. The cost is that a texture feature narrower than one mesh face
falls between the sample points and disappears entirely — however fine the texture is, only a finer mesh brought
it back. That bites geospatial callers who drape a small regional dataset onto a mostly-transparent global canvas
(found while adopting the glyph in Digital-Earth).
Two additions, each useful on its own, keeping the sample-once contract (the reduction runs once in
_prepare):sampling="point" | "area"."point"(default) is unchanged — the cheap per-centre lookup, the rightchoice for a photographic basemap.
"area"reduces the whole texture block each face covers, alpha-aware: aface's colour is the mean of the opaque cells it spans (or transparent if it spans none), so a small opaque
feature stays visible rather than fading toward transparent. Faces finer than a single texture cell fall back to
point sampling, so a coarse texture never gains gaps.
face_colorsproperty. A read-only accessor returning the per-face RGBA the globe will paint (beforeper-frame lighting), so a caller can check whether its data survived the sampling — e.g. that a small feature
landed on at least one face — without a
draw()and without reaching into_facecolors."point"behaviour is byte-identical to before. No new runtime dependencies.Issues
Type of change
Check relevant points.
How Has This Been Tested?
New
TestAreaSampling(8 tests) andTestFaceColors(4 tests) intests/test_textured_globe_glyph.py, coveringthe issue's Definition of Done:
"point"paints 0 faces (the gap),"area"paints it, and the reduced face keeps the opaque colour (alpha-aware, not a faded average)."area"matches"point"exactly(no gaps).
"area"paints nothing.face_colors— correct shape(n_lat-1, n_lon-1, 4), available without adraw(), returns a copy(mutation-safe), and reflects the sampling mode.
samplingvalue raisesValueError.Reproduce (external uv env, worktree
src):tests/test_textured_globe_glyph.py→ 97 passed (86 pre-existing + 11 new)pytest --doctest-modules src/cleopatra/glyphs/globe/textured_globe_glyph.py -q→ 9 passedChecklist: