MantisBT - Doomseeker
View Issue Details
0003319Doomseeker[All Projects] Suggestionpublic2017-10-29 13:542018-10-05 07:27
Zalewa 
 
lowfeatureN/A
feedbackopen 
1.1 
 
0003319: Themes
Introduce 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".
The 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.
No tags attached.
related to 0003320closed Zalewa Replace buddies.png (id Software / Doom) 
patch themes.patch (12,418) 2017-10-29 13:54
https://zandronum.com/tracker/file_download.php?file_id=2228&type=bug
Issue History
2017-10-29 13:54ZalewaNew Issue
2017-10-29 13:54ZalewaFile Added: themes.patch
2017-10-29 13:55ZalewaRelationship addedrelated to 0003237
2017-10-29 14:05ZalewaNote Added: 0018646
2017-10-29 18:33Blzut3Note Added: 0018653
2017-10-29 18:41ZalewaNote Added: 0018655
2017-10-29 18:47WubTheCaptainNote Added: 0018656
2017-10-29 18:47WubTheCaptainStatusnew => needs review
2017-10-29 18:56WubTheCaptainRelationship addedrelated to 0003320
2017-10-29 19:00WubTheCaptainRelationship deletedrelated to 0003237
2017-10-29 19:45WubTheCaptainNote Added: 0018662
2017-10-29 19:45WubTheCaptainNote Edited: 0018662bug_revision_view_page.php?bugnote_id=18662#r11218
2017-10-29 20:56Blzut3Note Added: 0018664
2017-10-29 23:53AOSPNote Added: 0018670
2017-10-30 01:21WubTheCaptainNote Added: 0018673
2017-10-30 01:21WubTheCaptainStatusneeds review => acknowledged
2017-10-30 06:00WubTheCaptainNote Added: 0018683
2017-10-30 06:00WubTheCaptainStatusacknowledged => new
2017-11-02 13:49WubTheCaptainStatusnew => feedback
2017-11-03 12:49WubTheCaptainSeverityminor => feature
2018-10-05 07:27WubTheCaptainPrioritynormal => low

Notes
(0018646)
Zalewa   
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?
(0018653)
Blzut3   
2017-10-29 18:33   
Strange that adding a search path didn't work since the documentation clearly states that it should:
Quote
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.
(0018655)
Zalewa   
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.
(0018656)
WubTheCaptain   
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.)
(0018662)
WubTheCaptain   
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.

(0018664)
Blzut3   
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.
(0018670)
AOSP   
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).
(0018673)
WubTheCaptain   
2017-10-30 01:21   
Feel free to assign to yourself if you're working on this.
(0018683)
WubTheCaptain   
2017-10-30 06:00   
Actually, do we need to do anything here? See the QT_STYLE_OVERRIDE environment variable or qt5ct.

https://wiki.archlinux.org/index.php/Uniform_look_for_Qt_and_GTK_applications [^]

Icon themes seem to be possible too.