SPlisHSPlasH icon indicating copy to clipboard operation
SPlisHSPlasH copied to clipboard

macOS compatibility

Open ruberith opened this issue 3 years ago • 18 comments

These adaptations allow builds using Clang on recent versions of macOS (tested on Apple silicon):

  • Updated and adapted versions of CompactNSearch, Discregrid and PositionBasedDynamics are used (see CompactNSearch#7, Discregrid#19 and PositionBasedDynamics#124). If the pull requests are accepted, CMake/NeighborhoodSearch.cmake and CMake/SetUpExternalProjects.cmake have to be updated to point to the original repositories again.
  • Clang seems to mistake some version files for headers, so these are given an extension.
  • The optimization flag -mcpu=apple-m1 is used for Apple silicon, for which -march=native is not yet supported.
  • OpenGL is deprecated for years on macOS, but still supported, so the corresponding warnings are silenced.
  • _NSGetExecutablePath is used to find program paths on macOS.
  • numpy is available for Apple silicon since version 1.21.0. The highest supported version can possibly be updated in setup.py.
  • AVX is disabled for ARM.
  • Catch2 is updated to v2.13.8.
  • Clang adheres more closely to the C++ standard regarding lookups, so overloaded operators with template parameters are moved to the namespace std.
  • Higher versions of OpenGL are only supported with core profiles on macOS which are not compatible with removed functionality. Since MiniGL follows the fixed function pipeline, OpenGL has to fall back to version 2.1. In this case, the corresponding implementation of ImGui is now used.
  • Recent versions of CMake automatically determine the correct flags for OpenMP and provide a target to link against. Nevertheless, Clang does not include an OpenMP implementation on macOS by default, but the user can easily install a build of LLVM libomp (e.g. via Homebrew) to be shared by all projects requiring OpenMP which is then detected by CMake.

ruberith avatar Apr 09 '22 11:04 ruberith

To allow for the same visual experience on all platforms, I now have ported MiniGL to the OpenGL 3.3 Core Profile by replacing all fixed-function calls with "modern" equivalents.

ruberith avatar May 21 '22 12:05 ruberith

AVX is now supported on ARM via SIMDe which forwards to the native intrinsics on x86 and provides equivalent implementations using NEON on ARM.

ruberith avatar Jun 11 '22 09:06 ruberith

I actually tested this PR on my 2013 intel macbook air. while i was able to compile everything, it got a grey background in the gui and when trying to start the simulation i got a segfault. i will try to look into it some more soon to isolate the problem.

digitalillusions avatar Jun 23 '22 15:06 digitalillusions

After further testing, I couldn't spot a noticeable performance difference with the SIMD option active, so I removed the dependency on SIMDe again. As recommended by Apple, I substituted the intrinsics in the AVX math wrapper with equivalent calls to the Accelerate framework for macOS systems which yielded the expected performance boost. Since I do not have an Intel Mac, I could not check whether the existing AVX calls or Accelerate calls are faster or if effectively the same intrinsics are used; the implementation details of the framework calls are only partially accessible.

ruberith avatar Jul 30 '22 19:07 ruberith

Ok, so I can confirm that this indeed compiles for me on an M2 macbook air. I will also make another attempt using the intel macbook air in a couple of days.

digitalillusions avatar Aug 06 '22 19:08 digitalillusions

I can't find CLOTH_BENDING_STIFFNESS in PBD because it already upgrade to 2.0.1, How can I fix PBDWrapper?

yangfengzzz avatar Aug 18 '22 00:08 yangfengzzz

So I confirmed that this works on both an intel macbook air and an m2 macbook air. I think we should consider merging this soon since there were multiple requests for macos compatibility in the past. It should just be noted that its hard for us to maintain macos compatibility and that this will probably have to be done by the community.

digitalillusions avatar Aug 26 '22 09:08 digitalillusions

@digitalillusions Thank you for verifying. I have just merged in the latest updates, so together with CompactNSearch#7, Discregrid#19 and PositionBasedDynamics#123, the compatibility should be ensured for now.

ruberith avatar Aug 29 '22 15:08 ruberith

So we went through the PR in a bit more detail and were wondering if all of the proposed changes are actually strictly necessary to make it functional. Specifically, are all the changes to the GUI system and shaders actually necessary? Also, is the change to the alignment allocator necessary?

If not, would it be possible for you to split up the PR into changes actually required to run on MacOS, and optional improvements.

digitalillusions avatar Sep 28 '22 11:09 digitalillusions

macOS does unfortunately only support the core profile of OpenGL 3.3, in which the fixed function calls (including GLU) do not work. All changes are specific replacements of these calls according to their documentation to have the same visual experience. Similarly, for AVX compatibility on macOS ARM, I used equivalent calls from the Accelerate framework. Concerning the alignment allocator, I ran into errors when not choosing the exact alignment of the underlying data structure (16 bytes for Accelerate's simd::float8), even if 32 bytes are supposed to be fine. This seems to be fixed in a recent version of Apple Clang, so I will revert these changes.

ruberith avatar Sep 28 '22 17:09 ruberith

Thank you for the review. I will check where I can add comments in the code for clarification.

ruberith avatar Dec 01 '22 17:12 ruberith

Thanks @ruberith for your continued efforts! It seems good to me so far, but somehow the selection box does not align correctly with the mouse position for me on macos. Have you encountered something similar? Tbf it seems to work on linux and windows.

digitalillusions avatar Mar 06 '23 16:03 digitalillusions

This was due to retina displays having a framebuffer size of double the window size and should now be fixed. I have an external monitor connected, so I didn't notice that, thank you. :D Feel free to tell me if there is anything else I should change. Otherwise I would rebase as suggested for a clearer commit history.

ruberith avatar Mar 12 '23 15:03 ruberith

Thanks for all the hard work! It would have probably taken me ages to find that particular problem! The changes look good to me, the only other minor problem that I experience is that key presses dont seem to be captured if you launch splishsplash with the interactive file selector. I.e. if you directly pass a file on the command line everything works as expected. However, if you just run the program and use the native file dialogue, the key presses dont do anything. Can you also reproduce this?

digitalillusions avatar Mar 13 '23 17:03 digitalillusions

The pull request is now rebased on top of the main branch. Regarding the issues with the file dialog, I found a warning in the README of NFDe that might apply. When a scene file is specified on the command line, so that NFDe is initialized after GLFW, everything seems to work just fine if, for instance, a state file is loaded via the file dialog.

ruberith avatar Mar 14 '23 20:03 ruberith

Sorry for the delay @ruberith, we have been very busy last month. I have taken some time to look through the PR and already notice some issues.

  1. when compiling on MacOS the main binary target SPHSimulator seems to work fine. However, when compiling the python bindings I get segmentation faults when trying to import the module. A part of this seems related to missing CMake flags from here https://github.com/BlueBrain/nmodl/issues/9, but this does not seem to completely fix the problem.
  2. when running the basic SPHSimulator executable on linux, a SIGSEGV occurs during teardown of the application in the destructor of the Shader object. Since this is something that was also part of this PR maybe you could take a look if you have access to a linux installation?

digitalillusions avatar Apr 26 '23 13:04 digitalillusions

Unfortunately, I can not reproduce the first error. Using the latest versions of Apple Clang, CMake, and miniconda with Python 3.8, the interop builds and works in both directions on my system. I get the visibility warnings as well; however, they have no impact on the functionality of the bindings.

The second problem is to be expected as ~Shader is called after the destruction of the context by glfwDestroyWindow. I don't know why the application only segfaults on Linux, but we are here in the territory of undefined behavior. :D The latest commit should ensure that everything is destroyed properly.

ruberith avatar May 05 '23 15:05 ruberith