| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Can check carefully in a second, but doesn't the maximum amount of extra clipping box needed depend on the miter limit? I would believe that it happens to occur at 45 degrees, but maybe documenting it that way is misleading? Edit: default PDF miter limit appears to be 10 (https://help.adobe.com/pdfl_sdk/18/PDFL_SDK_HTMLHelp/PDFL_SDK_HTMLHelp/API_References/PDFL_API_Reference/PDFEdit_Layer/General.html), which means the longest possible miter will occur for an angle of around 11.5 degrees (https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/stroke-miterlimit). |
Sorry, something went wrong.
|
P.S. Once #16832 gets merged, I'll be able to clean up #16607 to compute the "exact" Bbox for an arbitrary path given the stroke width and miter limit. Just dropping this reference here so I remember to also fix this code to set an exact clipping path once this is possible. |
Sorry, something went wrong.
|
I don't believe so, because all markers have cap style of 'butt'. |
Sorry, something went wrong.
|
Oh, default joinstyle is 'round', but some are 'miter'. I was a bit worried about that, but no markers in our tests or examples seem to have extended passed the sqrt(2) limit. |
Sorry, something went wrong.
|
Yeah the only ones which are miter are e.g. X and other 90 degree angle-heavy guys. Since this code is marker-specific (and isn't called for general paths, where we might have much more acute angles), I'd say it's good to go as-is, but would still recommend rewording the comment to clarify that the only reason this works is because the built-in markers never have angles more acute than 90 degrees. Nix what I said up there. We now allow custom markers and this will fail for those. I would recommend using an extra buffer of size "10/2" instead of "sqrt(2)/2" to account for the miter limit of 10 (your code worked because "sqrt(2)/2" is the miter limit that starts beveling after 90 degrees). |
Sorry, something went wrong.
|
Actually, I guess MarkerStyle._joinstyle is private, so 45 degrees should work for now. Although we will have to be careful to fix this in the future if we go down a path like suggested in #16773. |
Sorry, something went wrong.
|
Actually, I had focused on X initially, and that one is definitely 90° so 45° makes sense, but looking at triangles again, they are slightly clipped. You have to zoom in about 8x, but since PDF is vector, it's definitely there. |
Sorry, something went wrong.
|
I updated the scaling to 5 as you said, to match the miter limit, and threw in some spec references. This fixes the triangles, 'v', '^', '<', '>', and diamond 'd', which are all markers with sharper corners. I don't think my test triggers it, but I think we'd have to use huge markers to really see it. I'm a bit confused as to why, but I can confirm there's a difference when using the marker reference example. |
Sorry, something went wrong.
There was a problem hiding this comment.
Test seems to check what we want. The fix and comments seem readable. LGTM.
I think making the image tests larger so that it's more precise (to catch 'd', '<', etc.) is getting to be diminishing returns, I'd merge as-is.
Sorry, something went wrong.
|
Why did you need to change the baseline image? Here tests seem to pass without changing it. |
Sorry, something went wrong.
|
TBH, I'm not sure, but I think it has to do with Ghostscript rasterization. If I run diffpdf on it, it sees no difference, nor if I just look at the two in a PDF viewer. |
Sorry, something went wrong.
|
The tests fail with the old baselines for you? |
Sorry, something went wrong.
|
They do, with ghostscript 9.27, if that's the problem. |
Sorry, something went wrong.
|
I guess I would rather have this go on top of #17603 to avoid having to add a new baseline image. |
Sorry, something went wrong.
The bbox only contains the points of the marker, but the line will extend outside by half the line width, unless it's mitered. The PDF miter limit is 10, so pad by 5*line width (half the miter length). This fixes corners on 'v', '^', '<', '>', 'p', 'h', 'H', 'D', 'd', 'X'. Fixes matplotlib#9829.
There was a problem hiding this comment.
Postci.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The bbox only contains the points of the marker, but the line will extend outside by half the line width. This was handled before, except when the line is at an angle to the edge, as then the line outside is wider than half a line width. The maximum is at 45°, or √2.
This fixes corners on 'v', '^', '<', '>', 'p', 'h', 'H', 'D', 'd', 'X'.
Fixes #9829.
PR Summary
PR Checklist