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

IconEngine: Forward API to child icon engine #792

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

Conversation

palinek
Copy link
Contributor

@palinek palinek commented Mar 3, 2022

..instead of doing own implementation.

..instead of doing own implementation.
@tsujan
Copy link
Member

tsujan commented Mar 3, 2022

I'd experimented with almost exactly the same thing myself but I didn't make a PR for 2 reasons:

  1. Except for xdndworkaround.cpp, we never used a Qt private header anywhere. I really prefer to keep it that way; otherwise, it wouldn't be worth the probable future hassles.
  2. What follows was very confusing to me and I didn't have time to investigate: it worked with my icon set (https://github.com/tsujan/Plataro) but not with the Breeze set. My guess was that either Breeze had a problem or the problem wasn't completely in Fm::IconEngine.

Since the patches are practically identical, I'm sure I'd see the same things with this too.

@tsujan
Copy link
Member

tsujan commented Mar 3, 2022

Oh, right now, I investigated 2 :) Yes, the Breeze set only includes actions/24@2x in its index.theme, while it does have places/24@2x inside its directory tree. So, 2 is Breeze's fault and I was wrong at #788 (comment) saying, "Breeze is better for testing".

1 is still valid and worries me. But yes, this works.

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.

2 participants