Skip to content

fix(accessibility): add bezier and spline to accessible output - #9258

Open
Kunal241207 wants to merge 4 commits into
processing:mainfrom
Kunal241207:fix/accessibility-bezier-spline
Open

Kunal241207 wants to merge 4 commits into
processing:mainfrom
Kunal241207:fix/accessibility-bezier-spline

Conversation

@Kunal241207

Copy link
Copy Markdown

Resolves #9236

Changes:

  • Add accessibility output support for bezier() and spline().
  • Calculate approximate position and area for curves in textOutput() and gridOutput().
  • Respect spline ends and tightness properties when generating accessibility output.
  • Use the fill color and calculated area for filled curves, and stroke color with zero area for stroked curves.
  • Calculate the centroid of the filled curve region for a more accurate position.
  • Add unit tests covering bezier/spline output, spline ends and tightness, and noFill() behavior.

Screenshots of the change:
Not applicable — this change affects generated accessibility output rather than the visual canvas.

PR Checklist

@p5-bot

p5-bot Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

@perminder-17 perminder-17 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, @Kunal241207! I had a look at the diff and it seems quite large (550+ lines) for the scope of #9236. I'd have expected mostly a call to _accsOutput() from bezier()/spline()?

Could you help me understand what's driving the size?

If there's a way to make it smaller?

@bhabishnu

Copy link
Copy Markdown
Contributor

Hey @Kunal241207, I found a few things while testing the current branch that seem worth looking at.

  • splineProperty('ends') isn't reflected correctly. INCLUDE and EXCLUDE give the same accessibility output for the same spline even though the rendered shapes are very different. There are two parts to it: ends is compared to the string 'INCLUDE' even though it's a Symbol, and the sampled control points don't match the renderer. Fixing only the first still leaves EXCLUDE wrong.

  • Filled bezier area changes when the same curve is moved. One case reports 7%, then 14% after shifting the same curve 50px right, even though the painted area is unchanged. It looks like the sampled polygon isn't being closed before the shoelace calculation.

  • I also found a fully visible filled spline whose calculated centroid ends up off canvas. With gridOutput() that reaches an older negative-index bug and stops the sketch. The grid bug itself predates this PR.

On the size question, quite a bit of the new sampling logic seems to overlap with existing bezierPoint() / splinePoint() and spline machinery, so I think there may be room to simplify this quite a bit.

Would be interested in your take on that too, @perminder-17.

@Kunal241207

Copy link
Copy Markdown
Author

Thanks for the reviews and great catches @perminder-17 @bhabishnu!

Most of the original diff was test boilerplate and custom math. I've simplified it quite a bit:

  • Deduplication & size: Dropped the custom Bernstein and Catmull-Rom conversion logic in favor of existing this.bezierPoint() and this.splinePoint(), and trimmed duplicate test cases.
  • splineProperty('ends'): Now compared against constants.EXCLUDE, and samples just the P1 → P2 span to match the renderer.
  • Shoelace closure: Closed the polygon loop in _polygonArea() so translated curves keep their expected area.
  • Centroid & grid crash: Switched curve centers to the average of sampled points to prevent self-crossing curves from flying off-canvas, and added >= 0 guards in gridOutput.js so negative coordinates don't break grid indexing.

All tests and lint checks are passing now!

@ksen0 ksen0 added this to the 2.4 milestone Oct 20, 2026
@ksen0 ksen0 modified the milestones: 2.4, 2.5 Nov 4, 2026
@ksen0 ksen0 moved this to In Progress in p5.js 2.x 🌱🌳 Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Add screen reader output support for bezier() and spline() curves

4 participants