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

Modernise GLM CMake #732

Closed
5 tasks
ptheywood opened this issue Nov 1, 2021 · 1 comment · Fixed by #991
Closed
5 tasks

Modernise GLM CMake #732

ptheywood opened this issue Nov 1, 2021 · 1 comment · Fixed by #991

Comments

@ptheywood
Copy link
Member

ptheywood commented Nov 1, 2021

The GLM CMake was not modernised during the big CMake modernisation, so it still relies on global cmake variables rather than using a header only target.

This should be improved to match other header only targets.

Unfortunately, recent GLM seems to have dropped the use of version numbers / hasn't made a new versioned relese since april 2020, with almost 200 commits since this release, so using CMake to check the correct GLM was found might not be viable.

In addition, the vis repo GLM should be modenised together with this. Ideally with both repos requireing the same glm version, and only downloading it once.

Alonside the use of targets, the main flamegpu library must also inform RTC of include paths. This is currently handled by a macro definiation GLM_PATH, forwarding the include path from CMake. This is non portable, so we need to address this (part of #260, maybe #607).

Further more, 2.0.0a1 USE_GLM builds only work when visualiseation is enabled. This will be fixed quickly in the short term with a very minor change, to be improved at a later date.

  • Modernise GLM use, to use an imported interface target (see FindJitify.cmake)
  • Ideally find existing GLM versions and check if it is sufficiently new. This may not be possible as GLM seems to have stopped making new versioned releases
  • Update vis repo GLM to match
  • Use the same GLM in both repositories if required. I.e. only download if GLM was not already in _deps. Without usable version numbers it will be hard to support this and allow use-provided glm
  • Optionally: Expand CI to include a USE_GLM=ON build(s) to ensure it works in the future.
@Robadob
Copy link
Member

Robadob commented Nov 1, 2021

Further more, 2.0.0a1 USE_GLM builds only work when visualiseation is enabled. This will be fixed quickly in the short term with a very minor change,

Fixed now in branch host_env_glm, unclear when that will reach master.

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

Successfully merging a pull request may close this issue.

2 participants