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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.ktx2 files vary from parallelism #2991

Closed
bmwiedemann opened this issue Nov 10, 2022 · 6 comments
Closed

.ktx2 files vary from parallelism #2991

bmwiedemann opened this issue Nov 10, 2022 · 6 comments
Labels

Comments

@bmwiedemann
Copy link
Contributor

Describe the bug
While working on reproducible builds for openSUSE, I found that
our warzone2100 package started to vary in base.wz/texpages/page-102-bottom-rockies.ktx2 and 19 other .ktx2 files.
My tools tell me, that the results become reproducible if I run both builds on 1-core-VMs instead of varying cores from 1 to 4.

To Reproduce
Steps to reproduce the behavior:

  1. on Debian or openSUSE, you can do
osc co openSUSE:Factory/warzone2100 && cd $_
for N in 1 4 ; do
    osc build --clean --noservice --vm-type=kvm -j$N --keep-pkg=RPMS.$N
    unrpm RPMS.$N/warzone2100-data-*.noarch.rpm
    zipinfo usr/share/warzone2100/base.wz | grep ktx2 > $N.out
done
diff -u {1,4}.out

Expected behavior
Build results should be fully deterministic

Screenshots or Videos

build log diff showed

- Compression succeeded to file "/home/abuild/rpmbuild/BUILD/warzone2100/build/data/base/texpages/page-102-bottom-rockies.ktx2" size 1375248 bytes in 144.393 secs
+ Compression succeeded to file "/home/abuild/rpmbuild/BUILD/warzone2100/build/data/base/texpages/page-102-bottom-rockies.ktx2" size 1375250 bytes in 114.493 secs

Your System:

  • OS: Linux (openSUSE Tumbleweed-20221108)
  • Game version: 4.3.1

Additional context

Files are probably created by 3rdparty/basis_universal/basisu_tool.cpp .
Some compression methods (e.g. xz) are known to produce variations depending on number of threads. One workaround with .xz is to always use at least 2 threads, even on 1-core machines.

@past-due
Copy link
Member

past-due commented Nov 10, 2022

This is probably something worth opening as an Issue in the Basis-Universal project as well: https://github.com/BinomialLLC/basis_universal

The ktx2 files should be using Zstd compression, and my understanding is that Zstd (or, at least, recent versions) is supposed to produce reproducible output given the same input, settings, and version of Zstd.

Whether the basis-universal tool is passing different settings to it, or there's some other aspect of the earlier transcoding parts that is non-deterministic, I haven't yet had a chance to look into.

Happy to accept any suggestions / insight, though.

EDIT: Looking at the basis_tool options, it's possible that passing:

-uastc_rdo_m
Disable RDO multithreading (slightly higher compression, deterministic).

or

-no_multithreading
Disable multithreading

might do the trick. The first one sounds the most promising?

@bmwiedemann
Copy link
Contributor Author

Incidentally, I happen to be the openSUSE package maintainer for zstd, so I know that the zstd CLI tool is fine in this regard, but libzstd may still produce different outputs between 1-core and 2+ cores.

This makes it reproducible

--- warzone2100.orig/data/CMakeLists.txt
+++ warzone2100/data/CMakeLists.txt
@@ -131,7 +131,7 @@ if(WZ_ENABLE_BASIS_UNIVERSAL)
                set(_output_name "${TEXTURE_FILE_NAME_WE}.ktx2")
                add_custom_command(OUTPUT "${_output_dir}/${_output_name}"
                        COMMAND "${BASIS_UNIVERSAL_CLI}"
-                       ARGS -ktx2 -uastc -uastc_level 2 -uastc_rdo_l 1.0 -mipmap -resample ${_terrain_max_size} ${_terrain_max_size} -output_file "${_output_dir}/${_output_name}" -file "${TEXTURE}"
+                       ARGS -uastc_rdo_m -ktx2 -uastc -uastc_level 2 -uastc_rdo_l 1.0 -mipmap -resample ${_terrain_max_size} ${_terrain_max_size} -output_file "${_output_dir}/${_output_name}" -file "${TEXTURE}"
                        WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/base/texpages"
                        DEPENDS "${TEXTURE}"
                        VERBATIM

Do you think, basis_universal would take a patch to make it the default?

@bmwiedemann
Copy link
Contributor Author

-no_multithreading also helps there.

@past-due
Copy link
Member

This makes it reproducible

Great! 🥳 I've added -uastc_rdo_m to the flags we pass to basis_universal.

Do you think, basis_universal would take a patch to make it the default?

I have no idea, unfortunately, but it can't hurt to ask. (Regardless, this should be fixed for WZ's build scripts.)

@bmwiedemann
Copy link
Contributor Author

This was fixed by commit 27218d7

Author: past-due
Date:   Sun Nov 13 13:10:44 2022 -0500

    [CMake] Add -uastc_rdo_m to basis universal to improve reproducible builds

@bmwiedemann
Copy link
Contributor Author

filed new related BinomialLLC/basis_universal#374

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants