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

Minimal changes needed to make the example build and run on macOS #7

Merged
merged 3 commits into from
Dec 24, 2024

Conversation

floooh
Copy link
Contributor

@floooh floooh commented Dec 18, 2024

Fixes to make the example build and run on current Macs:

  • all the math functions in implot3d.cpp with std:: prefix throw compilation errors, fixed by adding a #include <cmath> at the top (I wonder though if it is better to just remove the std:: prefix. Let me know if I should try that instead.
  • there's one place with the same problem in implot3d_demo.cpp where std::sqrt() is called, here I simply removed the std:: prefix though (since the other math calls in that source file don't use the prefix either)

This was enough to make the example build, but then running failed with an error message that the GL context couldn't be created. This is fixed by changing the GL context version to 3.2 and enabling the GLFW_OPENGL_FORWARD_COMPAT GLFW window hint.

Result:

Screenshot 2024-12-18 at 13 48 39

NOTE: I haven't tested those changes on any other platform.

@floooh
Copy link
Contributor Author

floooh commented Dec 18, 2024

Ok, I just tested the alternative to not include <cmath> and instead replace the math functions with the Dear ImGui math functions (ImSin, ImCos, ImFabs, ImAcos and ImSqrt).

IMHO this is the better solution. From your comment it sounds like you want to make that change anyway:

#5 (comment)

...once that change lands I can remove the #include <cmath> and rebase the PR on your changes (or let me know if you'd rather want to close this PR and I can provide a fresh one based on your changes to fix the macOS build).

@brenocq brenocq self-requested a review December 19, 2024 23:48
@brenocq brenocq added type:bug Something isn't working prio:high High priority status:review The task is under review labels Dec 22, 2024
Copy link
Owner

@brenocq brenocq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work @floooh! I think it is ready for merging after cmath is removed

implot3d.cpp Outdated Show resolved Hide resolved
@brenocq brenocq self-requested a review December 24, 2024 12:32
@brenocq brenocq changed the base branch from main to dev December 24, 2024 12:35
Copy link
Owner

@brenocq brenocq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Thanks a lot @floooh 🚀

@brenocq brenocq merged commit 9205968 into brenocq:dev Dec 24, 2024
4 checks passed
@brenocq brenocq added status:done Task completed successfully and removed status:review The task is under review labels Dec 24, 2024
@floooh floooh deleted the macos-fixes branch December 25, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio:high High priority status:done Task completed successfully type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants