I noticed in a screenshot from another PR that out of the six backends displayed, only the Qt5Agg icon fails to adapt to dark mode. It remains black and blends in with the background, making it difficult to distinguish.

Old behavior

New behavior

Old behavior

New behavior

PR checklist

"closes #0000" is in the body of the PR description tolink the related issue
new and changed code istested
[N/A]Plotting related features are demonstrated in anexample
[N/A]New Features andAPI Changes are noted with adirective and release note
[N/A] Documentation complies withgeneral anddocstring guidelines

fix: Qt5Agg support darkmode icon

3b0b28b

github-actionsbot added the GUI: Qt label

Sep 16, 2025

LangQi99 marked this pull request as ready for review

September 16, 2025 03:03

LangQi99 marked this pull request as draft

September 16, 2025 04:25

fix(qt5agg): scale SVG toolbar icons to toolbar size and safely end Q…

f6063d3

…Painter to avoid crash

LangQi99 changed the title~~fix: Qt5Agg support darkmode icon~~fix: Qt5Agg support darkmode icon by using svg

Sep 16, 2025

LangQi99 marked this pull request as ready for review

September 16, 2025 07:52

tacaswell reviewed

Sep 16, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py OutdatedShow resolvedHide resolved

tacaswell reviewed

Sep 16, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py OutdatedShow resolvedHide resolved

LangQi99 added2 commits

September 17, 2025 19:03

Regenerate Qt5 tool icons when device pixel ratio changes

989ae90

Remove defensive check for Qt5 tool SVG icon existence

5c5d6e5

Copy link

Member

timhoffm commentedSep 17, 2025

Not sure, but would using aQIconEngine remove the need for a manual refresh? As far as I read it, the engine could provide the right-sized image on demand (pull principle), instead of explicitly updating on a dpi-change event (push principle).

LangQi99 added2 commits

September 18, 2025 12:24

refactor(backend_qt): Use QIconEngine for on-demand icon rendering

99265bb

Enhance SVG icon rendering support for multiple Qt versions

c06e132

QuLogic reviewed

Sep 18, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py OutdatedShow resolvedHide resolved

LangQi99 added2 commits

September 18, 2025 17:59

refactor(backend_qt): Improve code quality based on review feedback

3dba46e

Move QtSvg import to qt_compat

fa99cdd

LangQi99 force-pushed theqt5icon branch from267d76b tofa99cddCompare

September 18, 2025 09:59

timhoffm reviewed

Sep 18, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py OutdatedShow resolvedHide resolved

github-actionsbot added the topic: canvas and figure manager label

Sep 19, 2025

LangQi99 force-pushed theqt5icon branch 2 times, most recently from7c1c620 tofb0c6dfCompare

September 19, 2025 02:04

github-actionsbot removed the topic: canvas and figure manager label

Sep 19, 2025

LangQi99 added2 commits

September 19, 2025 10:06

Implement Qt toolmanager svg icon

9a154e3

Simplify dark mode and large image detection

462afeb

LangQi99 force-pushed theqt5icon branch fromfb0c6df to462afebCompare

September 19, 2025 02:07

timhoffm reviewed

Sep 19, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py OutdatedShow resolvedHide resolved

lib/matplotlib/backends/backend_qt.py Outdated

Comment on lines 740 to 744

		def_render_svg(self,svg_path,pixmap,size,dpr):
		"""Render SVG content to pixmap."""
		QSvgRenderer=qt_compat.get_qsvg_renderer()
		ifQSvgRendererisNone:
		returnQtGui.QPixmap()

Copy link

Member

timhoffmSep 19, 2025

Choose a reason for hiding this comment

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

I find the logic not quite clean. You passpixmap and sometimes return that pixmap with something rendered into, and sometimes you return a different pixmap. I'd rather have that we either only draw into the pixmap (and then do not need to return anything, or that we change to_create_pixmap_from_* and not pass a pixmap in.

QuLogic reviewed

Sep 19, 2025

View reviewed changes

lib/matplotlib/backends/qt_compat.py OutdatedShow resolvedHide resolved

refactor(backend_qt): Improve the clarity of the code's logic.

e94e9dd

timhoffm reviewed

Sep 19, 2025

View reviewed changes

lib/matplotlib/backends/backend_qt.py Outdated

Comment on lines 712 to 713

		# Get the current device pixel ratio
		dpr= (self.toolbar.devicePixelRatioF()or1)ifself.toolbarelse1

Copy link

Member

timhoffmSep 19, 2025

Choose a reason for hiding this comment

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

Idea for further simplification:

move this into a private function_devicePixelRatio().

Then ...

lib/matplotlib/backends/backend_qt.py Outdated

Comment on lines 718 to 726

		returnself._create_pixmap_from_svg(svg_path,size,dpr)
		else:
		large_path=self.image_path.with_name(self.image_path.stem+'_large.png')
		path=large_pathiflarge_path.exists()elseself.image_path
		scaled_size=QtCore.QSize(
		int(size.width()*dpr),
		int(size.height()*dpr)
		)
		returnself._create_pixmap_from_png(path,scaled_size,dpr)

Copy link

Member

timhoffmSep 19, 2025

Choose a reason for hiding this comment

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

Interpretsize as logical display size for the scope of this function and hide the calculation of the physical size fully inside the_create_pixmap_from_* functions, usingself._devicePixelRatio() in there.

This reduces here to

Suggested change

	returnself._create_pixmap_from_svg(svg_path,size,dpr)
	else:
	large_path=self.image_path.with_name(self.image_path.stem+'_large.png')
	path=large_pathiflarge_path.exists()elseself.image_path
	scaled_size=QtCore.QSize(
	int(size.width()*dpr),
	int(size.height()*dpr)
	)
	returnself._create_pixmap_from_png(path,scaled_size,dpr)
	returnself._create_pixmap_from_svg(svg_path,size)
	else:
	returnself._create_pixmap_from_png(self.image_path,size)

All the type-dependent implementation details (dpi handling, using a possibly available_large PNG image) are then hidden in the respective function.

Copy link

ContributorAuthor

LangQi99Sep 19, 2025

Choose a reason for hiding this comment

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

Thanks for your patient feedback and guidance. I've really learned a lot from it.💡

Copy link

Member

timhoffmSep 19, 2025

Choose a reason for hiding this comment

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

@LangQi99 thanks as well for the patience to go through the sequence of review cycles! We try to keep them low, but in this case, the proper solution only revealed itself step-by-step. But maybe seeing the evolution of thoughts is also instructive 😃.