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.
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
Bevel shape outline beyond threshold #2741
base: master
Are you sure you want to change the base?
Bevel shape outline beyond threshold #2741
Changes from all commits
6886d16
0b9c80d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation could need a bit more explanation on what "miter" is. If I don't know what "miter" is, the documentation essentially explains things by self reference as one also wouldn't understand what "miter length" is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to make it clearer adding documentation to
setMiterLimit
method. Feedback is welcome.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to challenge the "Miter" name usage. I believe it makes on a "domain" level a lot of sense to call it that, but as a simple users that wants to draw shapes, it's not easy to understand what "miter" or "miter limit" means.
Just searching the term "miter" leads to the "tall headdress of a bishop" or "joint made between two pieces of wood". You really have to search for "miter limit" or similar until you start getting documentation from SVG/canvas.
I don't have a better name ready just yet, and am also open to using "miter limit", but maybe we can brainstorm a bit, if there's something that describes it in a way that doesn't necessarily require a documentation study or web search.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR name uses the word
bevel
which I think is a good term to describe what's happening. This parameter controls how far the outline corner can stick out before it starts being beveled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe "miter limit" it is the correct name because it is already used by SVG as well as cairo library.
Edit: another example in Win32 API. And also Java.
It would be more confusing to come up with some other name. I agree the meaning is not obvious and it deserves more documentation. I will work on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that it's used like that within the domain of SVG/graphics, but we shouldn't just stop there and do as everyone else does. The term is not intuitively understood and as such it deserves a bit of a brainstorming in my opinion.
What speaks against bevel? Or bevel cutoff/limit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked a bit on social media, to see what people would more intuitively call this.
https://swiss.social/@darkcisum/111391908664038204
https://twitter.com/DarkCisum/status/1723282675878789343
One interesting thought that came out of these discussions is that this could in theory be generalized further and allow for different "styling" of the corners. I wouldn't to do this right now of course, but it might influence the thinking of naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify my point, I am not saying the term "miter limit" is used by other libraries because it is the right choice, but rather that it is the right choice because it is used by other libraries.
It seems there is not an obvious intuitive choice. Not choosing "miter limit" would be inconsistent with other libraries on top of not being intuitive and as a consequence would not bring related results in search engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally fine the "miter limit" terminology.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we sometimes expect
p1
andp2
to be identical? What does it mean if they're identical?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is probably not very meaningful to have overlapping points, but it is a possible state.
If two consecutive points are identical, their (direction, normal) pair would be a pair of zero vectors. Again not very meaningful but the following computations will not explode.