-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extend parsing of SVG path definitions #98
Draft
respatialized
wants to merge
13
commits into
thi-ng:feature/no-org
Choose a base branch
from
respatialized:svg-path-issue
base: feature/no-org
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Extend parsing of SVG path definitions #98
respatialized
wants to merge
13
commits into
thi-ng:feature/no-org
from
respatialized:svg-path-issue
Conversation
This file contains 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
The W3C triangle example parses correctly according to these tests.
Quoting from the section of the spec defining the grammar: "...in the string "M 100-200" ... the first coordinate will be "100" and the second coordinate will be "-200". "Similarly, for the string "M 0.6.5"... the first coordinate will be "0.6" and the second coordinate will be ".5".
This is a necessary requirement for the horizontal and vertical move commands.
One unresolved question: should move-only commands parse into Vec2 geometries or return nil?
The geometry tests still indicate that the parsed segments do not yet have a working geometric implementation.
Plus, align segment definition with terminology used elsewhere in the library.
This enables consistent sampling behavior on paths that consist only of a single move command
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.
This is a very much work-in-progress PR that extends SVG path string parsing to support the full range of expected commands (as discussed in #97).
Status
It's not ready to merge yet. I'm creating a PR to open a discussion on what should and shouldn't be included in the scope of this change to the library. Parsing SVG path strings into collections of segments is relatively complete and has a fair number of tests. However, performing useful geometric operations with those segments will require more work. Here are some of the areas where more work needs to be considered (and potentially separated out into additional issues with their own PRs).
Arc geometries
These are the failing tests in this PR. This library currently has no
Arc2
geometric type that could be used to implement sampling and other geometric operations along path segments consisting of elliptical arc commands.Multimethods vs Protocols
I adapted the implementation of sampling a path's segments from a now-obsolete multimethod defined in
org/src/types/path.org
. A comment there indicates a potential change that should be made: using protocols for more of the behavior of Path2 objects and their constituent segments.Next steps
I could try to get this PR ready for merging while leaving the actual geometry operations unimplemented, leaving implementations for future work. This would make the
Path2
geometry like other geometries in the library -Ellipse2
, for example:I will continue gathering to-dos and issues with this implementation here and try to figure out what does and doesn't make sense to include in this PR. I welcome suggestions and feedback on scope and the changes I've made so far.