Add Zstandard + compression level

[alphaonex86]

No description provided.

[bjorn]

Hmm, I can't accept a change that "presumes zstd library is installed". Either the zstd source code needs to be shipped or building with zstd support should be optional.

[alphaonex86]

Hi, fixed

[Oipo]

Appreciate the work done here by both of you, but I would really like this functionality. Is this something that will ever get merged into tiled? Otherwise I'll just fork tiled and keep it up to date myself.

[bjorn]

@Oipo So far I was rather reluctant to add this since I've heard nobody but @alphaonex86 about this and additional options do come at a maintenance cost. But you're the second person asking for this compression format, so maybe it's worth supporting it, especially since apparently the difference between zlib and zstd is significant enough that you'd maintain your own fork for it.

That said, this patch makes it an optional build option and zstd will need to be available on the system when compiling Tiled, and probably will also require shipping additional files (unless it's statically linked). Any help getting zstd supported in the macOS, Windows and Linux builds would be very appreciated!

[alphaonex86]

Hi, I can help you to build with zstd on all OS, it's what's I do (include web assembly).
On Mac: brew install zstd
On linux debian: libzstd-dev
On Windows: make;make install

[Oipo]

@alphaonex86 I fixed the merge conflicts and pushed some fixes to my fork: https://github.com/Oipo/tiled/commits/master

Perhaps you want to include those as well?

[bjorn]

@Oipo I've fixed up the merge conflicts as well and pushed it into @alphaonex86's master branch. I shall cherry-pick your fixes on top.

[Oipo]

@bjorn Adding zstd to the linux build/travis file is quite easy, but I don't know how to do it for the appveyor file. I assume that's the automated build for windows? Also, where is the build file for mac builds?

[bjorn]

@Oipo Right, the appveyor.yml makes the Windows builds (32-bit, 64-bit and WinXP as per the "environment.matrix" entries). The macOS builds happen on Travis CI (the os: osx section).

I've just pushed a bunch of small cleanups. Note that also, the "optional" nature of this patch currently requires commenting out lines in the libtiled.pro file. That should of course be changed to an actual build option. Also, actually I don't use qmake anywhere except for the snap builds (which are another thing to update when we want zstd supported there, see snap/snapcraft.yaml). The Qbs project files should be updated as well to add this option.

[bjorn]

@alphaonex86 Why is a separate buffer allocated here which is then copied from, rather than compressing into a QByteArray directly as is done for the gzip/zlib code path?

        size_t const cBuffSize = ZSTD_compressBound(data.size());

        void* const cBuff = malloc(cBuffSize);
        if (!cBuff) {
            qDebug() << "error to alloc" << cBuffSize;
            return QByteArray();
        }

        size_t const cSize = ZSTD_compress(cBuff, cBuffSize, data.constData(), data.size(), compressionLevel);
        if (ZSTD_isError(cSize)) {
            qDebug() << "error compressing:" << ZSTD_getErrorName(cSize);
            return QByteArray();
        }

        QByteArray data(static_cast<char *>(cBuff), cSize);
        free(cBuff);

[bjorn]

As one can see this PR required still a bit of work, and we're not done yet since it's not trivial to adjust the build scripts on each platform as well as the Windows installer. I have to pause my efforts here, possibly until Tuesday or Thursday next week depending on whether there is time available in the evenings.

[alphaonex86]

@alphaonex86 Why is a separate buffer allocated here which is then copied from, rather than compressing into a QByteArray directly as is done for the gzip/zlib code path?

I don't remember why now. I think it should be changed to QByteArray

[Oipo]

@bjorn Alright.

In the meantime, I've managed to make ZSTD configurable in qmake as well as qbs and integrate it into the linux builds. See the PR I opened. I think that for OSX, and possibly Windows as well, we'll have to download and compile the zstd source ourselves.

[bjorn]

@Oipo Hmm, something regarding the static linking appears to not work properly. All Windows builds now require a zstd.dll file, which is not included in the distribution. So at the moment the latest development snapshot fails to launch. I see you're doing cpp.staticLibraries: ["libzstd"], so I'm not sure why it anyway ended up needing the dynamic library. Any idea?

[Oipo]

That's weird. I'll check around on appveyor.

[bjorn]

I noticed that the output after "compiling static library" is empty in the AppVeyor build output.

[Oipo]

@bjorn see #2173