-
Notifications
You must be signed in to change notification settings - Fork 212
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
Linux Support #1
base: master
Are you sure you want to change the base?
Changes from all commits
ceeb356
7f7ccc2
4ed9f0d
4552282
194faa3
31bea36
1e13057
6204eb9
a2ac1a7
d45e96d
2b0580d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,19 @@ | ||
# Directories | ||
**bin/ | ||
**bin-int/ | ||
!/vendor/bin | ||
run/ | ||
|
||
# Files | ||
*.user | ||
*imgui.ini | ||
Makefile | ||
*.make | ||
|
||
# Visual Studio | ||
.vs/ | ||
.vscode/ | ||
*.sln | ||
*.vcxproj | ||
*.vcxproj.filters | ||
*.vcxproj.user | ||
*.vcxproj.user |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
|
||
#include "Input.h" | ||
|
||
#include <glfw/glfw3.h> | ||
#include <GLFW/glfw3.h> | ||
|
||
namespace GLCore { | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,10 @@ namespace GLCore { | |
s_GLFWInitialized = true; | ||
} | ||
|
||
glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 3); | ||
glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 3); | ||
glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggested addition of: glfwWindowHint(GLFW_OPENGL_FORWARD_COMPAT, GL_TRUE); before the OpenGL profile hint line, because we're using 3.3 which for some OS (macOS) may require core context compatibility-mode enabled. This is due to Apple's move on starting to deprecate a bunch of APIs - OpenGL, OpenCL, etc. in favor of Metal. I am aware that this MR is solely about Linux support, but just wanted to mention a point of consideration about GL context support:
I am only suggesting this because I've done some of the work on my Mac at some point and had to adjust a couple of things. It is up to you and maybe @TheCherno if he'd like to have a look, but you can use some PLATFORM macro at config time and add this for macOS only, or not do anything but keep in mind for future reference. Also, from the message of the commit adding the above hints:
wrt macOS but also on API support general: On macOS you might consider throwing an error message or something or just assert the version for the time being as creating an OpenGL core context below 3.1 is not supported at all. So maybe either disallow < 3.3 if we're not gonna consider API compatibility for these versions or wrap it around a PLATFORM macro too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not well versed on the specifics of OpenGL. I know that there are different OpenGL profiles but I don't know about the specifics of each. I held off on accepting the Hints PR into this branch until now since I rationalized that it is a minimum that can be just a starting point. Cherno plans to make videos on Apple support. My philosophy behind Linux support is to increase the portability of this project. Apple support is good but the actions of that company are intended to close their platform in order to add proprietary value to their Intellectual Properties. Therefore anyone that buys an Apple shouldn't expect compatibility and I'm not going to dive in further to engineer around Apple. If this PR is accepted, a future PR could be submitted to modify the new base project. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, I guess I just threw this out here for informative reasons. What you're saying makes sense. :) |
||
|
||
m_Window = glfwCreateWindow((int)props.Width, (int)props.Height, m_Data.Title.c_str(), nullptr, nullptr); | ||
|
||
glfwMakeContextCurrent(m_Window); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#!/bin/bash | ||
|
||
SCRIPTS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" | ||
cd "$SCRIPTS_DIR/.." | ||
|
||
vendor/bin/premake/premake5 gmake2 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
#!/bin/bash | ||
|
||
SCRIPTS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" | ||
cd "$SCRIPTS_DIR/.." | ||
|
||
mkdir -p run | ||
|
||
rm -rf run/assets | ||
cp -rf OpenGL-Sandbox/assets run | ||
|
||
cd run | ||
|
||
../bin/Debug-linux-x86_64/OpenGL-Sandbox/OpenGL-Sandbox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that Yan is using the token-pasting operator (##) to concat EventType:: with whatever is the name pasted as type argument, so I wouldn't remove the operator here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
##
operator is proprietary to Microsoft's compiler!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I am not so sure about it tho as I have used it with gcc and clang professionally under various architecture implementations of the compiler standard. Here's an example code that makes use of the
##
operator compiled usinggcc-7
: https://godbolt.org/z/TEGszsAlso, the cppreference preprocessor docs don't seem to say anything about compiler specifics: https://en.cppreference.com/w/cpp/preprocessor/replace
And I have personally compiled the code under clang and gcc on both Linux (gcc and clang) and MacOS (clang) as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this operator on Godbolt during the summer and found that
##
works on the most recent few major versions of GCC. The above is::##
which is different.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sweet, well not cool but ... 😆 I am wondering what could've made this compile fine for me, as far as I remember tho.
I guess there's nothing more that can be done for the above compiler feature support war. I've been misled :(