Removed AUTO joint/cap types, and made joint type non-nullable - #5012
LinesThatConnect wants to merge 1 commit into
Conversation
|
I appreciate the honesty with your disclaimer, but don't really feel that we should get a solution not well understood merged, especially if it constitutes a breaking change. I'm quite certain that there is a cleaner solution; the current behavior is likely because the miter type is a class and not a instance variable (or is it?) -- if you are willing to spend some more time figuring this out, then I'm happy to take a closer look at your proposal as well. |
|
Understood! I'm hoping that this will be seen by someone with knowledge of the history that led to the current code. That said, I believe I understand the issue.
ctx.set_line_join(LINE_JOIN_MAP[vmobject.joint_type])
ctx.set_line_cap(CAP_STYLE_MAP[vmobject.cap_style])That is the problem. Cairo does not expect you to pass If my analysis is correct, the current contract is overly loose, defining and defaulting to invalid enum values. Removing these values seems like the most reasonable option. I updated my disclaimer because it really overstated my doubts! |
|
We also discussed this subject in issue #4979 where I traced the problem to the same source, though without a good idea of the correct fix. It looks like |
|
I think it would probably be fine to outright remove If we really want to keep How do other graphics libraries handle this? |
Overview: What does this pull request change?
Removes
LineJointType.AUTOandCapStyleType.AUTO, and disallowsNonefor thejoint_typeconstructor argument inVMobject. This will may be a breaking change for anyone who explicitly sets the join/cap types toNoneorAUTOMotivation and Explanation: Why and how do your changes improve the library?
Currently, if you set the joint or cap type of a
VMobjectto something other thanAUTO, the joint/cap type is applied to all otherVMobjects that are rendered after it. This is a bug (or at least it's highly unintuitive behavior). This has been noted in issue #3410.The behavior of
AUTO(apart from the bug noted above) matchesMITERfor joints andBUTTfor caps, and the issue is resolved if we default to these values instead ofAUTO.Links to added or changed documentation pages
Further Information and Comments
I admit I don't know why the
AUTOvalues exist. They don't serve an obvious purpose, so it's worth double-checking that I haven't missed some use-case for them. I've tested this on my projects and haven't seen any issues. If my removal ofAUTOandNonechanges the contract too much, we could instead just change the defaults.Reviewer Checklist