search

c-koi / gmic-qt

G'MIC-Qt is a versatile front-end to the image processing framework G'MIC.
GNU General Public License v3.0
207 stars 54 forks source link

Support compilation using system libgmic and CImg #157

Closed amyspark closed 2 years ago

amyspark commented 2 years ago

:wave:

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

darix commented 2 years ago

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

dtschump commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

dtschump commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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