MantisBT - Doomseeker |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0003319 | Doomseeker | [All Projects] Suggestion | public | 2017-10-29 13:54 | 2018-10-05 07:27 |
|
| Reporter | Zalewa | |
| Assigned To | | |
| Priority | low | Severity | feature | Reproducibility | N/A |
| Status | feedback | Resolution | open | |
| Platform | | OS | | OS Version | |
| Product Version | 1.1 | |
| Target Version | | Fixed in Version | | |
|
| Summary | 0003319: Themes |
| Description | 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". |
| Steps To Reproduce | |
| Additional Information | 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. |
| Tags | No tags attached. |
| Relationships | | related to | 0003320 | closed | Zalewa | Replace buddies.png (id Software / Doom) |
|
| Attached Files |  themes.patch (12,418) 2017-10-29 13:54
https://zandronum.com/tracker/file_download.php?file_id=2228&type=bug |
|
| 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 | bug_revision_view_page.php?bugnote_id=18662#r11218 |
| 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 |
|
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. |
|
|
|
|
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.) |
|
|
|
|
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). |
|
|
|
|
|
Feel free to assign to yourself if you're working on this. |
|
|
|
|
|