Skip to content

Build plano-convex bar shapes as single coherent paths (fix internal seam when stroked)#153

Open
postfixNotation wants to merge 2 commits into
KoalaPlot:mainfrom
postfixNotation:bugfix/barplotshapes
Open

Build plano-convex bar shapes as single coherent paths (fix internal seam when stroked)#153
postfixNotation wants to merge 2 commits into
KoalaPlot:mainfrom
postfixNotation:bugfix/barplotshapes

Conversation

@postfixNotation

Copy link
Copy Markdown
Contributor

Summary

Builds the plano-convex bar shapes as a single continuous Path instead of several independent subpaths. Filling already rendered correctly, but stroking the outline (via DefaultBar's border) drew the rectangle's interior edge as a stray line across each bar at the body/cap junction. Merging body and cap into one contour removes that internal seam.

Background / root cause

DefaultVerticalPlanoConvexShape, DefaultHorizontalPlanoConvexShape and the index != 0 branches of VerticalPlanoConvexShape / HorizontalPlanoConvexShape assembled their outline from separate subpaths (addRect + addArc, or two addPath unions).

A single Path containing abutting subpaths fills as the correct union in one pass, so the fill output was fine. But Modifier.border(border, shape) strokes every contour of the outline, including the rectangle's top/side edge that sits interior to the shape. The result is a visible line where the body meets the convex cap — clearly seen on stacked bars, where it appears at the junction of every segment.

Changes

  • Merge body and cap into one contour via union (+) in DefaultVerticalPlanoConvexShape, DefaultHorizontalPlanoConvexShape, and the index != 0 branches of VerticalPlanoConvexShape / HorizontalPlanoConvexShape. The union removes the internal edge while leaving the filled area unchanged.
  • Leave the bi-convex shapes unchanged: their closing difference (-) already normalizes the boundary into a clean single contour.
  • index == 0 branches: return the (optionally inverted) path explicitly instead of mutating the wrapped outline in place — a small readability cleanup adjacent to the same code.

Result

  • One coherent contour per shape — no internal subpath seam.
  • Correct rendering for both fill and stroke (border).
  • Fill area and geometry are unchanged; the fix only affects the stroke/border case.

Screenshots

Stacked bars rendered with a border (the case that exposed the seam):

Before After
StackedBarsStroked_Before StackedBarsStroked_After

In Before, a horizontal line cuts across each segment where the body meets the cap. In After, only the outer outline is stroked.

Testing

Visual checks via Compose @Preview for vertical and horizontal plano-convex bars, single and stacked, both filled and with a border. Confirmed the fill output is unchanged and the internal seam is gone under stroking.

Notes for reviewers

  • No new imports: the + (union) and - (difference) operators wrap Path.op(..., PathOperation.…).
  • Allocations are in the same ballpark as before (index != 0 performs two differences + one union per createOutline, vs. two differences + two addPath previously); createOutline is not called per frame.

The plano-convex bar shapes assembled their outline from separate subpaths (raw addRect + addArc, and two addPath unions in the parameterized variants). Filling renders this correctly, but stroking the outline via DefaultBar's border draws the rectangle's interior edge as a stray line across the bar at the body/cap junction.

- Merge body and cap into one contour via union (+) in DefaultVerticalPlanoConvexShape, DefaultHorizontalPlanoConvexShape and the index != 0 branches of VerticalPlanoConvexShape / HorizontalPlanoConvexShape, removing the internal seam when stroked
- Leave the bi-convex shapes unchanged: their closing difference (-) already normalizes the boundary into a clean single contour
- index == 0 branches: return the (optionally inverted) path explicitly instead of mutating the wrapped outline in place
- Fill area and geometry are unchanged; the fix only affects the stroke/border case
Sign up for free to 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.

1 participant