Rebuild ConcaveConvexSlice & BiConvexSlice as single coherent paths (fix artifacts, add pie & full-circle support)#152
Open
postfixNotation wants to merge 3 commits into
Conversation
- significantly simplifying implementation - correct mental model of aforementioned shape - free of any artifacts - shape consists of single coherent path without multiple subpaths as in previous implementation
- Fix rendering artifacts that appeared for certain data configurations, caused by the shape being assembled from multiple subpaths (each implicitly closed on fill) - Build the slice as one continuous, self-closing path from a correct geometric model - Render holeRadius = 0 (pie instead of donut) as a plain sector (wedge) instead of the distorted ring slice the cap geometry produced - Handle the full-circle (360°) sweep explicitly - Significantly simplify the implementation
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.
Summary
Refactors the convex-capped slice shapes used for donut and pie charts —
ConcaveConvexSliceandBiConvexSlice— so that each slice is built as a single, continuousPathinstead of several independent subpaths. This removes rendering artifacts that appeared for a number of data configurations (most visibly onBiConvexSlice), simplifies the implementations, and extends the shapes to cover solid pie charts and full-circle slices.Background / root cause
The previous implementations assembled each slice from multiple
addArc/addRect/addPathsubpaths. When aPathis filled, every subpath is implicitly closed with a straight chord — and with separate subpaths those chords could cut across the slice interior. Certain sweep angles therefore produced wedge- or blob-shaped fill artifacts, and stroking the outline (e.g. via a border) drew spurious internal edges. The effect was most pronounced onBiConvexSlice.Changes
ConcaveConvexSliceclose()— chained witharcTo(forceMoveTo = false)so the entire outline is a single subpath.BiConvexSlicearcTo/addArc(start and end unit vectors coincide, so no arc is drawn). It is now handled explicitly and rendered as a ring (donut) or disc (pie) via the even-odd fill rule. A small tolerance (360f - 0.01f) also catches sweeps that land just below 360° through float accumulation.innerRadius == 0): the convex caps round the ends of a ring and are undefined without a ring thickness, so a holeless slice now falls back to a plain sector (wedge). This makes the shape usable for solid pie charts as well as donut charts.Result
Screenshots
Biconvex slices
Concave convex slices
Testing
Validated with Compose
@Previews covering: two slices (donut and pie), Fibonacci-distributed slices and the "halved" variant (many adjacent slices), a full circle (360°), a single 350° slice, and a 10° + 350° combination (a thin slice next to a near-full slice) — each filled and stroked.