[PR #3917] Remove initial hardcoded styling of taskview and taskheader

Merged, abandoned or rejected pull requests are moved here to clear the main Pull Requests forum.
Post Reply
User avatar
Chrismettal
Posts: 43
Joined: Fri Sep 18, 2020 11:44 am
Location: Germany
Contact:

[PR #3917] Remove initial hardcoded styling of taskview and taskheader

Post by Chrismettal »

Original description of this PR:
https://github.com/FreeCAD/FreeCAD/pull/3917
When the taskview is drawn for the first time without changing the workbench (like having PartDesign or Robot as your launch workbench) it would be drawn with hardcoded styling from the original Qsint stuff.
FreeCAD_MINdRecSim.png
FreeCAD_MINdRecSim.png (44.88 KiB) Viewed 11603 times
The colors would be correct when switching between workbenches or re-applying a stylesheet.
Removing all calls of setStylesSheet() for the initial draw of taskheader and taskview makes it load with the correct styling:
after.png
after.png (5.85 KiB) Viewed 11603 times
I was able to completely delete the TaskGroup::setScheme method, which only set the stylesheet, but the TaskHeader::setScheme method is still required as FreeCAD does not load with it removed, so only the setStyleSheet() call was removed here. I would have liked to remove all hardcoded initial stylings but right now i can't get it to work.

I could not find an existing issue ticket for this.
So while i was able to make the erroneus styling disappear i am not fully satisfied with cleaning up the now unused code. Is the QSint initial styling neccessary at all?
After this change we shouldn't be touching the "ActionBoxStyle" hardcoded stylesheets in actionbox.cpp / macpanelscheme.cpp etc. which are only used for the initial default stylings.

Taskgroup_p.cpp only ever set this hardcoded stylesheet in its setScheme() so this call and function could be removed completely:
Code_vyP7OuzX6D.png
Code_vyP7OuzX6D.png (272.13 KiB) Viewed 11603 times
Taskheader_p.cpp does more in its SetScheme() so i wasn't able to remove it completely:
Code_F0jK4lfhuH.png
Code_F0jK4lfhuH.png (102.36 KiB) Viewed 11603 times
This means Taskheader_p.cpp still pulls the headerSize from defaultScheme() which traces back to hardcoded QSint stuff like actionpanelscheme.cpp where headerSize is hardcoded again.

I take it it isn't possible to put the headerSize etc. in our stylesheets?
If it's just the header size that gets pulled from defaultScheme at this position i might be able to at least hardcode it instead of the taskheader_p.cpp setScheme() call so we might be able to remove more of the hardcoded qsint styles, but i don't know if it is even preferable to remove that code? Do we need that as a fallback or for future new windows?

Sorry for sounding confusing and/or confused. I am still very much wrapping my head around the FreeCAD source. Picking smaller bugs (or what i think are smaller bugs) to learn how to navigate FreeCAD.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: [PR #3917] Remove initial hardcoded styling of taskview and taskheader

Post by wmayer »

Is the QSint initial styling neccessary at all?
Yes, it is. QSint provides some platform-dependent schemes and they are needed to provide the correct look & feel on these platforms. Changing the QSint code by removing the functions may break this mechanism.

QSint's schemes technically are based on Qt's style sheets and they would collide when setting an application-wide style sheet file. In order to avoid such kind of problems we added some special event handling to clear the style sheet of the QSint widgets when the user selects one of the provided style sheet files.

The fact that this worked correctly in FreeCAD v0.18 shows that it's not a QSint problem but somewhere a regression in the FreeCAD code.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: [PR #3917] Remove initial hardcoded styling of taskview and taskheader

Post by wmayer »

Since v0.18 there were a couple of code refactorings where setting the style sheet has been moved to a central function:
https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L2286

At the end it calls the unpolish() function

Code: Select all

    if (mdi->style())
        mdi->style()->unpolish(qApp);
and exactly this seems to be the problem. When removing these two lines for testting it works as expected.

Here the documentation of the function: https://doc.qt.io/qt-5/qstyle.html#unpolish-1
Uninitialize the given widget's appearance.

This function is the counterpart to polish(). It is called for every polished widget whenever the style is dynamically changed; the former style has to unpolish its settings before the new style can polish them again.

Note that unpolish() will only be called if the widget is destroyed. This can cause problems in some cases, e.g, if you remove a widget from the UI, cache it, and then reinsert it after the style has changed; some of Qt's classes cache their widgets.
To me it seems that the last paragraph is related to the issue.

Now, we still need to call unpolish() when changing the style sheet at runtime because otherwise some widgets may be rendered incorrectly.
So, the easiest solution is to avoid to call it at startup time.
User avatar
Chrismettal
Posts: 43
Joined: Fri Sep 18, 2020 11:44 am
Location: Germany
Contact:

Re: [PR #3917] Remove initial hardcoded styling of taskview and taskheader

Post by Chrismettal »

Thank you for your explanation!

I never even checked if this worked in 0.18 as i always hat the "Start" workbench selected at bootup there and then the problem didn't appear. Should do that next time.
Definitely learning a lot here. Hopefully my future PRs won't cause you more work than help.
Jee-Bee
Veteran
Posts: 2566
Joined: Tue Jun 16, 2015 10:32 am
Location: Netherlands

Re: [PR #3917] Remove initial hardcoded styling of taskview and taskheader

Post by Jee-Bee »

The start work bench was in 0.16 already the default workbench where FC start with ;)
Post Reply