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

Add Linux support (updated) #27

Closed
wants to merge 11 commits into from
Closed

Conversation

cubedtear
Copy link

@cubedtear cubedtear commented Feb 3, 2019

I have based this PR off of #13 and updated it. This PR requires merging other two PRs in submodules, so that the libraries can be built on Linux. These are the PRs:

@DrBarnabus DrBarnabus mentioned this pull request Feb 3, 2019
@MichelMelhem
Copy link

wow

@cubedtear
Copy link
Author

cubedtear commented Feb 5, 2019

I merged a PR made by @DaveAxiom in my fork that adds a README.md file with the necessary steps to build Hazel on Linux (as well as some basic info).
If I recall correctly, @TheCherno metioned in a video the readme should explain how to make issues, PRs and similar things, but I think this is a good starting point.

@cubedtear cubedtear mentioned this pull request Feb 5, 2019
@@ -1,27 +1,37 @@
#pragma once

#ifdef HZ_PLATFORM_WINDOWS
#if defined HZ_PLATFORM_WINDOWS
#define HZ_DEBUG_BREAK { __debugbreak(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be cleaner to use function macros?

Suggested change
#define HZ_DEBUG_BREAK { __debugbreak(); }
#define HZ_DEBUG_BREAK() __debugbreak()

@cubedtear
Copy link
Author

@WhoseTheNerd I did it to be consistent with other macros. For example, HZ_ASSERT is just an if statement with its own braces. However, it is still surrounded by braces. Not how I would do it, but to follow the existing style.

@ritobanrc
Copy link

On Majaro 18.0.3, I am getting

ERROR: ImGui_ImplOpenGL3_CreateDeviceObjects: failed to compile vertex shader!
0:1(10): error: GLSL 4.10 is not supported. Supported versions are: 1.10, 1.20, 1.30, 1.00 ES, 3.00 ES, 3.10 ES, and 3.20 ES

ERROR: ImGui_ImplOpenGL3_CreateDeviceObjects: failed to compile fragment shader!
0:1(10): error: GLSL 4.10 is not supported. Supported versions are: 1.10, 1.20, 1.30, 1.00 ES, 3.00 ES, 3.10 ES, and 3.20 ES

ERROR: ImGui_ImplOpenGL3_CreateDeviceObjects: failed to link shader program! (with GLSL '#version 410
')
error: linking with uncompiled/unspecialized shadererror: linking with uncompiled/unspecialized shader
[1]    16787 segmentation fault (core dumped)  ./Sandbox

This may have something to do with git submodule update --init failing:

error: Server does not allow request for unadvertised object 009d641dc8966e3e61aedb190113cf6513c3d98c
Fetched in submodule path '../../../Hazel/vendor/imgui', but it did not contain 009d641dc8966e3e61aedb190113cf6513c3d98c. Direct fetching of that commit failed.

Any suggestions?

@cubedtear
Copy link
Author

cubedtear commented Feb 26, 2019

@ritobanrc I think they are two separate issues.
On the one hand, regarding the submodules, you will need to add my fork on those as remotes and fetch from them, in order for you to have the needed commits (this will be solved once the PRs in the description are merged).

On the other hand, I have not dedicated enough time to check those shaders, but by the looks of it, your hardware/drivers do not support the necessary OpenGL version. This could be solved by installing the latest drivers for your video card.
If you cannot solve it, open an issue on my fork and we will discuss it there 😊

@jonestristand
Copy link

Wow that's a lot of work, awesome! I do think that there should be a separate LinuxWindow class in platform, rather than modifying the WindowsWindow to work with Linux. Then select which platform-appropriate window class to use based on macros...

@ritobanrc
Copy link

ritobanrc commented Mar 21, 2019

@cubedtear I just re-cloned the project, and the submodule issue seems to have resolved itself. However, I'm still getting the issue with OpenGL and ImGui. This is the output of glxinfo | grep version,

server glx version string: 1.4
client glx version string: 1.4
GLX version: 1.4
    Max core profile version: 4.5
    Max compat profile version: 3.0
    Max GLES1 profile version: 1.1
    Max GLES[23] profile version: 3.2
OpenGL core profile version string: 4.5 (Core Profile) Mesa 18.3.4
OpenGL core profile shading language version string: 4.50
OpenGL version string: 3.0 Mesa 18.3.4
OpenGL shading language version string: 1.30
OpenGL ES profile version string: OpenGL ES 3.2 Mesa 18.3.4
OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.20

I have mesa 18.3.4-1 and xf86-video-intel 1:2.99.917+860+g3a2dec17-1 installed. Any suggestions?

I also tested the example_glfw_opengl2 and example_glfw_opengl3 examples from from ocornut/imgui and TheCherno/imgui, and they both work. Is there a particular reason Hazel appears to be using OpenGL 4?

@marian-bielcik
Copy link

@ritobanrc I had the same issue.
Solution: I downloaded GLAD for GL 3.0 and put it into vendor/Glad folder.
My recommendation: put GLAD for GL 3.0 also into imgui/examples/libs/glad folder - for testing with just the imgui examples.
As far as I know Hazel does not (yet) require OpenGL 4, so for now, you/we should be fine with GLAD for GL 3.0

@Gaztin
Copy link
Collaborator

Gaztin commented May 20, 2019

I suggest keeping the indentation method to tabs like it was before. it will make for a more pleasant diff and will comply with the current style of the engine. It will also be easier to merge.

@Gaztin Gaztin added the OS:Linux Operating system Linux label May 20, 2019
@DaveAxiom
Copy link
Contributor

https://github.com/cubedtear/Hazel/tree/master/Hazel/vendor
I spotted that the GLFW submodule link points to a false commit. The commit hash that the link points to was never pulled into TheCherno's branch! That could begin to explain why cubedtear's branch is faulty.

@NiGhMa
Copy link

NiGhMa commented Jun 29, 2019

@cubedtear Just to let you know I implemented your changes into my code running on Ubuntu 18.04 and it works perfectly! Thanks for your job! I'm working on my own version of the code here (https://github.com/NiGhMa/Hazelicious)

@cubedtear
Copy link
Author

@cubedtear Just to let you know I implemented your changes into my code running on Ubuntu 18.04 and it works perfectly! Thanks for your job! I'm working on my own version of the code here (https://github.com/NiGhMa/Hazelicious)

Thank you for your kind words, @NiGhMa, both in the comment and in the shoutout on your readme! I have been a little absent in this project lately, but I'm truly glad this was helpful 😄

Copy link
Collaborator

@LovelySanta LovelySanta left a comment

Choose a reason for hiding this comment

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

In discord was asked if this can be looked at; I would suggest:

  • update premake files in submodules (see inital post)
  • fast forward the branch (pull master into this) so it can be merged
  • ask someone with write acces to review (gaztin for example)
  • wait till thecherno wants to make a video about this since this is not "just a bugfix"

@@ -1,27 +1,37 @@
#pragma once

#ifdef HZ_PLATFORM_WINDOWS
#if defined HZ_PLATFORM_WINDOWS
#define HZ_DEBUG_BREAK { __debugbreak(); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

As @WhoseTheNerd suggested:

Wouldn't it be cleaner to use function macros?

Suggested change
#define HZ_DEBUG_BREAK { __debugbreak(); }
#define HZ_DEBUG_BREAK() __debugbreak()

Also added these changes in the assert statements

#endif

#ifdef HZ_DEBUG
#define HZ_ENABLE_ASSERTS
#endif

#ifdef HZ_ENABLE_ASSERTS
#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); __debugbreak(); } }
#define HZ_CORE_ASSERT(x, ...) { if(!(x)) { HZ_CORE_ERROR("Assertion Failed: {0}", __VA_ARGS__); __debugbreak(); } }
#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK; } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK; } }
#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK(); } }

#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); __debugbreak(); } }
#define HZ_CORE_ASSERT(x, ...) { if(!(x)) { HZ_CORE_ERROR("Assertion Failed: {0}", __VA_ARGS__); __debugbreak(); } }
#define HZ_ASSERT(x, ...) { if(!(x)) { HZ_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK; } }
#define HZ_CORE_ASSERT(x, ...) { if(!(x)) { HZ_CORE_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK; } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#define HZ_CORE_ASSERT(x, ...) { if(!(x)) { HZ_CORE_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK; } }
#define HZ_CORE_ASSERT(x, ...) { if(!(x)) { HZ_CORE_ERROR("Assertion Failed: {0}", __VA_ARGS__); HZ_DEBUG_BREAK(); } }

@@ -1,14 +1,16 @@
#include "hzpch.h"

// Include GLAD before GLFW, because it otherwise causes a compile error on linux
#include <glad/glad.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of including it before the header file, why not move it into the header file at once; where GLFW and glad can be included next to each other?

@DaveAxiom
Copy link
Contributor

@Gaztin Please close this PR and reopen #13 . This branch was abandoned in February while the parent branch is two weeks behind TheCherno:master.

@LovelySanta
Copy link
Collaborator

As mensioned in the comment TheCherno/imgui#3 (comment), Hazel is currently using the docking branch. In this case, can TheCherno/imgui#4 be added to the todo list in the first post @Gaztin?

@Gaztin
Copy link
Collaborator

Gaztin commented Jul 4, 2019

Done

@Gaztin
Copy link
Collaborator

Gaztin commented Jul 4, 2019

@DaveAxiom It all depends on the PR authors. If the author of #13 (@marian-bielcik) isn't active then there's no reason to reopen it.

@LovelySanta
Copy link
Collaborator

@Gaztin TheCherno/imgui#4 can be checked off in the task list ^_^

@LovelySanta
Copy link
Collaborator

At @DaveAxiom's request I've created new PR's (see references above this comment) in the submodules to expedite the PR's. These are final and can be merged.

Once these are merged by cherno, those tasks can be ticked off.

@LovelySanta LovelySanta mentioned this pull request Jul 11, 2019
7 tasks
@ShantanuJamble
Copy link

@cubedtear I was referring this PR to write a premake file for my repo. In addition to the links you have in the repo, I also needed to put in dl. to know the linking errors I faced you can check out following links,
https://stackoverflow.com/questions/48136263/makefile-for-opengl-with-glad-on-linux.
Hope this is useful. :)

@Gaztin
Copy link
Collaborator

Gaztin commented Aug 2, 2019

Closing since this has been picked up in #82 instead.

@Gaztin Gaztin closed this Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS:Linux Operating system Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet