OlivierLDff / Qaterial

🧩 Collection of Material Components based on QtQuickControls2.
https://olivierldff.github.io/Qaterial/
MIT License
300 stars 57 forks source link

Fix when QATERIAL_BUILD_SHARED=ON, build failed. #107

Closed pragmaticQt closed 3 years ago

pragmaticQt commented 3 years ago

NOTE: Dependencies ( QOLM and SPDLOG )_BUILD_SHARED=ON too

OlivierLDff commented 3 years ago

Thanks for the contribution, is this PR ready? I've never used as shared library, but isn't Q_COREAPP_STARTUP_FUNCTION for shared library? I think there is an error, and there should be only one Q_COREAPP_STARTUP_FUNCTION per dll.

So maybe we should add Q_COREAPP_STARTUP_FUNCTION(Qaterial_startup) that call the qml register function, and the load ressources one? With this, registerQmlTypes & loadQmlResources don't need to be exposed i think. What is your opinion?

pragmaticQt commented 3 years ago

Thanks for the contribution, is this PR ready? I've never used as shared library, but isn't Q_COREAPP_STARTUP_FUNCTION for shared library? I think there is an error, and there should be only one Q_COREAPP_STARTUP_FUNCTION per dll.

So maybe we should add Q_COREAPP_STARTUP_FUNCTION(Qaterial_startup) that call the qml register function, and the load ressources one? With this, registerQmlTypes & loadQmlResources don't need to be exposed i think. What is your opinion?

Hi Olivier, so happy to see your reply.

I agree that combining 2 calls as one Qaterial_startup makes the user's life easier and not hard to implement too.

But its con is losing some flexibility if not available e.g. loadQmlResources when "not using Qaterial as your main qt quick control style. Set autoRegisterStyle to false."

My advice is to let the user decide to whether use it or not and how.

Furthermore, I have another idea to build the whole as a plugin like Qt does and working on it now. So a client will not need to build it themselves and also with qmltypes info which helps the Qt creator code completion.

The problem here is Qaterial can't be built on Windows as a dll now which I'm trying to resolve. Following is how to reproduce it. on Windows

  1. Clone the Qaterial master branch
  2. Qt creator -> Projects -> Build -> Filter, shared ->check mark for QATERIAL_BUILD_SHARED
  3. Apply configuration changes
  4. Run Cmake
  5. Build (failed)

There will be a lot of warnings like

CMakeFiles/Qaterial.dir/cmake_pch.hxx /showIncludes /FoCMakeFiles\Qaterial.dir\Qaterial_icons\Qaterial\Icons.cpp.obj /FdCMakeFiles\Qaterial.dir\ /FS -c Qaterial_icons\Qaterial\Icons.cpp Qaterial_autogen\include\moc_Icons.cpp(14616): warning C4273: 'qaterial::Icons::qt_static_metacall': inconsistent dll linkage Qaterial_icons\Qaterial/Icons.hpp(16): note: see previous definition of 'qt_static_metacall' Qaterial_autogen\include\moc_Icons.cpp(20710): warning C4273: 'staticMetaObject': inconsistent dll linkage Qaterial_icons\Qaterial/Icons.hpp(16): note: see previous definition of 'public: static QMetaObject const qaterial::Icons::staticMetaObject' Qaterial_autogen\include\moc_Icons.cpp(20710): error C2491: 'qaterial::Icons::staticMetaObject': definition of dllimport static data member not allowed Qaterial_autogen\include\moc_Icons.cpp(20721): warning C4273: 'qaterial::Icons::metaObject': inconsistent dll linkage Qaterial_icons\Qaterial/Icons.hpp(16): note: see previous definition of 'metaObject' Qaterial_autogen\include\moc_Icons.cpp(20726): warning C4273: 'qaterial::Icons::qt_metacast': inconsistent dll linkage Qaterial_icons\Qaterial/Icons.hpp(16): note: see previous definition of 'qt_metacast' Qaterial_autogen\include\moc_Icons.cpp(20734): warning C4273: 'qaterial::Icons::qt_metacall': inconsistent dll linkage Qaterial_icons\Qaterial/Icons.hpp(16): note: see previous definition of 'qt_metacall' ninja: build stopped: subcommand failed. 21:00:58: The process "C:\Program Files\CMake\bin\cmake.exe" exited with code 1. Error while building/deploying project Qaterial (kit: QtVS2019) When executing step "Build" 21:00:58: Elapsed time: 00:07.

OlivierLDff commented 3 years ago

So we should remove completly Q_COREAPP_STARTUP_FUNCTION stuff before merging? I think you are right, for the users it is cool to have control on what is hapenning.

pragmaticQt commented 3 years ago

So we should remove completly Q_COREAPP_STARTUP_FUNCTION stuff before merging? I think you are right, for the users it is cool to have control on what is hapenning.

I haven't implement Q_COREAPP_STARTUP_FUNCTION in this pull request.

I recommend removing these in Utils.cpp? line 216

ifndef QATERIAL_STATIC

Q_COREAPP_STARTUP_FUNCTION(Qaterial_registerTypes); Q_COREAPP_STARTUP_FUNCTION(Qaterial_loadResources);

endif

Maybe adding some tips inside doc, let user know this is one way instead of manually registerTypes and loadResources.