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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support compilation using system libgmic and CImg #157

Merged
merged 2 commits into from Jun 29, 2022

Conversation

amyspark
Copy link
Contributor

馃憢

This PR upstreams the patches enabling building Krita's G'MIC-Qt using system libgmic and CImg. These were provided originally by @darix and @cryptomilk, with a recent bugfix by @antonio-rojas for G'MIC-Qt 3.1.4.

The attached commits are extensions of the below PRs, which will be closed as completed.

Closes #134
Closes #81

cc @dtschump @c-koi

cryptomilk and others added 2 commits June 28, 2022 22:02
Co-authored-by: Antonio Rojas <arojas@archlinux.org>
@darix
Copy link
Contributor

darix commented Jun 28, 2022

oh i was planning to refresh my PR over the weekend. thank you for doing it :)

@dtschump
Copy link
Collaborator

We don't really get what is your goal with this PR.
Could you explain it.
In what it is different from compiling with GMIC_DYNAMIC_LINKING=on ?

@amyspark
Copy link
Contributor Author

amyspark commented Jun 29, 2022

In what it is different from compiling with GMIC_DYNAMIC_LINKING=on ?

That's only available in the qmake toolchain, and is Linux only. Also cc @antonio-rojas and @darix for more context.

@dtschump
Copy link
Collaborator

That's only available in the qmake toolchain, and is Linux only. Also cc @antonio-rojas and @darix for more context.

Well, but why having to change all these files to make it work for other platforms than Linux?
In particular, I don't see the point with:

#ifndef gmic_core
#include "CImg.h"
#endif
#include "gmic.h"

that appears in most .cpp files.

@dtschump
Copy link
Collaborator

I see you want to remove the definition of gmic_core when compiling, so that's probably the reason why all these extra "CImg.h" includes are necessary.
But why trying to remove gmic_core ? What problem does it solve?

@dtschump
Copy link
Collaborator

Ha I guess, that's because you don't want gmic.h depends on gmic.cpp...

@dtschump
Copy link
Collaborator

@c-koi , maybe we can wait for @amyspark 's answer, but if so, the PR seems correct to me.
It allows G'MIC-Qt to be compiled in GMIC_DYNAMIC_LINKING=on mode without requiring a dependency to gmic.cpp, which is nice.
The change to be made in the gmic-qt.pro file should be only to undefine gmic_core when GMIC_DYNAMIC_LINKING=on (as you did on ZArt a few days ago).

@cryptomilk
Copy link
Contributor

The krita5 gmic plugin links against libgmic, We want to build with the header files and library provided by a gmic package and not do a in-source build in the gmic sources.

@darix
Copy link
Contributor

darix commented Jun 29, 2022

Krita forked gmic-qt for their integration into krita. Due to the way gmic-qt was built it included the gmic.cpp file and as such it was recompiled over and over. in an ideal world a distro builds one gmic package. and gmic-qt is a separate package that links against the gmic-devel package.

And then building krita's gmic integration becomes very simple without n compiled copies of gmic.cpp on a system.

This discussion is related to this PR. You accepted all the needed fixes for this in gmic itself. just the cleanup of the gmic-qt side is still pending.
https://discuss.pixls.us/t/revive-an-old-issue-allow-gmic-qt-to-link-the-shared-libgmic/28448

@dtschump
Copy link
Collaborator

I understand.
It looks good to me. @c-koi , I suggest this PR gets merged.

@c-koi c-koi merged commit 7daedbb into c-koi:master Jun 29, 2022
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.

None yet

5 participants