Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc/"Adding UI Controls" example does not work #364

Closed
wants to merge 1 commit into from

Conversation

lgtmak
Copy link
Contributor

@lgtmak lgtmak commented Jan 12, 2024

In chapter 3 of the docs Writing your first application, subsection Adding UI Controls the example main code does not work.

Example from docs:

#include <QskWindow.h>
#include <QskLinearBox.h>
#include <QskPushButton.h>

#include <QGuiApplication>

int main( int argc, char* argv[] )
{
    QGuiApplication app( argc, argv );

    auto* horizontalBox = new QskLinearBox( Qt::Horizontal );
    auto* button1 = new QskPushButton( "button 1", horizontalBox );
    auto* button2 = new QskPushButton( "button 2", horizontalBox );

    QskWindow window;
    window.addItem( horizontalBox );
    window.show();

    return app.exec();
}

This renders an empty window on X11 and on Wayland.

By adding a call to setPanel( true ); on the QskLinearBox the example will render the two buttons.

Is this necessary or is there a clearer simple example to demonstrate a basic QSkinny app?

@lgtmak lgtmak force-pushed the doc/button-example-fix branch from aacbec6 to 3f9fa56 Compare January 12, 2024 05:43
@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

As a first step I removed Qt/Widgets from the example as it gives the impression that QSkinny is anyhow related to it.

Today I would consider Qt5 being a corner case and I would prefer to have the docs for Qt6. I already found confusion in this area here: https://forum.qt.io/topic/152476/qskinny

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

I do not see that the setPanel should be necessary and why it should have any impact - beside of additional margins, that will affect the layout of the buttons.

Could you please double check, that the setPanel call that makes the code working in your environment ? Could you have an eye on the loading of the skin plugins - maybe something went wrong when you did the test without setPanel. Maybe have a look at QskPushButtonSkinlet::updateSubNode to see if any scenegraph nodes are created ?

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

@lgtmak @vrcomputing I'm wondering if QSK_QT_VERSION was the right solution - isn't using CMAKE_PREFIX_PATH superior ?

Chosing Qt 5.15, on a system where several Qt versions are installed can be done ( on my box ):
- cmake /opt/qskinny/qsk1 -DCMAKE_PREFIX_PATH=/opt/qt/Qt-5.15

For testing the Qt 6.7 Beta:
- cmake /opt/qskinny/qsk1 -DCMAKE_PREFIX_PATH=/opt/qt/Qt-6.7-Beta1

QSK_QT_VERSION expresses a preference for Qt 5/6 without giving any hint how to find the package. CMAKE_PREFIX_PATH not only allows a specific preference inside of the Qt6 series - it also provides the information where this preference can be found ?

@vrcomputing
Copy link
Contributor

@uwerat @lgtmak as I mentioned in my comment I'm sceptical that we (QSkinny) should add yet another way of configuring Qt CMake. Subjectively I think, if the user wants to build QSkinny with a specific version then he should provide its specific location. As @uwerat described its scenario, what if e.g. Qt versions 5.15, 5.12, 6.0, 6.6, ... are installed on one system? What would be the expected used major and minor version?

@lgtmak could you please check whether one of the following approaches works in your scenario?:

  1. cmake -DQt<5|6>_DIR:PATH= ...
    e.g. on Windows for Qt5: cmake -DQt5_DIR=C:\Qt\5.15.2\msvc2019_64\lib\cmake\Qt5
  2. cmake -DCMAKE_PREFIX_PATH:PATH= ...
    e.g. on Windows for Qt5: cmake -DCMAKE_PREFIX_PATH=C:\Qt\5.15.2\msvc2019_64

This is how I integrate Qt into CMake on Windows though

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

A couple of weeks before I was in contact with Miguel who was asking about the WebAssembly backend. My answer was maybe as I never tried it myself and I have no clue about the specifics of the platform. So Miguel was giving it a try to find out.

Miguel is familiar with qmake but new to cmake and it took him weeks to pass this hurdle. My takeaway from this was, that our build system ( or documentation ) is not good enough to guide users through the installation process. Attached is a screenshot he had sent to me
cmake

I don't know what he explicitly did to get there, but the information given is absolutely not appropriate for indicating, that he didn't specify the Qt installation properly and the cmake heuristics did not find one.

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

This is how I integrate Qt into CMake on Windows though

In my environment I'm using the cmake heuristics for the PATH variable.

However Qt6 comes with a shell script qt-cmake ( https://doc.qt.io/qt-6/cmake-build-on-cmdline.html#qt-cmake ). It has a similar effect like qmake: calling /opt/qt/Qt-6.5.1/bin/qt-cmake makes /opt/qt/Qt-6.5.1 the default Qt installation.

Actually this would be my recommendation for all users that are looking for a Qt6 build ?

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 13, 2024

That sounds fine, I can run some tests with some cmake vars and get back to you on what works. My main issue is that I should be able to run a Cmake build command and not specify a specific Qt version, it should always pick the latest I have installed. If I have Qt6.6 and Qt6.1 and Qt5.3 and Qt4.8 or whatever installed and I want to use Qt5.3 then I would need a Cmake build variable to specify otherwise it would use Qt6.6, hence my addition in the previous request. If you want my 2-cents, you will get more developers using QSkinny the faster I can get up and running without having to specify specific build variables, a simple cmake ../ && make or cmake -b build <BLAH BLAH BLAH> && make or whatever simple command should be enough and my Cmake file for my project handles the rest.

I think it's pretty safe to say that developers will be building against whatever latest Qt version they have installed as that is the advice of Qt itself is to use the latest, anything else can be configured usually in a script.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 13, 2024

As a first step I removed Qt/Widgets from the example as it gives the impression that QSkinny is anyhow related to it.

Today I would consider Qt5 being a corner case and I would prefer to have the docs for Qt6. I already found confusion in this area here: https://forum.qt.io/topic/152476/qskinny

I have built against Qt6 without issue but I had to call setPanel to get the buttons to show from the docs example. Developers will copy and paste the example code into a fresh project which should include at minimum 2 files: CMakeLists.txt and main.cpp. So you should be able to replicate the issue I'm experience as well with the simple existing tutorials. For the sake of brevity I have attached the two files I'm using to test. You should be able to make a build dir cd into the build dir and run cmake ../ make to see the issue.

If you add horizontalBox->setPanel( true ); to L12 it will then display the buttons after build.
qsk_example.tar.gz

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

That sounds fine, I can run some tests with some cmake vars and get back to you on what works.

Great.

I would also be interested in what happens when using the Qt5/6 packages from the your distro. These packages usually do not install all files of a Qt Version below a specific root directory. F.e on OpenSuse libs go straight to /usr/lib64/libQt6Quick.so.6

My main issue is that I should be able to run a Cmake build command and not specify a specific Qt version, it should always pick the latest I have installed.

My impression was that you can't select Qt6.6 over Qt6.1 today. In my environment I needed to add .../Qt6.6/bin to my PATH to trigger a cmake build-in heuristic.

Let's say someone has an outdated Qt6 package as dependency because of using KDE. However for development (s)he wants to use a more recent Qt6 release he has taken from the download server of the Qt company. Guess the distro adjusts the PATH for its package and - without additional help - cmake will always pick this one ?

If you want my 2-cents, you will get more developers using QSkinny the faster I can get up and running without having to specify specific build variables, a simple cmake ../ && make or cmake -b build <BLAH BLAH BLAH> && make or whatever simple command should be enough and my Cmake file for my project handles the rest.

This is why I was thinking about using qt-cmake:

  • typos will immediately be reported with a "command not found"
  • using the path of command is something users are used from qmake
  • very explicit and no cmake magic

qt-cmake does not exist for Qt5 but IMHO Qt5 is already a corner case and if someone wants to use it (s)he has to learn about using CMAKE_PREFIX_PATH.

I think it's pretty safe to say that developers will be building against whatever latest Qt version they have installed as that is the advice of Qt itself is to use the latest, anything else can be configured usually in a script.

My guess is, that the majority will check what QSkinny is and if it is worth to have a second look at it. Those users probably don't care at all - as long as it is a supported Qt6 version. When doing the second look things will change, but then they are willing to learn about more specific builds.

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

I have built against Qt6 without issue but I had to call setPanel to get the buttons to show from the docs example.

This should be a bug and I would prefer to understand and fix it - instead of adding a workaround in the example code.

I was copying your code over playground/glabels/main.cpp. This was leading to an assertion ( debug build ), as no skin plugin had been loaded. To solve this problem I had to add #include <SkinnyNamespace.h> and Skinny::init(); as first statement in main. After these modifications I did not have the described problem.

SkinnyNamespace.h is a library that is made for the examples only and is not intended to be installed. So when using an installed QSkinny we need to load at least one plugin factory so that something becomes visible. However I do not have clue so far how the setPanel call could be related to this. When there is no skin it should have no effect.

So there are 2 issues to be done:

  • Loading the skin in the example. Unfortunately we enter the issues with finding Qt Plugins, what is the reason for so many problems on the user side and can't be explained in one line.
  • What the heck is going on in your environment.

For trying to fix the bug: could you please check if your problem is still there when running your code like I did. If yes we could at least exclude this nasty plugin hurdles.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 13, 2024

I have built against Qt6 without issue but I had to call setPanel to get the buttons to show from the docs example.

This should be a bug and I would prefer to understand and fix it - instead of adding a workaround in the example code.

For the record I was executing the binary after build with QT_PLUGIN_PATH=/usr/local/lib/qskinny/plugins ./myProj

@uwerat
Copy link
Owner

uwerat commented Jan 13, 2024

However I do not have clue so far how the setPanel call could be related to this.

Ah - the Fluent2 design system works with transparencies and when you do not have a control, that is painted with the base color as the bottom layer, it might end up like being described. Then your workaround would actually be a correct fix - but one that needs to be explained.

The material3 system does not use semi transparencies and the problem is probably gone, when using it.
I added #include <SkinnyShortcut.h> and SkinnyShortcut::enable( SkinnyShortcut::AllShortcuts ); after the QGuiApplication line. Now when rotating the skins with CTRL-S I can confirm your observation for one ( probably Fluent2::Dark ) skin.

Think the problem is understood and no further investigation is needed from your side. Thanks for bringing it up.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 13, 2024

I am happy to update the docs to reflect this. Perhaps it would be a good idea to introduce the skins in the “Writing your first application” guide. This way a new user would have an idea of how to use the skins and any outstanding gotchas could be briefly mentioned.

Does QSkinny default to Fluent2 on all platforms?

@uwerat
Copy link
Owner

uwerat commented Jan 14, 2024

Does QSkinny default to Fluent2 on all platforms?

Should be the first of the found skins in alphabetical order - so quite random.

Today the application creates a flat list of skins from the skin factories. But this is not good enough to cover the concept of design systems and the light/dark color schemes. Until this is not done it is hard to initialize/adjust the application with something that makes sense.

Fluent2 should be the default for Windows, Material3 for Android. However we do not have anything appropriate for the Linux Desktops or the Mac so far. Not 100% sure if it makes sense to implement the Fusion style - it would be easy to do and we would have at least something "established". The existing "Squiek" style has some value for testing but will be removed before the first release of QSkinny.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 14, 2024

I think targeting Fusion would be ideal, that way it’s in sync with Qt to some extent. I also see that QApplication has a standard palette that is relative to user system design that could be applied, but am unsure of the difficulty. I need to familiarize myself with Qskinny skins vs Qt themes.

@uwerat
Copy link
Owner

uwerat commented Jan 15, 2024

I think targeting Fusion would be ideal, that way it’s in sync with Qt to some extent.

Actually I would prefer to have a modern KDE style.

To me the Fusion style is kind of an alien on all platforms - its main intention seems to be to reduce the workload on Qt development side. Nonetheless I see arguments for implementing it:

  • we can find its specs in the Qt implementation
  • it is available for Qt/Widgets, where we have more type of widgets than what is available with QC2.
  • it allows to compare ( f.e memory footprint ) applications being done with QC2 and QSkinny
  • we can't be blamed for its ugliness

I also see that QApplication has a standard palette that is relative to user system design that could be applied, but am unsure of the difficulty. I need to familiarize myself with Qskinny skins vs Qt themes.

I do not want to go too deep into this topic, but at least one note: QSkinny does scene graph node composition, while QC2 does item composition - each of it having its own palette. That has the consequence, that a Qsk control needs way more color/metric definitions than those that can be stored in only one QPalette.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 15, 2024

If the goal is to look modern on Linux with some sort of relativity to a recognizable style without reinventing the wheel and trying to utilize Qt components, there is also the Imagine style in QQuickControls. It looks pretty modern generic at base but would allow for high customizability due to it’s simplicity.

https://doc.qt.io/qt-6/qtquickcontrols-imagine.html

I am not sure how adaptable it is from that system however.

@uwerat
Copy link
Owner

uwerat commented Jan 16, 2024

If the goal is to look modern on Linux with some sort of relativity to a recognizable style without reinventing the wheel

QSkinny is reinventing the wheel. The not very hidden statement between the lines is the author believes that it is better to create a system from scratch than trying to work with the existing one. It is always arguable if certain design decisions are indeed better, but whenever QSkinny takes a different direction it is intentional.

and trying to utilize Qt components, there is also the Imagine style in QQuickControls. It looks pretty modern generic at base but would allow for high customizability due to it’s simplicity.

Copying raster graphics might be a solution for specific use cases where you are in control of every size of every control on the screen. However I would consider it a nogo when layouts depend on factors like screen sizes, i18n etc.

You often find specifications for all metrics/colors of certain design systems at figma. F.e https://www.figma.com/file/NAWMapFlXnoOb86Q2H5GKr/Windows-UI-(Community). The main problem with applying them is that every design system covers a subset of the existing controls only and you need to be "creative" with what to do for the others.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 16, 2024

The main problem with applying them is that every design system covers a subset of the existing controls only and you need to be "creative" with what to do for the others.

I am not trying to derail the conversation but I believe this is definitely true. The cousin of QSkinny (imo), Kirigami seems to be going through a similar state right now where the design aspect of the framework originally had a unique tone but in the pursuit of covering gaps in Qt QuickControls has found itself adopting the Fusion theme and producing missing common components. The following thread is a good read about that but this specific comment sums it up nicely: https://discuss.kde.org/t/we-need-your-thoughts-on-kirigami/6115/6

I think one solution is to ensure a base library common controls and a superset library that captures the unique design. I imagine there are other good solutions as you define the required criteria at project level and documentation level.

@lgtmak
Copy link
Contributor Author

lgtmak commented Jan 19, 2024

I see this was fixed at some point as well so there seems to be no need for this PR either......

@lgtmak lgtmak closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants