On Fri, 7 Jul 2023 at 10:13, Yu Jin <technikma...@gmail.com> wrote:

> Am Fr., 7. Juli 2023 um 04:37 Uhr schrieb Thibaut Cuvelier:
>
>> I'm having an issue with your patch: QStyle::name has only been added in
>> Qt 6.1 (https://doc.qt.io/qt-6/qstyle.html#name). I believe
>> QObject::objectName should do the trick too.
>>
>> As I understand your problem, in LyX.cpp, LyX::exec first creates a
>> GuiApplication (line 358) before initialising LyX (line 366). If I do the
>> initialisation in GuiApplication::exec instead, I don't get any crash;
>> actually, your patch works :)!
>> I have added a similar line in PrefInput::applyRC so that the style
>> change applies immediately when clicking Apply in the Preferences window.
>>
>
> I think it is actually better to do that in PrefUserInterface::applyRC for
> affiliation.
>

To be honest, I took a place that might match and where the code worked. I
tried to understand the differences between these functions, but I couldn't
get it. A bit of documentation would have helped


> I'm attaching the corresponding patch.
>>
>
> I didn't understand this part:
> @@ -718,13 +718,13 @@ GuiView::GuiView(int id)
>   connect(zoom_value_, SIGNAL(pressed()), this,
> SLOT(showZoomContextMenu()));
>   // zoom_value_->setPalette(palette);
>   zoom_value_->setForegroundRole(statusBar()->foregroundRole());
>   zoom_value_->setFixedHeight(fm.height());
>  #if (QT_VERSION >= QT_VERSION_CHECK(5, 11, 0))
> - zoom_value_->setMinimumWidth(fm.horizontalAdvance("444\%"));
> + zoom_value_->setMinimumWidth(fm.horizontalAdvance("444%"));
>  #else
> - zoom_value_->setMinimumWidth(fm.width("444\%"));
> + zoom_value_->setMinimumWidth(fm.width("444%"));
>  #endif
>   zoom_value_->setAlignment(Qt::AlignCenter);
>   zoom_value_->setText(toqstr(bformat(_("[[ZOOM]]%1$d%"), zoom)));
>   statusBar()->addPermanentWidget(zoom_value_);
>   zoom_value_->setEnabled(currentBufferView());
>

It's just fixing a few compiler warnings, it doesn't have to be in the same
patch.


> Side question: why is createApplication declared in Application.h but
>> defined in GuiApplication.cpp?
>>
>>
>> On Fri, 7 Jul 2023 at 02:23, Thibaut Cuvelier wrote:
>>
>>> Can't you use setStyle later on, once LyXRC is read?
>>>
>>
> You could, like in your patch, but this creates another problem (see below)
>
>
>> I suspect it should be enough to apply the style to the whole
>>> application, based on the source code (
>>> https://github.com/qt/qtbase/blob/dev/src/widgets/kernel/qapplication.cpp#L971),
>>> as I understand that GuiApplication is the root widget for the whole LyX.
>>>
>>> Also, I believe that LyXRC shouldn't take precedence over CLI arguments
>>> (-style windows, for instance); does Qt implement this itself or do we have
>>> to check in QCoreApplication::arguments if a style is given?
>>>
>>
> I believe the way would be to set the style *before* the QApplication is
> created, then if any style is given in CLI arguments, QApplication uses
> that over the one previously set.
>
> I don't know, to me it seems we *need* to know style setting from lyxrc
> before we create the gui app
> frontend::GuiApplication * guiApp = new frontend::GuiApplication(argc,
> argv);
> which is not given.
> otherwise we would need to check cli arguments manually for "-style" and I
> don't want to do that, if Qt decides to change those in the future we would
> have to put more work into it.
> What can be done is calling readRcFile("preferences", true) before
> CreateApplication, like I have done here in the attached patch, but I don't
> know if that is ok or not, it seems to work, but I can't assess it 100%,
> any ideas/objections?
>

Qt hasn't changed its -style parameter at least since Qt 4 (2005), that's
quite stable (there is also an environment variable, QT_STYLE_OVERRIDE). I
couldn't find any way to tell if the style was set from the command line or
not (or to get the default style for the current platform), so we would
always need to check manually whether there has been some CLI-set style,
whatever path we end up taking. (There is some precedent with locale
handling, where Qt is trying hard to impose its locale, it seems.)
(More details: the -style parameter is dealt with in
QGuiApplicationPrivate, not accessible from LyX and not available anywhere
in the API, as far as I can tell.)

As Qt can dynamically change the style at runtime, why would you need to
know it when instantiating GuiApplication? Anyway, in my patch, the style
is set before calling QApplication::exec, before any user interaction can
take place, the user should not see any difference in delay before
interacting with LyX.

I have no opinion on whether the LyXRC file should be read before or after
createApplication, I have no idea about the trade-offs here. What do others
think about it? This change may impact other platforms too.
Anyway, it works as expected.


> Also note that in the patch I have set "fusion" to default on Windows,
> other OSes can do it the same way if they want.
>

Fusion doesn't look native at all on any platform by design
<https://www.qt.io/blog/2012/10/30/cleaning-up-styles-in-qt5-and-adding-fusion>,
but it's the only not-too-terrible choice for dark themes on Windows :/. I
would do either of the following: let Qt handle the default theme or only
impose the strange Fusion style on users when we know it's the least worst
for users. The idea is to have a good default for users, with LyX being as
native-looking as possible --- a goal that is currently achieved, albeit
the support for dark themes on Windows is really subpar (like many Windows
apps).

Actually, I'd be OK with merging this patch with a change in the way the
default theme is selected.
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel

Reply via email to