Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt to changes in private headers Qt v6.9 #311

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

palinek
Copy link
Contributor

@palinek palinek commented Dec 10, 2024

fixes #310

Copy link
Member

@tsujan tsujan left a comment

Choose a reason for hiding this comment

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

GTM.

We may need a point release before Qt 6.9 comes out.

@tsujan
Copy link
Member

tsujan commented Dec 13, 2024

Unfortunately, this breaks our icon engine.

This is a desktop file in pcmanfm-qt's thumbnail view, with a 128-px size, and with libqtxdg 4.1.0:
1

And the same file with this patch:
2

The desktop file has Icon=emblem-music-symbolic, which is a 16-px SVG icon belonging to Breeze. Without the patch, it's correctly replaced by emblem-music from my icon set (according to our old rule: "give priority to the set, and only if not found, go to the inherited sets"); with the patch, it's taken from Breeze, which is inherited.

Without the patch, it has the correct size (128); with the patch, its size is 16 px.

EDIT: See "EDIT" in my next comment.

@tsujan tsujan self-requested a review December 13, 2024 22:49
@tsujan
Copy link
Member

tsujan commented Dec 13, 2024

Solved the size problem by replacing

if (dir.type == QIconDirInfo::Scalable || dynamic_cast<ScalableEntry *>(entry))
return size;

with

        if (dir.type == QIconDirInfo::Scalable
            || dynamic_cast<ScalableEntry *>(entry)
            || dynamic_cast<ScalableFollowsColorEntry *>(entry)) {
            return size;
        }

But the path problem still exists.

EDIT:
Please forget about path. I was mistaken; it was the same before (I'd changed the Icon= line in the above screenshot for testing).

{
#if (QT_VERSION >= QT_VERSION_CHECK(6,8,0))
QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) override;
#else
QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) override;
#endif
QIcon svgIcon;
Copy link
Member

@tsujan tsujan Dec 13, 2024

Choose a reason for hiding this comment

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

Isn't it better to put the above changes inside #if (QT_VERSION >= QT_VERSION_CHECK(6,8,0))? We still support Qt 6.6.

EDIT: Never mind; Qt 6.6 will be OK with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FTBFS with Qt 6.9.0
2 participants