Skip to content

PatchCollection edges incorrect when clip path is set #15946

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.

Sign up for GitHub

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

Jump to bottom
Open
QuLogic opened this issue Dec 16, 2019 · 6 comments
Open

PatchCollection edges incorrect when clip path is set #15946

QuLogic opened this issue Dec 16, 2019 · 6 comments
Labels
backend: agg topic: path handling

Comments

@QuLogic
Copy link
Member

QuLogic commented Dec 16, 2019 •
edited
Loading

Bug report

Bug summary

Cartopy has a mostly-superfluous GeoAxes.background_patch which I'm trying to convert to the standard Axes.patch. By default, the Axes.patch is a Rectangle, which triggers a fast-path to set a clip box instead of a clip path. However, since most map boundaries are non-square, the full clip path code is used instead. When doing so, this causes some strange artifacts.

(Un)Fortunately, it can be reproduced with plain Matplotlib below.

Code for reproduction

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches
from matplotlib.collections import PatchCollection
from matplotlib.path import Path


pth = Path([[0, 0], [1, 0], [1, 1], [0, 1], [0, 1]],
           [1, 2, 2, 2, 79])

fig, ax = plt.subplots(2, 2, sharex=True, sharey=True)
ax[0, 0].set_xlim(-1, 2)
ax[0, 0].set_ylim(-1, 2)

for a in ax[0]:
    collection = a.add_collection(
        PatchCollection([mpatches.PathPatch(pth)],
                        facecolor='C0', edgecolor='k'))

collection.set_clip_path(
    Path([[0, 0], [1, 0], [1, 1], [0, 1], [0, 1]]),
    a.transAxes)

for a in ax[1]:
    patch = a.add_patch(mpatches.PathPatch(pth, facecolor='C0', edgecolor='k'))

patch.set_clip_path(
    Path([[0, 0], [1, 0], [1, 1], [0, 1], [0, 1]]),
    a.transAxes)

ax[0, 0].set_ylabel('PatchCollection')
ax[1, 0].set_ylabel('PathPatch')
ax[1, 0].set_xlabel('Default clip box')
ax[1, 1].set_xlabel('Custom clip path')

plt.savefig('test.png')

Actual outcome

In the bottom row is a plain PathPatch, and the top row is a PatchCollection containing the same PathPatch. The left column shows the default clipping (to the clip box of the Axes). The right column uses a clip path of a manually-specified 0-1 rectangle in Axes space, so it should be equivalent.

But the horizontal edges are either doubled over or the wrong width in the top-right case.

test

Expected outcome

All path edges should look the same.

Matplotlib version

  • Operating system: Fedora 30
  • Matplotlib version: 3.1.3 and master, 9d00ca8
  • Matplotlib backend (print(matplotlib.get_backend())): TkAgg (probably only the Agg bit is relevant)
  • Python version: 3.6.9 / 3.6.3
@QuLogic
Copy link
Member Author

QuLogic commented Dec 16, 2019

Can be reproduced as far back as 1.5.3, and though mplcairo has different antialiasing, it does produce the correct result.

It seems to have something to do with the single path optimization for Collection, which calls Renderer.draw_markers (instead of a draw_path*) and does path drawing completely differently.

@anntzer
Copy link
Contributor

anntzer commented Dec 16, 2019 •
edited
Loading

the builtin vector backends also seem immune, so it's agg only.

This was referenced Dec 16, 2019
Closed
Merged
@QuLogic
Copy link
Member Author

QuLogic commented Jan 7, 2020

So obviously, if I force set do_single_path_optimization = False, then this works correctly. But doing so would break about 48 test images. This is because there's a 0.5px shift for markers that's also translated to these paths. I'm not sure it makes sense for this to happen just because you passed one vs. two colours.

There's an explanation of what the markers optimization is by @mdboom at #3626 (comment), though I don't really understand the benefit for things that are single paths anyway (as opposed to scatter, ticks, etc.)

@anntzer
Copy link
Contributor

anntzer commented Jan 7, 2020

Seems reasonable to disable stamping for single paths, except that IIRC (it's been a while, though) I tried this and ticks (which are single paths for now -- they are drawn independently from one another, which as a side point is terrible performance-wise) look much better when snapped to integer pixels (basically "short" lines (i.e. ticks) look particularly bad when antialiased between two pixel rows). I guess it was pretty accidental that marker stamping helped with tick antialiasing, but heh.
What do the broken test image diffs look like?

@QuLogic QuLogic mentioned this issue Feb 15, 2020
Closed
@tacaswell tacaswell added this to the v3.3.0 milestone Mar 14, 2020
@QuLogic
Copy link
Member Author

QuLogic commented May 23, 2020

Looking at the test results for forcing do_single_path_optimization = False, the failures are most likely from single scatters that would of course also be a single path. I think to cover this case, the single path optimization should not be done if not drawing markers.

@QuLogic
Copy link
Member Author

QuLogic commented May 28, 2020

I tried that out, and while it does work, it also breaks a few tests. I think those tests may be wrong, but we have to see which way we want to go there. This may take a bit more time to decide.

@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 28, 2020
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 27, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Sep 25, 2021
@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Aug 24, 2022
@ksunden ksunden modified the milestones: v3.7.0, v3.7.1 Feb 14, 2023
@QuLogic QuLogic modified the milestones: v3.7.1, v3.7.2 Mar 4, 2023
@QuLogic QuLogic modified the milestones: v3.7.2, v3.7.3 Jul 5, 2023
@QuLogic QuLogic modified the milestones: v3.7.3, v3.8.0 Sep 9, 2023
@ksunden ksunden removed this from the v3.8.0 milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend: agg topic: path handling
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tacaswell @QuLogic @anntzer @ksunden