Confirm / Fix miter joins in Canvas#4162
Conversation
Most backends have a "miter limit" parameter or something like that that bevels sharp miter joins because they can get arbitrarily long. You may need to ensure that this is also set appropriately on all platforms. Edit to add: this is probably what you want https://developer.android.com/reference/android/graphics/Paint#getStrokeMiter() |
|
My 2 cents -- the HTML canvas has a default miter limit ratio of 10. Qt's value is 2... it may make sense to set the miter limit to 10 to match HTML5 on all platforms; or better yet, expose values for the user to do so. I think as a default for this PR as a bugfix, though, mitering and using a limit of 10 is fine. (P.S... considering your job I think I know why you'd like miter joins :) ) |
|
It turns out it's a little more complicated than that... Qt treats the miter limit differently.
*That is, from the path intersection projected over to the edge of the stroke. I doesn't seem like there is a way to individually turn mitering entirely on and off on a per-corner basis, short of a complicated approach of us drawing individual pairs of segments separately. |
|
On the other hand, Windows seems to be the same as HTML:
And yet it's mitering even an 11º angle, when the cutoff should be about 11.5º... Edit: upon rereading the description, I think it's like Qt too, in that it just stops at the limit, rather than switching mitering entirely off if it would exceed the limit. |
I think this might have to be just one of those differences between the backends (like text rendering) where they intrinsically do things differently. Pixel-perfect agreement on everything cross-platform is probably too much to hope for. We certainly don't want to be getting into the business of figuring out corner shapes individually. |
| stroke.setStyle(Paint.Style.STROKE) | ||
| stroke.setStrokeWidth(2.0) | ||
| stroke.setColor(BLACK) | ||
| stroke.setStrokeMiter(10.0) |
There was a problem hiding this comment.
I think should be 5 (actually probably sqrt(24)) given how it is measured differently?
There was a problem hiding this comment.
Correct, I pushed this before reading that.
Took me a sec to figure out where you got sqrt(24) from, but thank you, that makes sense. (For some reason I had it in my head that the difference would depend on the angle, but right, we're picking the specific angle for the cutoff.)
There was a problem hiding this comment.
My initial intuition was partly correct. That starts truncating at the same angle that HTML stops mitering, which I guess it as good as we can do.
I wish we could make it consitently stop drawing at the same distance that's the bevel cutoff for the HTML spec. But we can't (for all but one angle), because a fixed distance where Qt measures it (along the stroke edge) doesn't stay consistent with where the miter limit should be when measured up from the inside corner:
(The green measurement on the upper right is the Qt limit drawn at sqrt(24), and the bit that's sliding up and down is where the HTML spec puts the miter limit at each angle.)
There was a problem hiding this comment.
Oh, wait, I just noticed where this comment is. Android does things properly, it just has a different default value. It's Qt that's the especially weird one.
|
To summarize:
|
|
Okay... Winforms is definitely not behaving as documented. 😡 Here's an animation of varying the default limit (10) down to 5 and back up (with red lines denoting where the cutoff should be at 10): This is clearly not measuring from the inside of the miter, like it says. All of them are the same height, so it must be measuring from the actual intersection of the path. But it does seem to be starting the truncation at the right angle, at least. |
|
Oops. I hadn't even realised one could remove a review request, let alone that I'd done so. |
freakboy3742
left a comment
There was a problem hiding this comment.
Where do I submit my application for danger pay for having to do this much geometry, this early in the morning? 🤣
This all makes sense to me. The only comment I'd make is that given we know there's some minor platform discrepancies, we should add a platform note in the Canvas widget docs. I've pushed a draft of a new note (and also changed the linting order so spelling is done first...). I'm happy for this to be merged with that note, or if you think there is a need for any other clarification, let me know.
Looks good to me, thanks! |





I noticed while working on #4159 that the Qt backend was beveling its line joins instead of mitering them. Thankfully it's a one-line fix.
As long as I was making such a minor change and it wouldn't be confusing, I went ahead and standardized the docstrings in the test file.
(Marking as "misc" instead of "bugfix" because the Qt Canvas widget has yet to be in a release.)
PR Checklist: