Fix ArrayIndexOutOfBoundsException with compact SVG arc notation#1282
Fix ArrayIndexOutOfBoundsException with compact SVG arc notation#1282hxrshxz wants to merge 4 commits intoprocessing:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an ArrayIndexOutOfBoundsException that occurred when parsing SVG arc commands with compact notation where flags and coordinates are concatenated (e.g., a2 2 0 013 3). The fix adds support for detecting and properly parsing this compact format.
Key changes:
- Added
isCompactArcNotation()helper method to detect concatenated arc flags and coordinates - Modified arc parsing logic for both absolute (
A) and relative (a) commands to handle compact notation - Added comprehensive test coverage for various compact arc notation scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/src/processing/core/PShapeSVG.java | Implements compact arc notation detection and parsing logic |
| core/test/processing/core/PShapeSVGPathTest.java | Adds test cases for compact notation, standard notation, and edge cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@catilac have a look :) |
|
Hi @hxrshxz great work! Thank you! Looking at the tests, the one thing I would say to be missing is a step that actually checks if the parsing occurred correctly by looking at the numbers that the parser spits out, I would say just checking if the shape parses is not enough. |
|
Thanks @Stefterv I have added tests to verify the actual parsed endpoint values, not just that the shape doesn't crash |
catilac
left a comment
catilac
left a comment
There was a problem hiding this comment.
Looks good! I have some questions. Sorry to take so long with this.
| fa = PApplet.parseFloat(token4) != 0; | ||
| fs = PApplet.parseFloat(pathTokens[i + 5]) != 0; | ||
| endX = PApplet.parseFloat(pathTokens[i + 6]); | ||
| endY = PApplet.parseFloat(pathTokens[i + 7]); |
There was a problem hiding this comment.
what is the significance of the 5, 6 and 7 index offsets for pathTokens?S Specifically pathTokens[i+5] etc
There was a problem hiding this comment.
The i + 5, i + 6, and i + 7 offsets correspond to the standard SVG arc command structure which takes 7 arguments: (rx ry x-axis-rotation large-arc-flag sweep-flag x y).
When parsing the A (or a) command at index i:
i + 1: rx
i + 2: ry
i + 3: x-axis-rotation (angle)
i + 4: large-arc-flag (token4)
Therefore, the remaining arguments are at:
i + 5: sweep-flag (fs)
i + 6: x (endX)
i + 7: y (endY)
| (token.charAt(0) == '0' || token.charAt(0) == '1') && | ||
| (token.charAt(1) == '0' || token.charAt(1) == '1') && | ||
| (token.length() == 2 || | ||
| (token.length() > 2 && ( | ||
| Character.isDigit(token.charAt(2)) || | ||
| token.charAt(2) == '+' || | ||
| token.charAt(2) == '-' || | ||
| token.charAt(2) == '.'))); |
There was a problem hiding this comment.
this makes sense but could use some comments to describe what is going on
|
really sorry for taking so long |
|
@hxrshxz this is approved! yay. we are going to do some additional hand testing before it gets merged in. we have an upcoming 4.5 release so i'm being extra careful. but thank you so much! sorry to take so long in accepting this. |
3 participants
Fixes #1244
Summary
Adds support for compact SVG arc notation where flags and coordinates are concatenated (e.g.,
a2 2 0 013 3instead ofa 2 2 0 0 1 3 3). Previously crashed withArrayIndexOutOfBoundsException.Changes
core/src/processing/core/PShapeSVG.java: AddedisCompactArcNotation()helper method to detect and parse concatenated arc flags/coordinates for bothAandacommandscore/test/processing/core/PShapeSVGPathTest.java: Added 4 test cases covering compact notation, standard notation, and edge casesBefore
Screencast.from.2025-10-15.00-31-18.mp4
After
Screencast.from.2025-10-14.23-14-15.mp4