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')

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:

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.

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

brunobeltran force-pushed thestroked_path_width branch 2 times, most recently from5a401ee to7d84966Compare

April 20, 2020 11:58

This was referencedApr 20, 2020

BUGFIX: use true bbox for rasters in backend_mixed#17182

Draft

Line2d window extents#17199

Draft

brunobeltran force-pushed thestroked_path_width branch from7d84966 to0a92c2fCompare

April 20, 2020 15:05

brunobeltran added New feature topic: path handling labels

Apr 22, 2020

brunobeltran force-pushed thestroked_path_width branch from0a92c2f to265caecCompare

June 22, 2020 19:09

Copy link

Member

jkseppan commentedAug 14, 2020

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

jkseppan approved these changes

Aug 14, 2020

View reviewed changes

Copy link

Member

jkseppan 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

jkseppan requested a review fromQuLogic

August 14, 2020 06:19

Copy link

Member

jkseppan commentedAug 14, 2020

The failure diff on appveyor:

Is this an Agg version difference?

Copy link

Member

jkseppan commentedAug 14, 2020

The test result:

And the expected image:

Copy link

Member

jkseppan commentedAug 14, 2020

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

QuLogic reviewed

Aug 25, 2020

View reviewed changes

Copy link

Member

QuLogic 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.

lib/matplotlib/path.py Outdated

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

QuLogicAug 25, 2020

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

brunobeltranAug 31, 2020

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.

lib/matplotlib/path.py OutdatedShow resolvedHide resolved

lib/matplotlib/path.pyShow resolvedHide resolved

lib/matplotlib/path.py OutdatedShow resolvedHide resolved

lib/matplotlib/path.py

		----------
		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

QuLogicAug 25, 2020

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

brunobeltranAug 31, 2020

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

QuLogicAug 31, 2020

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?

lib/matplotlib/path.py OutdatedShow resolvedHide resolved

FEATURE: Path.get_stroked_extents

a055b24

brunobeltran force-pushed thestroked_path_width branch from265caec to48fe47aCompare

August 31, 2020 18:54

Copy link

ContributorAuthor

brunobeltran commentedAug 31, 2020

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 force-pushed thestroked_path_width branch from48fe47a toefceb57Compare

August 31, 2020 19:21

DOCS:@QuLogic's suggestions for stroked extents docs

dad23ba

brunobeltran force-pushed thestroked_path_width branch fromefceb57 todad23baCompare

August 31, 2020 20:10

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> .

QuLogic reviewed

Sep 1, 2020

View reviewed changes

Copy link

Member

QuLogic 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.

lib/matplotlib/path.py

Comment on lines +512 to +515

		cap_angles = [first_tan_angle, prev_tan_angle]
		cap_vertices = [first_vertex, prev_vertex]
		for cap_angle, cap_vertex in zip(cap_angles, cap_vertices):
		yield VertexInfo(cap_vertex, cap_angle, None)

Copy link

Member

QuLogicSep 1, 2020

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)

lib/matplotlib/path.py

Comment on lines +510 to +511

		# deal with capping ends of previous polyline, if it exists
		if prev_tan_angle is not None and is_capped:

Copy link

Member

QuLogicSep 1, 2020

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.

lib/matplotlib/path.py

Comment on lines +562 to +564

		for cap_angle, cap_vertex in [(first_tan_angle, first_vertex),
		(prev_tan_angle, prev_vertex)]:
		yield VertexInfo(cap_vertex, cap_angle, None)

Copy link

Member

QuLogicSep 1, 2020

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)

lib/matplotlib/path.py

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

QuLogicSep 1, 2020

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

lib/matplotlib/path.py

		----------
		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

QuLogicSep 1, 2020

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`.

lib/matplotlib/path.py

		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

QuLogicSep 1, 2020

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

lib/matplotlib/path.py

Comment on lines +1440 to +1450

		cap_perp = vinfo.incidence_angle + perp_dir
		x += (markeredgewidth/2) * np.cos(cap_perp)
		if x < extents[xmin]:
		extents[xmin] = x
		if x > extents[xmax]:
		extents[xmax] = x
		y += (markeredgewidth/2) * np.sin(cap_perp)
		if y < extents[ymin]:
		extents[ymin] = y
		if y > extents[ymax]:
		extents[ymax] = y

Copy link

Member

QuLogicSep 1, 2020

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)

lib/matplotlib/path.py

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

Copy link

Member

QuLogicSep 1, 2020

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.

lib/matplotlib/path.py

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

QuLogicSep 1, 2020

Choose a reason for hiding this comment

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

Clockwise, or counter-clockwise?

lib/matplotlib/tests/test_path.py

Comment on lines +104 to +105

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

Copy link

Member

QuLogicSep 1, 2020

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)

QuLogic added the status: needs revision label

Oct 9, 2020

jklymak added the status: needs rebase label

Apr 23, 2021

jklymak marked this pull request as draft

April 23, 2021 15:31

Copy link

github-actionsbot commentedJul 26, 2023

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-actionsbot added the status: inactiveMarked by the “Stale” Github Action label

Jul 26, 2023

anntzer mentioned this pull request

Aug 11, 2023

get_tightbbox doesn't account for markeredgewidth#16606

Open

Labels

New feature status: inactive

Marked by the “Stale” Github Action

status: needs rebase status: needs revision topic: path handling

4 participants

Movatterモバイル変換

Uh oh!

Stroked path width#17198

Are you sure you want to change the base?

Stroked path width#17198

Uh oh!

Conversation

brunobeltran commentedApr 20, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

jkseppan commentedAug 14, 2020

Uh oh!

jkseppan left a comment

Choose a reason for hiding this comment

Uh oh!

jkseppan commentedAug 14, 2020

Uh oh!

jkseppan commentedAug 14, 2020

Uh oh!

jkseppan commentedAug 14, 2020

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brunobeltran commentedAug 31, 2020

Uh oh!

brunobeltran commentedSep 1, 2020 via email

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actionsbot commentedJul 26, 2023

Uh oh!

Uh oh!

brunobeltran commentedApr 20, 2020•
edited
Loading