Skip to content

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

Merged
brenocq merged 3 commits intobrenocq:devfrom
floooh:macos-fixes
Dec 24, 2024
Merged

Fix: Minimal changes needed to make the example build and run on macOS#7
brenocq merged 3 commits intobrenocq:devfrom
floooh:macos-fixes

Conversation

@floooh
Copy link
Copy Markdown
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
Copy Markdown
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:fix Something isn't working prio:high High priority status:review The task is under review labels Dec 22, 2024
Copy link
Copy Markdown
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

Comment thread implot3d.cpp Outdated
@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
Copy Markdown
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
@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
@brenocq brenocq changed the title Minimal changes needed to make the example build and run on macOS Fix: Minimal changes needed to make the example build and run on macOS May 25, 2025
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:fix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants