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

WASM: Some collisions are ignored in Safari #577

Closed
Myrindd opened this issue Jun 16, 2023 · 18 comments
Closed

WASM: Some collisions are ignored in Safari #577

Myrindd opened this issue Jun 16, 2023 · 18 comments

Comments

@Myrindd
Copy link

Myrindd commented Jun 16, 2023

I am currently using Jolt as part of a C++ engine compiled to WASM (both are compiled together so the cmakelist parameters are identical); which works fine with chrome on windows but I am encountering some issues on mac (M2 chipset):

  • chrome macOS works but is unstable especially when a body slides over two bodies
  • safari is having issues with some bodies colliding with one another and some others going right through one another. There is even cases where the order of the activation affect the ability of two bodies to collide with one another.

As mentioned, the issue is only happening on macs, so it might be related to the chipset; although the behavior is quite different between chrome and safari.

Compiling parameters:

  • double_precision
  • build_shared_libs
  • stack size = 4MB
  • physics is single-threaded
@Myrindd
Copy link
Author

Myrindd commented Jun 16, 2023

Upon further investigation, it seems that even if I do not use any threads, simply compiling with multiple threads is enough to trigger the issue with Safari.

@jrouwe
Copy link
Owner

jrouwe commented Jun 16, 2023

I'm sorry, I don't own a Mac so I have no way to help you with this. Maybe I have an idea if you post a movie of the behavior, but this is a long shot.

@Myrindd
Copy link
Author

Myrindd commented Jun 16, 2023

physics_jolt.mp4

Sure, here is a video.
I am using forces applied to body by IDs (bodyinterface->addforce) before each update iteration. I am using Jolt's job system with 0 threads (will change in the future) to update the physics.
The table should collide with everything, but it does not collide with the ground in Safari. The problem disappears if I compile everything without multi-threading.

@jrouwe
Copy link
Owner

jrouwe commented Jun 16, 2023

If you're running with 0 threads then that rules out any race conditions so it sounds like this is really a problem with the compiler or the browser (most likely the latter if the same thing runs fine on other browsers). This is something that would need to be debugged to see if there is a workaround possible. E.g. do you get to PhysicsSystem::ProcessBodyPair for the chair vs ground and does it find any collisions there.

@Myrindd
Copy link
Author

Myrindd commented Jun 16, 2023

Thank you for the feedback; I will start by updating emscripten to the latest version; then start digging into the physics to figure out where it fails; but it does feel like the work could be somehow shared with a "ghost thread" which is not doing the job.

@Myrindd
Copy link
Author

Myrindd commented Jun 18, 2023

To keep you updated: by simplifying the job system so as to remove all barriers (does not matter as the physics is single-threaded), I was able to get the collision with the ground to work in Safari; although the system ends up crashing after a few seconds with the following error:
image

@jrouwe
Copy link
Owner

jrouwe commented Jun 18, 2023

The error is very vague, can you show what you changed?

@Myrindd
Copy link
Author

Myrindd commented Jun 18, 2023

I simplified the job system as follow for testing purpose:
image

This is obviously very crude; but while the configuration works in chrome, the problem still happens in Safari.
When I enable assertions, I get the following message:

Jolt\Physics\Collision\BroadPhase\QuadTree.cpp:127: (mAllocator->Get(inNodeIdx).mChildNodeID[inChildIdx] == inBodyID) Make sure that the body is in the node where it should be

Safari only works if I compile everything with no threads (without -s USE_PTHREADS=1).

Suprisingly, safari would work in an earlier iteration of the simplification of the job system; but it would crash after a few seconds. This is why I was led to believe this is related to the job system; but who knows really...

@Myrindd
Copy link
Author

Myrindd commented Jun 18, 2023

To add to the above; the QuadTree error is happening whenever a body is added (BodyInterface->AddBody); even if the body is but a simple sphere.
Anyway, Safari is probably having issues with the way the system handles shared memory (atomics, etc.). I am not sure there is much that can be done at this point.

@jrouwe
Copy link
Owner

jrouwe commented Jun 19, 2023

mJobs[cJobCount++] = job;

will start corrupting memory when there are more jobs than cMaxJobCount and those jobs will not get executed because in the run you and with cMaxJobs - 1 (not sure if this happens in your case).

@Myrindd
Copy link
Author

Myrindd commented Jun 19, 2023

For sure; but as mentioned, it was for testing purpose with a simple environment where I did not expect more than 1024 jobs per barrier. It worked just fine on Chrome but made no difference on Safari.
In fact, the above mentioned error happens whenever a body is added, so it is likely not related to the job management system in itself, but possibly to the quad tree...?

@jrouwe
Copy link
Owner

jrouwe commented Jun 21, 2023

The assert that is triggering is part of an integrity check of the tree. It could mean that the tree was corrupted earlier on (e.g. during the Update), but it could also mean that you are sharing bodies between multiple PhysicsSystems (this is not allowed).

@Myrindd
Copy link
Author

Myrindd commented Jun 21, 2023

I would have to check again, but I am pretty sure the issue was happening even before the very first update; perhaps I will just run the helloworld example on Safari, see what happens.
I only use one physicssystem, so there should be no issue with that regard.
In any case, this is an issue that is specific to Safari, so it is likely something deeper than a wrong configuration. Safari is especially sensitive to SharedArrayBuffer (I have had my fair share of despair because of it).

@jrouwe
Copy link
Owner

jrouwe commented Jun 21, 2023

I created a single threaded job system here: https://github.com/jrouwe/JoltPhysics/tree/feature/single_threaded_job_system

Perhaps you can try that out.

@jrouwe
Copy link
Owner

jrouwe commented Jun 21, 2023

If that doesn't help, I suggest we close this ticket as I can't really fix a browser bug.

@Myrindd
Copy link
Author

Myrindd commented Jun 21, 2023

Thank you, I will try it out.
Sure, if I find something; I will let you know.

@Myrindd
Copy link
Author

Myrindd commented Jun 22, 2023

So, I have tried switching to the single-threaded job system you made; also reverted to the helloworld example. Unfortunately, no luck:

image

Thank you for your help, I think that at this point there is not much that can be done beside wait for the next safari update... As for me, I will keep on using my own physics engine for now.

For the ones interested in using Jolt on the web: it works as long as the program is compiled without pthread.

@Myrindd Myrindd closed this as not planned Won't fix, can't repro, duplicate, stale Jun 22, 2023
@jrouwe
Copy link
Owner

jrouwe commented Jun 23, 2023

Thanks for testing!

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

No branches or pull requests

2 participants