wangwenx190 / framelesshelper

Project moved to: https://github.com/stdware/qwindowkit Cross-platform window customization framework for Qt Widgets and Qt Quick. Supports Windows, Linux and macOS.
MIT License
846 stars 202 forks source link

[ATTENTION] The next major release - 2.0 early feedback #104

Closed wangwenx190 closed 2 years ago

wangwenx190 commented 2 years ago

Please don't hesitate to tell me your feature requests and any bugs you have found! Thanks!

wangwenx190 commented 2 years ago

@JulienMaille

wangwenx190 commented 2 years ago

To be fixed when 2.0 is out:

Detector-I commented 2 years ago

one thing that I think could help in the project structure (not in any programing way) is to separate source and include files in src and include folder, its a library after all... and other thing that I checked, looks like you removed the QMake project files in wip branch, the CMake is sure enough, but qmake is still have its worth, like for example in visual studio we can use them out of the box to convert a project to vs project... so I think having that is still good too

shujaatak commented 2 years ago

You can find some help from this amazing project too: https://github.com/hcaihao/StyleWindow

JulienMaille commented 2 years ago

@shujaatak is StyleWindow a knock-off framelesshelper? Does it have limitations too?

wangwenx190 commented 2 years ago

You can find some help from this amazing project too: https://github.com/hcaihao/StyleWindow

I’ve read it’s source code and sadly it has many issues, and compared to the wip branch it lacks many features, there’s nothing I could learn from it. I write many code to workaround Windows bugs also, but that repo doesn’t have such code either.

wangwenx190 commented 2 years ago

one thing that I think could help in the project structure (not in any programing way) is to separate source and include files in src and include folder, its a library after all... and other thing that I checked, looks like you removed the QMake project files in wip branch, the CMake is sure enough, but qmake is still have its worth, like for example in visual studio we can use them out of the box to convert a project to vs project... so I think having that is still good too

Thanks for your suggestions! Sure I can make a separate include directory, I’ll change it in next commit. About the qmake projects, it’s quite a burden to maintain two different build systems so I removed one, but I can add it back when the final structure of 2.0 is in shape

wangwenx190 commented 2 years ago

There’s thousands lines of code in this repo, I don’t think I just write them for fun, they are all important

graycatya commented 2 years ago

作者你好,我这边已根据1.x,与altairwei/2.0分支做了份独立模块。并制作了视频演示测试视频测试的qt版本是5.15.2,测试系统windows11,macos,kali linux(使用了ubuntu18打包的项目),并测试了qmake与cmake编译适配本人项目链接。在演示视频中我已标明的您的项目链接。

wangwenx190 commented 2 years ago

作者你好,我这边已根据1.x,与altairwei/2.0分支做了份独立模块。并制作了视频演示测试视频测试的qt版本是5.15.2,测试系统windows11,macos,kali linux(使用了ubuntu18打包的项目),并测试了qmake与cmake编译适配本人项目链接。在演示视频中我已标明的您的项目链接。

我前几天刷到过你的项目,看起来不错👍

开源就是要相互借鉴,相互促进,我这个项目也用了很多别人的代码。

Detector-I commented 2 years ago

one thing that I think could help in the project structure (not in any programing way) is to separate source and include files in src and include folder, its a library after all... and other thing that I checked, looks like you removed the QMake project files in wip branch, the CMake is sure enough, but qmake is still have its worth, like for example in visual studio we can use them out of the box to convert a project to vs project... so I think having that is still good too

Thanks for your suggestions! Sure I can make a separate include directory, I’ll change it in next commit. About the qmake projects, it’s quite a burden to maintain two different build systems so I removed one, but I can add it back when the final structure of 2.0 is in shape

thank you for the new changes in wip, its certainly looks better now, but still it was not what I think of... currently the most easy way to use the project is to add it as an dependency into project... but what if we want to just build it seperetly and then add then link the libs and add header..? we need to still hand pick the header from src folder... it would be much better to have an structure like this

.
├── include/
│   └── FramelessHelper/
│       ├── Quick/
│       │   ├── framelesshelperimageprovider.h
│       │   └── ...
│       └── ...
└── src/
    ├── Quick/
    │   ├── framelesshelperimageprovider.cpp
    │   └── ...
    └── ...

having the header inside the src folder, and then link them to an empty file in include folder don't make much sense... this way could be a much cleaner way then having an empty file in include folder that just include real file...

wangwenx190 commented 2 years ago

@Detector-I OK, I got your point. If you use target_link_libraries in your project, CMake will setup the libraries and include directories for you automatically, but in your case you have to do all this manually. It's not convenient enough indeed, will change in some later commit

wangwenx190 commented 2 years ago

@Detector-I OK, I got your point. If you use target_link_libraries in your project, CMake will setup the libraries and include directories for you automatically, but in your case you have to do all this manually. It's not convenient enough indeed, will change in some later commit

The project has been reorganized, it may suit your needs now. Please check again.

Detector-I commented 2 years ago

The project has been reorganized, it may suit your needs now. Please check again.

thank you very much, its much better now.

Detector-I commented 2 years ago

also one other thing, is it possible to customize the system menu like programs like chromo did? image I mean adding custom functionality... and also maybe change its color? image the photo is from notepad++

wangwenx190 commented 2 years ago

@Detector-I Adding custom menu items is possible and not hard to implement, but I’m wondering whether it’s beyond the ability of a frameless framework or not.

the color can be changed to dark indeed and I know how to do it as well, but it will influence some widgets’ appearance, for example, some applications may don’t have dark theme at all but making the menu become dark will also make some widgets become dark, the result is a user interface mixed with both light color and dark color, quite ugly

Detector-I commented 2 years ago

@Detector-I Adding custom menu items is possible and not hard to implement, but I’m wondering whether it’s beyond the ability of a frameless framework or not.

kind of right 😅 but as the framework is in task of creating the menu itself, maybe also managing it make sense...

the color can be changed to dark indeed and I know how to do it as well, but it will influence some widgets’ appearance, for example, some applications may don’t have dark theme at all but making the menu become dark will also make some widgets become dark, the result is a user interface mixed with both light color and dark color, quite ugly

its not possible to manage the color of menu separately? (maybe leave it to user to manage and change color if its needed) or is it base on the whole program?

wangwenx190 commented 2 years ago

@Detector-I About the custom system menu, it may be very useful for some users indeed, but for now it's not the central functionality of this project, I can implement this feature, but in a later time. As for the menu color, I'll re-check and tell you the result.

wangwenx190 commented 2 years ago

@Detector-I I've re-checked just now and found I was wrong, actually I can change the menu background color separately, but since the core functionality of 2.0 is not fully implemented yet, I'm going to implement the most important functionalities first. But this is an interesting feature indeed, I'll surely implement it, but later.

wangwenx190 commented 2 years ago

Note to myself and others:

The current 2.0 implemtation status:

Platform Core Widgets Quick
Windows done mostly done, in good shape now in progress
Linux in plan in plan in plan
macOS in plan in plan in plan

I'm also planning to add more demo applications

JulienMaille commented 2 years ago

I tried compiling the library today (Win11 - VS2019 - x64). I'm getting several errors due to #include <atlbase.h>

1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(788,11): error C2440: 'return': cannot convert from 'LPCTSTR' to 'LPCOLESTR'
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(788,9): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(792,11): error C2440: 'return': cannot convert from 'LPCTSTR' to 'LPCOLESTR'
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(792,9): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(798,11): error C2440: 'return': cannot convert from 'LPCOLESTR' to 'LPCTSTR'

Could be related to unicode setting.

wangwenx190 commented 2 years ago

I tried compiling the library today (Win11 - VS2019 - x64). I'm getting several errors due to #include <atlbase.h>

1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(788,11): error C2440: 'return': cannot convert from 'LPCTSTR' to 'LPCOLESTR'
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(788,9): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(792,11): error C2440: 'return': cannot convert from 'LPCTSTR' to 'LPCOLESTR'
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(792,9): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
1>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\ATLMFC\include\atlconv.h(798,11): error C2440: 'return': cannot convert from 'LPCOLESTR' to 'LPCTSTR'

Could be related to unicode setting.

Yes I have same issue before, but the latest code should be ok, maybe you didn’t pull the latest commit

JulienMaille commented 2 years ago

I manually set this and it works fine: resizing outside the window is now perfect, frame border color also work fine under Win11 !! image

But you are correct I had failed to pull latest code. Now with latest code I get this

2>D:\Qt\5-build\qt-5.15.3.kde-dynamic-msvc2019-x64\include\QtCore/qstringbuilder.h(120,1): error C2220: the following warning is treated as an error
2>D:\Qt\5-build\qt-5.15.3.kde-dynamic-msvc2019-x64\include\QtCore/qstringbuilder.h(120,1): warning C4127: conditional expression is constant
2>D:\Qt\5-build\qt-5.15.3.kde-dynamic-msvc2019-x64\include\QtCore/qstringbuilder.h(120,1): message : consider using 'if constexpr' statement instead

With suggested fix it compiles

       if constexpr (!QConcatenable< QStringBuilder<A, B> >::ExactSize && int(len) != d - start) {
            // this resize is necessary since we allocate a bit too much
            // when dealing with variable sized 8-bit encodings
            s.resize(d - start);
        }
wangwenx190 commented 2 years ago

Sadly I can’t do much about Qt ‘s own headers, maybe you can remove the /WX parameter from CMakeLists.txt

JulienMaille commented 2 years ago

Don't you have some flag forcing warning C4127 to be shown as an error?

JulienMaille commented 2 years ago

On a side note. Bug #92 is back (open menu, click title bar, then mouseover on titlebar is broken) And icons in MainWindow are twice too big image

wangwenx190 commented 2 years ago

The first issue is known but I was busy adding the Qt Quick implementation so didn’t have time to fix the details, the second issue is strange, the icons are totally normal on my side, on a 4K monitor with 200% scaling

wangwenx190 commented 2 years ago

Don't you have some flag forcing warning C4127 to be shown as an error?

I just use /WX to force all warnings as errors, so you just need to check and remove this parameter from your project

JulienMaille commented 2 years ago

I test this on a regular 1920x1080 screen, 100% scaling, Qt5.15

Utils::tryToEnableHighestDpiAwarenessLevel() gets trapped in ERROR_ACCESS_DENIED

wangwenx190 commented 2 years ago

I test this on a regular 1920x1080 screen, 100% scaling, Qt5.15

I also tested on 1080p monitors but the icon size is also normal, it may be something wrong from Qt, I need to double check

wangwenx190 commented 2 years ago

ERROR_ACCESS_DENIED is the expected behavior because we have set the dpi awareness in the manifest file so we won’t be able to change it through win32 apis, but that should not cause any problems

wangwenx190 commented 2 years ago

If you remove the call to that function, will the issue disappear?

JulienMaille commented 2 years ago

no

wangwenx190 commented 2 years ago

@JulienMaille I've re-checked the demo application on Windows 10 and the icon size is correct as usual. It's also correct on Windows 11 as well. Maybe there's something different with your own copy of this repo.

JulienMaille commented 2 years ago

Can you explain what does the manifest brings that could not be done before? (I'm looking at how I can embed that manifest from a .pro file)

wangwenx190 commented 2 years ago

We have set the DPI awareness level in the manifest file in the following two lines:

https://github.com/wangwenx190/framelesshelper/blob/d4e711d679acee0605e72b6e545a8a2e4b816d7e/examples/example.manifest#L30-L31

We set both of dpiAware and dpiAwareness to cover all Windows versions (from Windows Vista to Windows 11).

But once we have done this, we won't be able to modify the DPI awareness level programatically, any attempts of changing them will always get the ACCESS_DENIED error.

(I'm looking at how I can embed that manifest from a .pro file)

I see you are using Qt 5.15.3, there are two ways to embed manifest file. One is add QMAKE_MANIFEST = ... to your pro file, but you'll have to make sure the file path you give to that variable is absolute path, not relative path. The other way is adding an rc file and embed the manifest file directly in the rc file. The latter is supported on all Qt versions, the former is only supported on Qt 5.15 and newer

wangwenx190 commented 2 years ago

Can you explain what does the manifest brings that could not be done before?

Do you mean the necessity of the manifest file? It's not necessary actually, but as I explained above, it can save us some API calls and have good compatibility cross many Windows versions. Since Qt only support the PMv2 DPI awareness level since Qt 6.2, using the manifest file is quite convenient to add PMv2 support for Qt versions older than 6.2. I also provide a helper function Utils::tryToEnableHighestDpiAwarenessLevel() to achieve what we do through the manifest file. If you find it hard to embed the manifest file into your executable, you can use the helper function instead. They can achieve exactly the same thing.

wangwenx190 commented 2 years ago

Qt internally will also set the DPI awareness level, but it's not raised to PMv2 until 6.2. It also means the manifest file is also no longer needed since Qt 6.2. But leaving the manifest file there also won't bring any harm, it just become useless. And in fact setting the DPI awareness level is not the only function of manifest file, it has a lot more interesting options, please refer to the official documentation https://docs.microsoft.com/en-us/windows/win32/sbscs/application-manifests for more information.

JulienMaille commented 2 years ago

The project has become quite complex with .h spread across source and include. What do I need to add to my pro in SOURCE/HEADERS if I want to compile core and widget statically?

wangwenx190 commented 2 years ago

The project has become quite complex with .h spread across source and include. What do I need to add to my pro in SOURCE/HEADERS if I want to compile core and widget statically?

If you are using this repo as an external library, there's a CMake switch FRAMELESSHELPER_BUILD_STATIC to control this. If you are copying the source files to your project directly, just leave the src/quick folder, all you need is the src/core and src/widgets folder, and the real header files are in include, you can copy the header files to the corresponding source folder (replace the files with same name). To build statically, just add FRAMELESSHELPER_CORE_STATIC and FRAMELESSHELPER_WIDGETS_STATIC to your global definitions.

JulienMaille commented 2 years ago

This is my adapted pro but I'm getting include errors

QT += gui-private

HEADERS += $$PWD/include/FramelessHelper/Core/Utils   \
           $$PWD/include/FramelessHelper/Core/Global  \
           $$PWD/include/FramelessHelper/Core/FramelessWindowsManager  \
           $$PWD/include/FramelessHelper/Core/FramelessHelper_Windows \
           $$PWD/include/FramelessHelper/Core/FramelessHelper_Win \
           $$PWD/include/FramelessHelper/Core/FramelessHelper_Qt
SOURCES += $$PWD/src/core/framelesshelper_qt.cpp       \
           $$PWD/src/core/framelesshelper_win.cpp      \
           $$PWD/src/core/qwinregistry.cpp             \
           $$PWD/src/core/utils.cpp                    \
           $$PWD/src/core/utils_win.cpp

FORMS += $$PWD/TitleBar.ui

DEFINES += FRAMELESSHELPER_CORE_STATIC FRAMELESSHELPER_WIDGETS_STATIC

qtHaveModule(widgets) {
    HEADERS += $$PWD/include/FramelessHelper/Widgets/Global \
               $$PWD/include/FramelessHelper/Widgets/FramelessWidgetsHelper \
               $$PWD/include/FramelessHelper/Widgets/FramelessWidget        \
               $$PWD/include/FramelessHelper/Widgets/FramelessMainWindow
    SOURCES += $$PWD/src/widgets/framelessmainwindow.cpp         \
               $$PWD/src/widgets/framelesswidget.cpp             \
               $$PWD/src/widgets/framelesswidgetshelper.cpp
}

win32 {
    #LIBS += -luser32 -lshell32
}
JulienMaille commented 2 years ago

attempt n°2

QT += gui-private

INCLUDEPATH += $$PWD/include/FramelessHelper/Core/
HEADERS += $$files($$PWD/include/FramelessHelper/Core/*.h)
SOURCES += $$files($$PWD/src/core/*.cpp)

FORMS += $$PWD/TitleBar.ui

DEFINES += FRAMELESSHELPER_CORE_STATIC FRAMELESSHELPER_WIDGETS_STATIC

qtHaveModule(widgets) {
    INCLUDEPATH += $$PWD/include/FramelessHelper/Widgets/
    HEADERS += $$files($$PWD/include/FramelessHelper/Widgets/*.h)
    SOURCES += $$files($$PWD/src/widgets/*.cpp)
}
wangwenx190 commented 2 years ago

I have three suggestions:

  1. There are also same name header files in the source file folder, add them to your HEADERS as well
  2. Change the non-suffixed header files of your HEADERS to the real header files, for example: $$PWD/include/FramelessHelper/Core/Utils --> $$PWD/include/FramelessHelper/Core/utils.h
  3. Add include/FramelessHelper/Core and include/FramelessHelper/Widgets folder to your INCLUDEPATH: INCLUDEPATH += $$PWD/include/FramelessHelper/Core $$PWD/include/FramelessHelper/Widgets
wangwenx190 commented 2 years ago

The second attempt looks much better, maybe adding HEADERS += $$files($$PWD/src/widgets/*.h) will be even better

JulienMaille commented 2 years ago

I'm almost there, I'm getting 1>D:\Dev\MainWindow.cpp(3714,5): error C2653: 'FrameLessMainWindow': is not a class or namespace name in

void MainWindow::changeEvent(QEvent *event)
{
    if( event->type() == QEvent::LanguageChange )
        ui->retranslateUi(this); // Imperfect translation but better than nothing
#if FRAMELESS_WINDOW
    FrameLessMainWindow::changeEvent(event);
#else
    QMainWindow::changeEvent(event);
#endif
}
wangwenx190 commented 2 years ago

Try FRAMELESSHELPER_PREPEND_NAMESPACE(FramelessMainWindow)::changeEvent(event);? Or add one line FRAMELESSHELPER_USE_NAMESPACE on the top of your source file?

JulienMaille commented 2 years ago

Got it, I'm now having a very weird problem with a qt class

1>D:\Qt\5-build\qt-5.15.3-msvc2019-x64\include\QtCore\quuid.h(48,16): error C2143: syntax error: missing ';' before '<class-head>' (compiling source file main.cpp)
1>D:\Qt\5-build\qt-5.15.3-msvc2019-x64\include\QtCore\quuid.h(49,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int (compiling source file main.cpp)

image

JulienMaille commented 2 years ago

Is LIBS += -luser32 -lshell32 not needed anymore?

wangwenx190 commented 2 years ago

Got it, I'm now having a very weird problem with a qt class

Maybe caused by some magic in framelesshelper_windows.h? I could not imagine it comes from elsewhere

Is LIBS += -luser32 -lshell32 not needed anymore?

I'm not sure about QMake. For CMake they are always added the linked libraries by default. You can try to remove it to see if it still compile.

JulienMaille commented 2 years ago

I'm sorry to bother you with all my issues. I moved the include later and the quiid issue is gone. I'm still having a link issue

1>utils.obj : error LNK2019: unresolved external symbol "int __cdecl qInitResources_framelesshelpercore(void)" (?qInitResources_framelesshelpercore@@YAHXZ) referenced in function "void __cdecl initResource(void)" (?initResource@@YAXXZ)
1>bin_debug\\Test_debug.exe : fatal error LNK1120: 1 unresolved externals