search

libglui / glui

GLUI is a GLUT-based C++ user interface library which provides controls such as buttons, checkboxes, radio buttons, and spinners to OpenGL applications. It is window-system independent, using GLUT or FreeGLUT.
Other
200 stars 81 forks source link

Issue #90: Adding support for windows 10 SDK #96

Closed T-Jin closed 3 years ago

T-Jin commented 5 years ago

I ran into some issues with building glui on windows 10. Made a quick fix. I later saw the exact same issue was previously reported in issue #90, but no PR yet. So here it is.

nigels-com commented 5 years ago

I think by convention it's better for the client code to include the Windows headers, along with decisions about NOMINMAX and so on. Declining this change.

nigels-com commented 5 years ago

Ah there is more to this story.

There was a change in 2017 to remove the GLUT dependency from the GLUI header. A side-effect of that is that now Windows builds break because GLUT is/was including windows.h

https://github.com/libglui/glui/pull/76

nigels-com commented 5 years ago

Modern GLUT is now going to great lengths to avoid the windows.h dependency... https://github.com/markkilgard/glut/blob/master/include/GL/glut.h

Should GLUI also do the same, at least for (more) compatibility with the previous release?

T-Jin commented 5 years ago

Ah there is more to this story.

There was a change in 2017 to remove the GLUT dependency from the GLUI header. A side-effect of that is that now Windows builds break because GLUT is/was including windows.h

76

I think it was the right move to remove GLUT dependency from GLUI header. I noticed that GLUI Windows build with FreeGLUT also breaks because FreeGLUT is/was including windows.h.

Now knowing that, the original changes I made in this PR is not a good solution, with all that windows namespace pollution.

T-Jin commented 5 years ago

Modern GLUT is now going to great lengths to avoid the windows.h dependency... https://github.com/markkilgard/glut/blob/master/include/GL/glut.h

Should GLUI also do the same, at least for (more) compatibility with the previous release?

Wow, those guys really went through all that effort just to get those macro defined. I would be inclined to do the same for GLUI.

nigels-com commented 4 years ago

Out of interest.

Does this also work for previous Windows and SDK versions, or just the current one?

nigels-com commented 3 years ago

Hello @T-Jin this PR is still active for GLUI, but it looks like you're using the branch for other things. Do you want to restart a review, or happy for this to be closed?

T-Jin commented 3 years ago

Ah, apology for the noise, I should have created the PR on a dedicated branch. I don't have the time to follow it through, so let me close this for now