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

Exporting concise SVG#30598

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

Open
melwyncarlo wants to merge2 commits intomatplotlib:main
base:main
Choose a base branch
Loading
frommelwyncarlo:issue_17424

Conversation

melwyncarlo
Copy link

@melwyncarlomelwyncarlo commentedSep 25, 2025•
edited
Loading

PR summary

Currently all SVG elements are exported as path, which is currently the most viable solution, as other solutions have their pitfalls. But a concise and apt SVG could make things better and is something probably to work on.

This PRcloses#17424. (well, not really, just wanted to link it to the issue as per the PR checklist)
It's not the final solution but it solves the problem. There are restrictions involved in this solution (reference):

# Option-1: (preferred alternative; currently in use over here)#   Set attrib['vector-effects'] = 'non-scaling-stroke'#   in _get_style_dict()#       Issue:#           Part of SVG Tiny 1.2 and SVG 2 (active development)#           Supported on modern browsers but not all image editors#           Does not scale stroke-width on scaling/zooming,#           though it zooms well on browsers while viewing.#           Does not directly scale stroke-width when exporting to#           higher resolution raster images (PNG); requires first#           manually scaling the SVG by:#               1. scaling the SVG width, height, and viewbox#                   e.g.: current width = 480.0, height = 345.6#                         viewbox = 0 0 480.0 345.6#                         To obtain 4800px x 3456px image, set#                         new width = 4800.0, height = 3456.0#                         viewbox = 0 0 4800.0 3456.0#               2. Applying transform scale() to the figure#                   e.g.: (cont'd) scale factor = 4800 / 480 = 10#                         so, change:  <g> to:#                         <g transform="scale(10)"># Option-2:#   gc.set_linewidth(gc.get_linewidth() / transform_scale_factor)#       Issue:#           Works well if tranform_matrix consists only of#           uniform scaling and translation, i.e.,#           transform_scale_factor = abs(transform_matrix[0, 0])#           Otherwise, it requires an advanced affine matrix#           decomposition algorithm (SVD or Polar) to compute the#           transform_scale_factor.#           Still does not correct non-uniform stroke-width#           caused by shearing/skewing.# Option-3: (currently in use in main branch)#   Convert all shapes to path, apply transformations to path,#   and then draw path. This basically avoids all the above#   mentioned pitfalls.

I have performed relevant tests:

P.S.: It's not meant to pass any CI/CD tests, as the solution itself is assumed to be subject to changes.

draw_path_instead=False
# Below two lines are only for testing/comparing
# and can be removed
withopen('pure_svg_mode','r')asf:
Copy link
Member

Choose a reason for hiding this comment

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

If we were to merge this, we would have to provide an rcparam to control it (like we do with text in vector backends that let you pick "text + embed the fonts / hope they are available in the viewer" vs "draw text as paths")

Copy link
Author

Choose a reason for hiding this comment

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

with open('pure_svg_mode', 'r') as f: isNOT for merging (release version). It's simply there to perform my comparison tests as mentioned in my Test Code Repository (link in issue description), i.e., to compare between the old method and the new method. It will be deleted.

common_args= (gc,
base_transform,
draw_path_args[2])# rgbFace
ifisinstance(patch, (Polygon,RegularPolygon,
Copy link
Member

Choose a reason for hiding this comment

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

I believe that we now have a high enough minimum python we can use thematch statement.

Copy link
Author

Choose a reason for hiding this comment

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

Oh yes, I will make that change.
I was also wondering if instead of multiple conditions in this draw function, shouldn't they be called from within their respective classes itself? Or is this the right way forward?

withopen('pure_svg_mode','r')asf:
iff.read()=='1':
frommatplotlib.backends.backend_mixedimportMixedModeRenderer
ifisinstance(renderer,MixedModeRenderer)andrenderer.is_svg_renderer():
Copy link
Member

Choose a reason for hiding this comment

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

Rather than pin to the svg renderer, if we are going to do this then we should duck-type and allow backends to opt-in to providing more specialized rendering calls.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, something to work on.

@tacaswell
Copy link
Member

I'm mildly in favor of this conceptually, but I think the first thing to sort out is how we want to allow backends to provide additional methods that not all have.

I think the path is to add "additional methods that backendsmay implement for specialization reasons" + an rcparam to globally turn trying to use them on/off. This is not perfectly clear (as say the users asks for specialization and the backend happens to havedraw_circle, should that be prefferniatlly used bydraw_marker when the marker is definitly a circle?) but I think is a good starting point. This has some (conceptual) overlap with the methods related to blitting.

Unfortunately given that we require changes inside ofdraw, I don't see an easy way to do this in an extension module first (I can think of terrible ways that involve sub-classing and monkey-patching but that is a bit too messy to advocate for).

melwyncarlo reacted with thumbs up emojijsalsman reacted with eyes emoji

@melwyncarlo
Copy link
Author

melwyncarlo commentedSep 25, 2025•
edited
Loading

I agree this will take time and a lot of revisions.

I thought there was only one SVG backend. Are there others?

I'm not well knowledgeable on the topic ofspecialisation that you're talking about.

Extension modules for SVG implementation or backends?

Wouldn't markers too internally be simplified into their basic shape classes?

@anntzer
Copy link
Contributor

anntzer commentedSep 29, 2025•
edited
Loading

The cairo (internal) and mplcairo (third-party) backends can also render to svg.
mplcairo actually already internally detects circlemarkers (by checking for whether the path is the same object as the Path.unit_circle() singleton) and partially special-cases them (see block starting athttps://github.com/matplotlib/mplcairo/blob/d922884dcaac3a52ec122d378b0e17629b4909b2/ext/_util.cpp#L652).

@timhoffm
Copy link
Member

I think the path is to add "additional methods that backendsmay implement for specialization reasons"

I'd like to bring backend versioning back to the table.#30559 While it doesn't solve all issues here, it would really help to formally specify variants of the backend API.

tacaswell reacted with thumbs up emoji

@anntzer
Copy link
Contributor

I don't think we should really ever require (or suggest that it be required) that backends provide all these draw_foo methods. Something more reasonable may be to add a single pair ofcan_draw_patch(Patch) -> bool anddraw_patch(Patch) methods (and if the backend says that it cannot natively draw the patch, then fall back to the standard Path representation.

@QuLogic
Copy link
Member

I don't think we should really ever require (or suggest that it be required) that backends provide all these draw_foo methods. Something more reasonable may be to add a single pair ofcan_draw_patch(Patch) -> bool anddraw_patch(Patch) methods (and if the backend says that it cannot natively draw the patch, then fall back to the standard Path representation.

Similar happens for text; you can implementdraw_text, but if not, then everything goes throughdraw_path (whichis required).

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[feature request] circle -> SVG <circle>
5 participants
@melwyncarlo@tacaswell@anntzer@timhoffm@QuLogic
[8]ページ先頭
©2009-2025 Movatter.jp