Bug 277133

I already fixed this code in git master. Please checkout code from KDE repository and try like this...

Gilles Caulier

Look here :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/dimg/loaders/pgfloader.cpp#L433

Gilles Caulier

That should build, yes. But what happens if PGFCodecVersionID is defined and smaller 0x061124? (That happens if your FindPGF.cmake identifies an older codec, right?) As I read it there will be no call to Write at all... This is why I simplified the #if's

PGF codec ID do not exist in official libpgf yet. I patch internal lib and reported that to PGF team.

This ID for the moment is managed in PGF find cmake script :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/cmake/modules/FindPGF.cmake#L26

This ID is always different and value progress incrementally for for each releases

Gilles Caulier

(In reply to comment #4)
> PGF codec ID do not exist in official libpgf yet. I patch internal lib and
> reported that to PGF team.
> This ID for the moment is managed in PGF find cmake script
(...)

Yes, I know. Sorry I should have been more precise. But... let's assume we build the >>current git master<< digikam against an external system libpgf-6.09.44

Then, the FindPGF.cmake script defines PGF_CODEC_VERSION_ID=060944 and digikam/utils/config-digikam.h sets 
#define PGFCodecVersionID 0x${PGF_CODEC_VERSION_ID}
i.e. PGFCodecVersionID=0x060944

Now if you look at libs/dimg/loaders/pgfloader.cpp line 433 and following, PGFCodecVersionID is defined, but it is also smaller than 0x061124. Meaning, no branch of the #if statements is actually compiled, and there is no call to pgf.Write at all. (Of course this compilation always succeeds, but the resulting code does not do anything.)

With the current version of FindPGF.cmake, PGFCodecVersionID is always defined after the pgf library has been found. This is why I suggest changing the code to 

#if PGFCodecVersionID >= 0x061124
        pgf.Write(&stream, &nWrittenBytes, CallbackForLibPGF, this);
#else
        pgf.Write(&stream, 0, CallbackForLibPGF, &nWrittenBytes, this);
#endif

The same problem also exists in libs/threadimageio/pgfutils.cpp line 156 and following, which should in my opinion be 

#if PGFCodecVersionID >= 0x061124
        pgfImg.Write(&stream, &nWrittenBytes);
#else
        pgfImg.Write(&stream, 0, 0, &nWrittenBytes);
#endif

>let's assume we
>build the >>current git master<< digikam against an external system
>libpgf-6.09.44

In this case, FindPGF will report to not found an external libpgf. So, FinPGF will not define codec ID and value from digiKam core will be used as well..

Gilles Caulier

(In reply to comment #6)
> >let's assume we
> >build the >>current git master<< digikam against an external system
> >libpgf-6.09.44
> 
> In this case, FindPGF will report to not found an external libpgf. So, FinPGF
> will not define codec ID and value from digiKam core will be used as well..

Why? 

-- checking for module 'libpgf'
--   found libpgf, version 6.09.44
-- PGF_INCLUDE_DIRS     = /usr/include/libpgf
-- PGF_INCLUDEDIR       = /usr/include/libpgf
-- PGF_LIBRARIES        = pgf
-- PGF_LDFLAGS          = -lpgf
-- PGF_CFLAGS           = -I/usr/include/libpgf
-- PGF_VERSION          = 6.09.44
-- PGF_CODEC_VERSION_ID = 60944

Because libpgf uninstall ùethod is bugous and pkconfig file in not removed from the system.

I tested to compile digiKam with olders version of libpgf (compiled and installed using official tarballs), and at each time, after to run libpgf "make uninstall", i see this problem relevant of pkconfig file not cleaned on my system.

Gilles Caulier

What I mean is, right now digikam finds and recognizes the old libpgf just fine (see the configure snippet in comment #7), and defines the version accordingly. I cannot find any code in FindPGF that disables this for older libpgf versions. This is why I dont understand your comment #6; that's clearly wrong.

As for the problematic uninstall method, well that is what you have distributions, packagers and package manager programs for... :)

>This is why I dont understand your comment #6; that's clearly wrong.

There is 2 cases : 

- you use an external libpgf : FindPGF script setup codec ID

- you don't use an external libpgf : internal code is used instead and setup codec ID.

that simple. 

>As for the problematic uninstall method, well that is what you have
>distributions, packagers and package manager programs for... :)

No. i don't use a package from my distro. I downloaded the libpgf tarball from SF.net, and compiled/installed it as well using automake/autoconf. The make provide uninstall command which forget to remove pkconfig file (all the rest is cleaned from my system). It's a problem from libpgf tarball, not my distro.

Gilles Caulier

(In reply to comment #10)
> >This is why I dont understand your comment #6; that's clearly wrong.
> 
> There is 2 cases : 
> 
> - you use an external libpgf : FindPGF script setup codec ID
> 
> - you don't use an external libpgf : internal code is used instead and setup
> codec ID.
> 
> that simple. 

So in either case PGFCodecVersionID is defined, see comment #5.

From Comment #5 :

>Then, the FindPGF.cmake script defines PGF_CODEC_VERSION_ID=060944 and
>digikam/utils/config-digikam.h sets 
>#define PGFCodecVersionID 0x${PGF_CODEC_VERSION_ID}
>i.e. PGFCodecVersionID=0x060944

This is true, but in this case _external_ libpgf include dir will be used instead internal libpgf dir. To resume, internal libpgf will be dropped as well, and ID used will be one defined from FindPGF script. 

Rules to manage libpgf Include dir is here :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/CMakeLists.txt#L561

Gilles Caulier

Yes, but that means that libs/dimg/loaders/pgfloader.cpp and libs/threadimageio/pgfutils.cpp (which are not part of the pgf library but of digikam) are compiled with the external libpgf includes and the ID defined by FindPGF from the version of the external libpgf (which can be below 0x061124).

Absolutly, and it's the case (:=)))

Gilles Caulier