Skip to content

Fix marker fillstyle rotation transform#31301

Open
jayaprajapatii wants to merge 3 commits intomatplotlib:mainfrom
jayaprajapatii:fix-fillstyle-rotation
Open

Fix marker fillstyle rotation transform#31301
jayaprajapatii wants to merge 3 commits intomatplotlib:mainfrom
jayaprajapatii:fix-fillstyle-rotation

Conversation

@jayaprajapatii
Copy link
Contributor

@jayaprajapatii jayaprajapatii commented Mar 13, 2026

Closes #31257

Problem

Updating the fillstyle of a marker using Line2D.set_fillstyle() recreates the marker using MarkerStyle(self._marker.get_marker(), fs). This drops existing attributes stored in the original MarkerStyle, including the rotation transform. As a result, rotated markers lose their rotation after updating the fillstyle.

Cause

set_fillstyle() constructs a new MarkerStyle from the marker symbol. This discards internal attributes such as the transform that were present in the original MarkerStyle instance.

Solution

Use the MarkerStyle._with_attrs() factory method to construct the updated marker while preserving the attributes of the existing marker.
This ensures that properties such as transform, joinstyle, and capstyle remain unchanged when updating the fillstyle.

Changes

  • Updated Line2D.set_fillstyle() to use MarkerStyle._with_attrs() instead of constructing a new MarkerStyle.
  • Preserved marker attributes such as transform, joinstyle, capstyle and snap_threshold.
  • Added a regression test to ensure that rotated markers retain their transform when fillstyle is updated.

Result

Rotated markers now maintain their rotation when set_fillstyle() is called, matching the expected behaviour described in the issue.

Comment on lines +554 to +555
if not isinstance(marker, MarkerStyle):
marker = MarkerStyle(marker)
Copy link
Member

Choose a reason for hiding this comment

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

What can marker be apart of MarkerStyle. I suspect we should rather coerce self._marker to always be a MarkerStyle right away instead of doing a conversion here.

Comment on lines +275 to +280
new = cls(marker=marker, fillstyle=fillstyle)

new._user_transform = other._user_transform
new._joinstyle = other._joinstyle
new._capstyle = other._capstyle
new._snap_threshold = other._snap_threshold
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly copying selected values puts us in dager of forgetting something.

Please check: Is it reasonable to copy the whole other.__dict__ and then replace the required attrs? This among other things depends on whether the attributes are independent of each other.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Please also answer the questions in the two comments from previous reviews, that I have not marked as resolved.

Comment on lines +533 to +534
from matplotlib.markers import MarkerStyle

Copy link
Member

Choose a reason for hiding this comment

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

Imports must be placed at the top, not somewhere in the middle of the file. Also, the import is already there.

Suggested change
from matplotlib.markers import MarkerStyle

from matplotlib.transforms import Affine2D

import pytest
from matplotlib.markers import MarkerStyle
Copy link
Member

Choose a reason for hiding this comment

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

Please ensure a consistent use throughout the module; i.e. don't mix markers.MarkerStyle and MarkerStyle.

@jayaprajapatii
Copy link
Contributor Author

Hello @timhoffm

I explored approaches using copy.copy and copying __dict__, but they led to incorrect rendering (e.g., markers appearing fully filled), likely due to internal cached state not being properly reconstructed.
So I switched to reconstructing MarkerStyle using the base marker (via get_marker) and passing fillstyle in the constructor. This ensures correct path rebuilding and preserves rotation via user_transform.

Happy to revise if you'd prefer a different approach.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Update of markers' fillstyle removes rotation transform

2 participants