taisei: update to 1.4.1

[aryalaadi]

Testing the changes

• I tested the changes in this PR: YES

[tornaria]

Great! Could you submit your patches as PR to upstream?

@ahesford: this LGTM now

[ahesford]

What motivates the switch to SSE2 contrary to upstream's specification?

Please add an explanation to the top of the zstdoom patch about why it's necessary. Also, because it would be best to send this patch upstream, you might consider a more descriptive argument like --disable-zstd instead of --monkey_patch_i686.

[tornaria]

What motivates the switch to SSE2 contrary to upstream's specification?

I suggested it, since cglm uses sse2 intrinsics and makes no attempt at being compatible with sse. Note that x86_64 includes sse2 by definition, which is why -msse and -msse2 make no difference at all on 64 bits.

It's been discussed before that using sse2 is ok-ish for i686. We try to avoid it for core packages so the distro still has a chance of running in 25 year old cpus.

Please add an explanation to the top of the zstdoom patch about why it's necessary. Also, because it would be best to send this patch upstream, you might consider a more descriptive argument like --disable-zstd instead of --monkey_patch_i686.

This makes sense. As a matter of fact, I was thinking of just checking if arch is i686 directly in the python script rather than in meson.

[aryalaadi]

This makes sense. As a matter of fact, I was thinking of just checking if arch is i686 directly in the python script rather than in meson.

how is the python script supposed to know target arch

[aryalaadi]

Also, because it would be best to send this patch upstream

sure, but before that I will look into cglm and zstandard to see if I fix the root issue

[ahesford]

I suggested it, since cglm uses sse2 intrinsics and makes no attempt at being compatible with sse. Note that x86_64 includes sse2 by definition, which is why -msse and -msse2 make no difference at all on 64 bits.

It's been discussed before that using sse2 is ok-ish for i686. We try to avoid it for core packages so the distro still has a chance of running in 25 year old cpus.

It seems OK to enable SSE2 if it fixes the build for i686. This should definitely be submitted upstream in case they know of some issue we don't, but we can carry the patch in the meantime.

[Akaricchi]

-msse2 is ok, but the proper fix would be to correct this check in CGLM: https://github.com/recp/cglm/blob/1de373a9bd453d1fff6846db3a01ade8270f12bb/include/cglm/simd/intrin.h#L27

It should only check for SSE2 there. I can try to submit that upstream a bit later, though I'll need to set up a 32-bit environment to test it first.

For what it's worth, openSUSE doesn't build Taisei for i686, because CGLM fails (or at least used to fail) some unit tests on that platform. It doesn't look like i686 support is well maintained there, and we neither test nor provide i686 Linux builds of Taisei either. I don't believe there's an x86 system around that can handle this game acceptably well while not supporting x86-64.

As for why we enforce -msse: what we really want is -mfpmath=sse, which prevents the compiler from using x87 FPU instructions for floating point math. It's to reduce the potential for replay compatibility problems between different builds, due to the extended precision used by those instructions.

[tornaria]

-msse2 is ok, but the proper fix would be to correct this check in CGLM: https://github.com/recp/cglm/blob/1de373a9bd453d1fff6846db3a01ade8270f12bb/include/cglm/simd/intrin.h#L27

It should only check for SSE2 there. I can try to submit that upstream a bit later, though I'll need to set up a 32-bit environment to test it first.

An easy way is to check out this branch, then do

./xbps-src -A i686 binary-bootstrap
./xbps-src -A i686 pkg cglm
./xbps-src -A i686 pkg taisei

You can put patches in srcpkgs/cglm/patches/*.patch before the pkg cglm step.

For what it's worth, openSUSE doesn't build Taisei for i686, because CGLM fails (or at least used to fail) some unit tests on that platform. It doesn't look like i686 support is well maintained there, and we neither test nor provide i686 Linux builds of Taisei either. I don't believe there's an x86 system around that can handle this game acceptably well while not supporting x86-64.

Isn't that a good reason to build with sse2 by default?

As for why we enforce -msse: what we really want is -mfpmath=sse, which prevents the compiler from using x87 FPU instructions for floating point math. It's to reduce the potential for replay compatibility problems between different builds, due to the extended precision used by those instructions.

You can achieve a similar result without sse using -ffloat-store (at a cost).

BTW, do you really need level=20 for zstd? That's supposed to use a lot of memory. Maybe 19 is good enough (or you could use something like 20 if sys.maxsize > 2**32 else 19).

[aryalaadi]

musl builds failing now...

[Akaricchi]

An easy way is to check out this branch, then do

I'm not a Void user, so I think I'll just set up the i686 version of Void in a container for testing.

Isn't that a good reason to build with sse2 by default?

I don't mind bumping it up to -msse2 upstream. I only went with -msse by default because that's the minimum requirement for -mfpmath=sse, and I was somewhat more conservative about i686 support back then.

You can achieve a similar result without sse using -ffloat-store (at a cost).

Not really. -ffloat-store only prevents individual variables from being stored in registers. It won't break up calculations in expressions, e.g. float x = a * b / (c - d); the whole thing will be computed in extended precision unless you move every calculation into a separate variable, which is not acceptable for Taisei. Even then, a calculation performed at 80-bit precision truncated to 64 bits may still have a slightly different result. Not to mention the overhead introduced by placing every single float variable into RAM.

BTW, do you really need level=20 for zstd? That's supposed to use a lot of memory. Maybe 19 is good enough (or you could use something like 20 if sys.maxsize > 2**32 else 19).

The script definitely wasn't written with limited memory/address space in mind. I'll try 19 on i686 and see if it helps.

There's an alternative solution though: you could split the package into taisei and taisei-data. The latter would be noarch and can be built just once, on a 64-bit userspace.

# taisei without data
cd /the/build/dir
ninja src/taisei doc/GAME.html doc/ENVIRON.html doc/STORY.txt doc/README.txt
meson install --destdir=/whatever --no-rebuild --tags runtime,doc,license,xdg

# taisei-data only
cd /the/build/dir
ninja resources/00-taisei.zip
meson install --destdir=/whatever --no-rebuild --tags resources

Unfortunately meson install isn't smart enough to only rebuild what's needed according to --tags, so the build targets have to be specified manually for now. I can alleviate this problem upstream by adding some shorthand alias build targets, though.

[Akaricchi]

musl builds failing now...

Looks like another python-zstandard problem. My split package proposal should "fix" that too. But really it's python-zstandard that needs fixing here. You can also use meson configure -Dpackage_data=disabled to sidestep the whole compression issue.

[ahesford]

We no longer support noarch packages, so there is no option but to build the package data for i686.

[tornaria]

musl builds failing now...

Looks like another python-zstandard problem. My split package proposal should "fix" that too. But really it's python-zstandard that needs fixing here. You can also use meson configure -Dpackage_data=disabled to sidestep the whole compression issue.

This seems to be just a side-effect of zstd update which broke python-zstandard. Often the musl builders finish before the glibc builders, so your glibc CI run without the update and the musl CI run with the update.

Can you try again, now that python-zstandard has been rebuilt? (just rebase to current master and force push so the CI runs again).

[aryalaadi]

BTW, do you really need level=20 for zstd? That's supposed to use a lot of memory. Maybe 19 is good enough (or you could use something like 20 if sys.maxsize > 2**32 else 19).

I tried this and it builds for i686 without any issues (on my machine, idk why github builds are failing)

[tornaria]

BTW, do you really need level=20 for zstd? That's supposed to use a lot of memory. Maybe 19 is good enough (or you could use something like 20 if sys.maxsize > 2**32 else 19).

I tried this and it builds for i686 without any issues (on my machine, idk why github builds are failing)

Have you tried looking at "details" on the github run? It seems python3-zstandard is completely broken as you can check locally with

$ python -m zstandard
Traceback (most recent call last):
  File "<frozen runpy>", line 189, in _run_module_as_main
  File "<frozen runpy>", line 148, in _get_module_details
  File "<frozen runpy>", line 112, in _get_module_details
  File "/usr/lib/python3.12/site-packages/zstandard/__init__.py", line 39, in <module>
    from .backend_c import *  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^
ImportError: zstd C API versions mismatch; Python bindings were not compiled/linked against expected zstd version (10506 returned by the lib, 10506 hardcoded in zstd headers, 10505 hardcoded in the cext)

It seems upstream for python3-zstandard hardcodes the version of zstd that can be used :facepalm:

[tornaria]

@unspecd here in this PR we are having trouble with python3-zstandard which seems to be completely broken atm.

[unspecd]

@tornaria I just updated the package python3-zstandard.

[recp]

since cglm uses sse2 intrinsics and makes no attempt at being compatible with sse

I've just started to fix SSE / SSE2 compatibility issue at https://github.com/recp/cglm/pull/412 since I have 64bit arch only any help, testing on see-only environment, feedback would be awesome

[recp]

For what it's worth, openSUSE doesn't build Taisei for i686, because CGLM fails (or at least used to fail) some unit tests on that platform. It doesn't look like i686 support is well maintained there, and we neither test nor provide i686 Linux builds of Taisei either.

@Akaricchi I have fixed some tests when -ffast-math option enable at: https://github.com/recp/cglm/pull/412 maybe that caused to fail. I would like to see the test results to fix them asap. I also tried to test on 32bit Debian, all tests are passed now.

v0.9.4 of cglm is released: https://github.com/recp/cglm/releases I hope SSE can now work without any issue and SSE2 requirement 🚀

[aryalaadi]

v0.9.4 of cglm is released: https://github.com/recp/cglm/releases I hope SSE can now work without any issue and SSE2 requirement 🚀

thank you, I'll bump the cglm package to 0.9.4 when I'm free

[aryalaadi]

@tornaria I've added the python3-standard patch you suggested and also bumped (and adopted) cglm.

[tornaria]

This looks better to me than #49606 in that it avoids the vendored zstd.

[github-actions[bot]]

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

[classabbyamp]

needs rebase

[github-actions[bot]]

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.