Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Stroked path width#17198

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
brunobeltran wants to merge2 commits intomatplotlib:main
base:main
Choose a base branch
Loading
frombrunobeltran:stroked_path_width

Conversation

@brunobeltran
Copy link
Contributor

@brunobeltranbrunobeltran commentedApr 20, 2020•
edited
Loading

PR Summary

Introduces code topath.py andbezier.py that allows us to compute the extents of strokedPaths analytically. Required for bugfix PRs#17182 and#17199, whichfixes#16606.

Previous Behavior

The following code should nominally get the extents of the path, but the currentget_extents only takes into account the control points:

markeredgewidth=10leg_length=1joinstyles= ['miter','round','bevel']capstyles= ['butt','round','projecting']# The common miterlimit defaults are 0, :math:`\sqrt{2}`, 4, and 10. These# angles are chosen so that each successive one will trigger one of these# default miter limits.angles= [np.pi,np.pi/4,np.pi/8,np.pi/24]# Each column tests one join style and one butt style, each row one angle# and iterate through orientationsfig,axs=plt.subplots(len(joinstyles),len(angles),sharex=True,sharey=True)# technically it *can* extend beyond this depending on miterlimit....axs[0,0].set_xlim([-1.5*leg_length,1.5*leg_length])axs[0,0].set_ylim([-1.5*leg_length,1.5*leg_length])fori, (joinstyle,capstyle)inenumerate(zip(joinstyles,capstyles)):forj,corner_angleinenumerate(angles):rot_angle= (i*len(angles)+j)*2*np.pi/12# A path with two caps and one corner. the corner has:# path.VertexInfo(apex=(0,0), np.pi + rot_angle + corner_angle/2,#                 corner_angle)vertices=leg_length*np.array(            [[1,0], [0,0], [np.cos(corner_angle),np.sin(corner_angle)]])path=Path(vertices, [Path.MOVETO,Path.LINETO,Path.LINETO])path=path.transformed(Affine2D().rotate(rot_angle))patch=PathPatch(path,linewidth=markeredgewidth,joinstyle=joinstyle,capstyle=capstyle)axs[i,j].add_patch(patch)# plot the extentsdata_to_pts= (Affine2D().scale(72)+fig.dpi_scale_trans.inverted()+axs[i,j].transData)bbox=path.get_extents()axs[i,j].plot([bbox.x0,bbox.x0,bbox.x1,bbox.x1,bbox.x0],                        [bbox.y0,bbox.y1,bbox.y1,bbox.y0,bbox.y0],'r-.')axs[i,j].axis('off')

wrong

New Behavior

Now we can use thePath.get_stroked_extents method:

markeredgewidth=10leg_length=1joinstyles= ['miter','round','bevel']capstyles= ['butt','round','projecting']# The common miterlimit defaults are 0, :math:`\sqrt{2}`, 4, and 10. These# angles are chosen so that each successive one will trigger one of these# default miter limits.angles= [np.pi,np.pi/4,np.pi/8,np.pi/24]# Each column tests one join style and one butt style, each row one angle# and iterate through orientationsfig,axs=plt.subplots(len(joinstyles),len(angles),sharex=True,sharey=True)# technically it *can* extend beyond this depending on miterlimit....axs[0,0].set_xlim([-1.5*leg_length,1.5*leg_length])axs[0,0].set_ylim([-1.5*leg_length,1.5*leg_length])fori, (joinstyle,capstyle)inenumerate(zip(joinstyles,capstyles)):forj,corner_angleinenumerate(angles):rot_angle= (i*len(angles)+j)*2*np.pi/12# A path with two caps and one corner. the corner has:# path.VertexInfo(apex=(0,0), np.pi + rot_angle + corner_angle/2,#                 corner_angle)vertices=leg_length*np.array(            [[1,0], [0,0], [np.cos(corner_angle),np.sin(corner_angle)]])path=Path(vertices, [Path.MOVETO,Path.LINETO,Path.LINETO])path=path.transformed(Affine2D().rotate(rot_angle))patch=PathPatch(path,linewidth=markeredgewidth,joinstyle=joinstyle,capstyle=capstyle)axs[i,j].add_patch(patch)# plot the extentsdata_to_pts= (Affine2D().scale(72)+fig.dpi_scale_trans.inverted()+axs[i,j].transData)bbox=path.get_stroked_extents(markeredgewidth,data_to_pts,joinstyle,capstyle)bbox=bbox.transformed(data_to_pts.inverted())axs[i,j].plot([bbox.x0,bbox.x0,bbox.x1,bbox.x1,bbox.x0],                        [bbox.y0,bbox.y1,bbox.y1,bbox.y0,bbox.y0],'r-.')axs[i,j].axis('off')

Saving as a PDF or PNG produces the following:

test

Remaining bugs

If we instead output an SVG, it's wrong (because the default miterlimit is different). To fix this, we would need to deal with#9830, because matplotlib doesn't set the miterlimit right now. This feels acceptably out of scope for this PR. This PR contains all the code required to deal with an arbitrary miterlimit, but it has been hardcoded to 10, since that's the nominal miterlimit for Agg, which is in charge of rasterizing, allowing us tofix#17182.

test svg

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jkseppan
Copy link
Member

Looks good to me. I wonder what's going to with the AppVeyor and Azure checks.

Copy link
Member

@jkseppanjkseppan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Approving modulo CI passing

@jkseppanjkseppan requested a review fromQuLogicAugust 14, 2020 06:19
@jkseppan
Copy link
Member

The failure diff on appveyor:

stroked_bbox-failed-diff

Is this an Agg version difference?

@jkseppan
Copy link
Member

The test result:

stroked_bbox

And the expected image:

stroked_bbox-expected

@jkseppan
Copy link
Member

Once this is merged, we could use this inwriteMarkers inbackend_pdf.py.

Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I only really read docs so far.

Comment on lines 35 to 36
form a corner, the angle swept out by the two lines that meet at the
vertex.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm confused what this means with relation to the next member, especially since that one hascorner in the name.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good catch. This was bad copy/paste from an older version where the names were too difficult to remember (phi/theta, as below). I fixed the description here to actually make sense.

----------
extents : 4*[float]
The extents (xmin, ymin, xmax, ymax) of the `~.transforms.Bbox` of the
vertices. Modified in place so that the corner described by *vinfo*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The type should be more explicit then; it has to a mutable reference of some sort to work (i.e., not tuples.)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is(4,) array_like of float better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Maybe, I'm not sure we have a convention for must-be-mutable,@timhoffm?

@brunobeltran
Copy link
ContributorAuthor

Thanks@QuLogic as usual for your detailed documentation read, I'm glad you caught these lingering copy/paste errors, as they made the code hard to go back to even for me. Hopefully these documentation fixes help the code make lots more sense now.

Let me know if you have any issues parsing what something is supposed to be doing.

I assume you'll ask this question when you actually read the code itself:

"why did he get the angle information in global coordinates, then include the logic to pretend all corners point in the positive x direction right at the end?"

The formulas are much simpler if you fix the corners to be aligned in on specific direction, it turns out. I tried both ways (the other way being to include the switching logic for which direction the corner is pointing directly in the code that's currently in "stroke_x_overflow") and I found this one much more readable.

@brunobeltran
Copy link
ContributorAuthor

brunobeltran commentedSep 1, 2020 via email

The original idea was that ``4*[float]`` was at least not ``4*(float, )``...
…
On Mon, Aug 31, 2020 at 4:42 PM Elliott Sales de Andrade < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/matplotlib/path.py <#17198 (comment)> : > + elif joinstyle == 'round': + return width/2 # hemispherical cap, so always same padding + else: + raise ValueError(f"Unknown joinstyle: {joinstyle}") + + +def _pad_extents_stroked_vertex(extents, vinfo, markeredgewidth, joinstyle, + capstyle): + """ + Accumulator for building true extents from `.VertexInfo`s. + + Parameters + ---------- + extents : 4*[float] + The extents (xmin, ymin, xmax, ymax) of the `~.transforms.Bbox` of the + vertices. Modified in place so that the corner described by *vinfo* Maybe, I'm not sure we have a convention for must-be-mutable,@timhoffm <https://github.com/timhoffm>? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#17198 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALIGPUEAIH5ZFF7Q26TRTTSDQYPHANCNFSM4MML7XCA> .

Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This was pretty-well commented, which made it easier to review. I mean, it still took hours, but probably less than if it weren't.

Comment on lines +512 to +515
cap_angles= [first_tan_angle,prev_tan_angle]
cap_vertices= [first_vertex,prev_vertex]
forcap_angle,cap_vertexinzip(cap_angles,cap_vertices):
yieldVertexInfo(cap_vertex,cap_angle,None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Just yielding the two things seems more straightforward?

Suggested change
cap_angles= [first_tan_angle,prev_tan_angle]
cap_vertices= [first_vertex,prev_vertex]
forcap_angle,cap_vertexinzip(cap_angles,cap_vertices):
yieldVertexInfo(cap_vertex,cap_angle,None)
yieldVertexInfo(first_vertex,first_tan_angle,None)
yieldVertexInfo(prev_vertex,prev_tan_angle,None)

Comment on lines +510 to +511
# deal with capping ends of previous polyline, if it exists
ifprev_tan_angleisnotNoneandis_capped:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Capping occurs if the polyline was unclosed, so I'm 50/50 whetheris_capped should beis_closed (with opposite values), or this comment should mention that.

Comment on lines +562 to +564
forcap_angle,cap_vertexin [(first_tan_angle,first_vertex),
(prev_tan_angle,prev_vertex)]:
yieldVertexInfo(cap_vertex,cap_angle,None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Same here.

Suggested change
forcap_angle,cap_vertexin [(first_tan_angle,first_vertex),
(prev_tan_angle,prev_vertex)]:
yieldVertexInfo(cap_vertex,cap_angle,None)
yieldVertexInfo(first_vertex,first_tan_angle,None)
yieldVertexInfo(prev_vertex,prev_tan_angle,None)

Comment on lines +529 to +530
# often CLOSEPOLY is used when the curve has already reached
# it's initial point in order to prevent there from being a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
#often CLOSEPOLY is used when the curve has already reached
#it's initial point in order to prevent there from being a
#Often CLOSEPOLY is used when the curve has already reached
#its initial point in order to prevent there from being a

----------
markeredgewidth : float
Width, in points, of the stroke used to create the marker's edge.
For ``markeredgewidth = 0``, same as `.get_extents`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
For``markeredgewidth=0``,sameas`.get_extents`.
For``markeredgewidth=0``,theresultwillbecomethesameas`.get_extents`.

phi1=phi+theta/2
phi2=phi-theta/2
return (width/2)*max(np.abs(np.cos(phi1)),np.abs(np.cos(phi2)))
# finally, _joinstyle = "round" is just _joinstyle = "bevel" but with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

?

Suggested change
# finally,_joinstyle = "round" is just_joinstyle = "bevel" but with
# finally,joinstyle = "round" is justjoinstyle = "bevel" but with

Comment on lines +1440 to +1450
cap_perp=vinfo.incidence_angle+perp_dir
x+= (markeredgewidth/2)*np.cos(cap_perp)
ifx<extents[xmin]:
extents[xmin]=x
ifx>extents[xmax]:
extents[xmax]=x
y+= (markeredgewidth/2)*np.sin(cap_perp)
ify<extents[ymin]:
extents[ymin]=y
ify>extents[ymax]:
extents[ymax]=y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm not sure if it looks better this way (ditto the others), or worse and slower...

cap_perp=vinfo.incidence_angle+perp_dirx+= (markeredgewidth/2)*np.cos(cap_perp)extents[xmin]=min(extents[xmin],x)extents[xmax]=max(extents[xmax],x)y+= (markeredgewidth/2)*np.sin(cap_perp)extents[ymin]=min(extents[ymin],y)extents[ymax]=max(extents[ymax],y)

# of the bbox, and see if the stroked vertex expands the extents...
x,y=vinfo.apex
ifnp.cos(vinfo.incidence_angle)>0:
incidence_angle=vinfo.incidence_angle+np.pi/2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

In these cases, I'd name the local variabletheta, as the reference for the angle is different (I think?), so it's different thanincidence_angle.

Comment on lines +1271 to +1272
For vertices with one incoming line, *phi* is the incidence angle that
line forms with the positive y-axis. For corners (vertices with two
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Clockwise, or counter-clockwise?

Comment on lines +104 to +105
@image_comparison(['stroked_bbox'],remove_text=True,
extensions=['pdf','svg','png'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

These are the default extensions?

Suggested change
@image_comparison(['stroked_bbox'],remove_text=True,
extensions=['pdf','svg','png'])
@image_comparison(['stroked_bbox'],remove_text=True)

@github-actions
Copy link

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actionsgithub-actionsbot added the status: inactiveMarked by the “Stale” Github Action labelJul 26, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@QuLogicQuLogicQuLogic left review comments

@jkseppanjkseppanjkseppan approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

get_tightbbox doesn't account for markeredgewidth

4 participants

@brunobeltran@jkseppan@QuLogic@jklymak
[8]ページ先頭
©2009-2025 Movatter.jp