Zandronum Chat on our Discord Server Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003319Doomseeker[All Projects] Suggestionpublic2017-10-29 13:542018-10-05 07:27
Assigned To 
PlatformOSOS Version
Product Version1.1 
Target VersionFixed in Version 
Summary0003319: Themes
DescriptionIntroduce concept of appearance themes to Doomseeker. Themes could define appearance of such objects as fonts, backgrounds, icons or widget general appearance.

Qt supports a limited subset of CSS which can be used to apply theming to widgets.

Resources can be placed in appropriate subdir in "theme" dir located in all of the "Static Data Paths".
Additional InformationThe primary purpose of this ticket is to be able to unbake from the executable the non-free buddies.png icon. Current buddies.png can then be kept in a theme directory, thus we can both keep it and satisfy "freedom" requirements.
Attached Filespatch file icon themes.patch [^] (12,418 bytes) 2017-10-29 13:54 [Show Content]

- Relationships
related to 0003320closedZalewa Replace buddies.png (id Software / Doom) 

-  Notes
User avatar (0018646)
Zalewa (developer)
2017-10-29 14:05

The included "themes.patch" does a primitive implementation of icon theming. This currently will only work for buddies.png. Unfortunately all references to ":" when creating QPixmaps or QIcons need to be replaced. Basically, QIcon(":icons/buddies.png") becomes Theme::main.icon("icons/buddies.png").

We also still want to keep the more-default-than-default icons baked into the executable. If theme fails to load, this will prevent Doomseeker from suddenly starting with a broken UI.

Unfortunately, this ruins the neatness of Qt Designer - any icon that we specify there will have to be overwritten manually in code. I tried to QDir::addSearchPath("", theme_dir), but this doesn't work when the prefix is empty. QIcon(":icons/buddies.png") will not look up the theme_dir. This means that Qt Designer should no longer be used to specify icons, because its duplicated work that will get overwritten. This also means that we lose preview and gain tedium.

Furthermore, we will have to go over all places in the code where icons are applied and convert the statements there to Theme::main.

Any comments on how to best approach this?
User avatar (0018653)
Blzut3 (administrator)
2017-10-29 18:33

Strange that adding a search path didn't work since the documentation clearly states that it should:
Qt's resources support the concept of a search path list. If you then refer to a resource with : instead of :/ as the prefix, the resource will be looked up using the search path list. The search path list is empty at startup; call QDir::addSearchPath() to add paths to it.

That said Qt Designer unfortunately uses ":/" instead of just ":" so unfortunately that sounds like your out of luck as far as that goes.

They do however mention loading qrc binaries through QResource::registerResource which sounds like it has potential even though it would make producing icon themes slightly more complicated.
User avatar (0018655)
Zalewa (developer)
2017-10-29 18:41

I cooked up a minimal example to verify this and in fact it doesn't work.

Quote from Blzut3
They do however mention loading qrc binaries through QResource::registerResource which sounds like it has potential even though it would make producing icon themes slightly more complicated.

Yes, I was aware of that, however for the reason you stated I figured this is unviable in the long run.
User avatar (0018656)
WubTheCaptain (developer)
2017-10-29 18:47

Tagging for review (Blzut3), since a patch is included.

It's a nice idea overall, but later I'd imagine asking "why did we do this?". It's a complex solution with more code added to primarily tackle a problem of legally encumbered distribution of non-free buddies.png, as elegant and iconic as the icon looks like.

Let's imagine distribution of non-free "buddies.png" to be barred completely: Would this patch/feature be useful anymore after that, or does it become featuritis? Thinking more open-minded about this, I can see themes to replace baked icons may have an use because Doomseeker supports more engine plugins than Doom too. I can also see e.g. a dark theme being implemented (in example: OBS Studio has one).

In regards to 0003237, the existence of non-free "buddies.png" in the source archive is still something downstream distributors have to handle with care and burden. This proposed patch does not resolve that.

Breaking Qt Designer is also something I'm not a fan of thought.

I'd reject this patch, but would be fine on limited scope of theming like light vs dark themes.

(Kdenlive supports icon theming, but alas it also has default icons baked in and it's KDE. The UI is still broken in parts with missing icons.)
User avatar (0018662)
WubTheCaptain (developer)
2017-10-29 19:45
edited on: 2017-10-29 19:45

For clarification: If the current id Software / Doom "buddies.png" becomes a non-default theme in a contrib directory (and distribution is legal), all fine for me so I could perhaps support this. See 0003320, which was my primary idea of resolving the underlying issue.

User avatar (0018664)
Blzut3 (administrator)
2017-10-29 20:56

Very curious. Given how popular skinning is in Windows apps it really surprises me that Qt doesn't make that easy. Oh well I guess.

I don't think the registerResource solution (assuming it works) is too much of a hurdle, but I don't know how much anyone cares to skin Doomseeker.
User avatar (0018670)
AOSP (reporter)
2017-10-29 23:53

Oh yes, please. Anything to solve the bad contrast the red icons have on dark Qt system themes (ie Breeze Dark).
User avatar (0018673)
WubTheCaptain (developer)
2017-10-30 01:21

Feel free to assign to yourself if you're working on this.
User avatar (0018683)
WubTheCaptain (developer)
2017-10-30 06:00

Actually, do we need to do anything here? See the QT_STYLE_OVERRIDE environment variable or qt5ct. [^]

Icon themes seem to be possible too.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: AOSP
Opponents: WubTheCaptain

- Issue History
Date Modified Username Field Change
2017-10-29 13:54 Zalewa New Issue
2017-10-29 13:54 Zalewa File Added: themes.patch
2017-10-29 13:55 Zalewa Relationship added related to 0003237
2017-10-29 14:05 Zalewa Note Added: 0018646
2017-10-29 18:33 Blzut3 Note Added: 0018653
2017-10-29 18:41 Zalewa Note Added: 0018655
2017-10-29 18:47 WubTheCaptain Note Added: 0018656
2017-10-29 18:47 WubTheCaptain Status new => needs review
2017-10-29 18:56 WubTheCaptain Relationship added related to 0003320
2017-10-29 19:00 WubTheCaptain Relationship deleted related to 0003237
2017-10-29 19:45 WubTheCaptain Note Added: 0018662
2017-10-29 19:45 WubTheCaptain Note Edited: 0018662 View Revisions
2017-10-29 20:56 Blzut3 Note Added: 0018664
2017-10-29 23:53 AOSP Note Added: 0018670
2017-10-30 01:21 WubTheCaptain Note Added: 0018673
2017-10-30 01:21 WubTheCaptain Status needs review => acknowledged
2017-10-30 06:00 WubTheCaptain Note Added: 0018683
2017-10-30 06:00 WubTheCaptain Status acknowledged => new
2017-11-02 13:49 WubTheCaptain Status new => feedback
2017-11-03 12:49 WubTheCaptain Severity minor => feature
2018-10-05 07:27 WubTheCaptain Priority normal => low

Questions or other issues? Contact Us.


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker