Skip to content

Arcs with large radii in small #17547

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
Closed
LanceJenkinZA opened this issue Jun 1, 2020 · 4 comments · Fixed by #17564
Closed

Arcs with large radii in small #17547

LanceJenkinZA opened this issue Jun 1, 2020 · 4 comments · Fixed by #17564
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone
v3.2.2

Comments

@LanceJenkinZA
Copy link

LanceJenkinZA commented Jun 1, 2020 •
edited
Loading

Bug report

Bug summary

No matplotlib.patches.Arc is being rendering when trying to add a small segment of a circle with a large radius.

If I increase the x and y limits to include a larger section of the arc then it is rendered.

Code for reproduction

import matplotlib.pylab as pl
from matplotlib import patches

fig = pl.figure()
ax = pl.gca()
x = 212.02529006006412
y = -2115.252900600641
diameter = 4261.655587271447
a = patches.Arc((x, y), diameter, diameter, fill=False)

ax.add_patch(a)
pl.xlim(0, 20)
pl.ylim(0, 20)
fig.show()

Actual outcome

An empty figure:

image

Expected outcome

A figure with a small section of an arc starting a $(0, 5)$ and ending at $(7.98894818241678, 10.0)$
image

Matplotlib version

  • Operating system: Windows 10 Pro
  • Matplotlib version: 3.2.1
  • Matplotlib backend (print(matplotlib.get_backend())): module://ipykernel.pylab.backend_inline
  • Python version: Python 3.7.5
  • Jupyter version (if applicable): Version 0.35.6
  • Other libraries:
@LanceJenkinZA
Copy link
Author

The issue is with the comparison on Line L1669 of patches.py:

        # Get width and height in pixels
        width, height = self.get_transform().transform((width, height))
        inv_error = (1.0 / 1.89818e-6) * 0.5
        if width < inv_error and height < inv_error:
            self._path = Path.arc(theta1, theta2)
            return Patch.draw(self, renderer)

For this case I'm getting:

inv_error = 263410.21399445785
width = 21326216.85880001
height = 89320455.38223404

I've disabled the check and it does render correctly:

image

@tacaswell tacaswell added this to the v3.2.2 milestone Jun 2, 2020
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Jun 2, 2020
@anntzer anntzer mentioned this issue Jun 2, 2020
Open
@anntzer
Copy link
Contributor

anntzer commented Jun 2, 2020

This bisects to #8047 (2.1). So it should be fixed but I'm not sure it's release critical.

@tacaswell
Copy link
Member

The Arc artists is designed to either draw the full circle and let it get clipped or to render the arc using a more accurate (and expensive) method.

I also just got to this bisecting to a 2.1 change so it is not a recent regression, but it should still be fixed ASAP as this is a rather terrible bug.

@tacaswell
Copy link
Member

The fix seems to be pretty simple, we need to not do the stretching when going through the accurate path.

tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 3, 2020
@tacaswell
FIX: correctly handle large arcs
697f330 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - only "squash" the angles in the "small" case is we handle it
   in the "big" case via transforms
 - make sure that we don't re-order the angles while "squashing" them.

Adjusted the test image to use this failing case and to exercise both
code paths.

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 3, 2020
@tacaswell
FIX: correctly handle large arcs
18c5357 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - only "squash" the angles in the "small" case is we handle it
   in the "big" case via transforms
 - make sure that we don't re-order the angles while "squashing" them.
 - the computed height / width is offset in pixels of the center of the arc
   from the lower left of the figure.  We do not care about the sign,
   only that it is "far away" or not.

Adjusted the test image to use this failing case and to exercise both
code paths.

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 4, 2020
@tacaswell
FIX: correctly handle large arcs
332a207 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - only "squash" the angles in the "small" case is we handle it
   in the "big" case via transforms
 - make sure that we don't re-order the angles while "squashing" them.
 - the computed height / width is offset in pixels of the center of the arc
   from the lower left of the figure.  We do not care about the sign,
   only that it is "far away" or not.

Adjusted the test image to use this failing case and to exercise both
code paths.

closes matplotlib#17547
@tacaswell tacaswell mentioned this issue Jun 4, 2020
Merged
2 tasks
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 5, 2020
@tacaswell
FIX: correctly handle large arcs
cc9e35c 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - make sure that we don't re-order the angles while "squashing" them.
 - the computed height / width is offset in pixels of the center of the arc
   from the lower left of the figure.  We do not care about the sign,
   only that it is "far away" or not.

Adjusted the test image to use this failing case and to exercise both
code paths.

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 8, 2020
@tacaswell
FIX: correctly handle large arcs
719a920 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - the computed height / width is offset in pixels of the center of the arc
   from the lower left of the figure.  We do not care about the sign,
   only that it is "far away" or not.
 - make sure that we keep angles in [0, 360) range

Tests:

 - Adjusted an existing test image to use this failing case and to exercise both
   code paths.
 - Added a test function of "low resolution" angles, Should draw the arc
   from the red to blue lines in the counter-clockwise direction.

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 10, 2020
@tacaswell
FIX: correctly handle large arcs
9ef4674 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - make sure that we keep angles in [0, 360) range
 - only go through the angle stretching code if we need to (to avoid
   numerical instability of angles not round-tripping with scale=1)

Tests:

 - Adjusted an existing test image to use this failing case and to exercise both
   code paths.
 - Added a test function of ensuring we can draw a big arc in each
   quadrant

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 10, 2020
@tacaswell
FIX: correctly handle large arcs
620af63 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - make sure that we keep angles in [0, 360) range
 - only go through the angle stretching code if we need to (to avoid
   numerical instability of angles not round-tripping with scale=1)

Tests:

 - Adjusted an existing test image to use this failing case and to exercise both
   code paths.
 - Added a test function of ensuring we can draw a big arc in each
   quadrant

closes matplotlib#17547
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 11, 2020
@tacaswell
FIX: correctly handle large arcs
03da86e 
The draw method of mpatches.Arc has two paths:
 - if the arc is "small" compared to its size in the rendered image
   then render the whole arc and let clipping do it's thing
 - if the arc is "big" compared to its size on the screen then sort
   out where the circle intersects the axes boundary and only draw
   that part of it

This makes several changes to the Arc draw method:

 - make sure that we keep angles in [0, 360) range
 - only go through the angle stretching code if we need to (to avoid
   numerical instability of angles not round-tripping with scale=1)

Tests:

 - Adjusted an existing test image to use this failing case and to exercise both
   code paths.
 - Added a test function of ensuring we can draw a big arc in each
   quadrant

closes matplotlib#17547
@Lucas-Steinmann Lucas-Steinmann mentioned this issue Nov 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.2.2
Development

Successfully merging a pull request may close this issue.

FIX: correctly handle large arcs tacaswell/matplotlib
3 participants
@tacaswell @anntzer @LanceJenkinZA