Add support of MacOS

[MrOnlineCoder]

This is a one-hell a huge PR that includes 3 features at once:

• Switch to using FetchContent in CMake for downloading dependencies
• Implementation of OpenGL 4.1 RHI backend
• Other fixes needed to get engine and sandbox running on MacOS

I am attaching the screenshot that it's working.

I am sorry in advance for a) big PR b) I maybe written some code or made changes that do not fit your code style and conventions, in this case please let me know what to fix.

This PR is still a WIP/draft because for example I've got the PrimitivesSandbox running, but failed to get MeshModelSandbox up.

I will leave all my questions/concerns that require some discussion from you (@denyskryvytskyi) as comments

P.S. even if this does not get merged - it was fun at least as a challenge :D

[MrOnlineCoder]

Multisampled textures are not supported in OpenGL 4.1 explicitly, so not sure if there is an equivalent.

[denyskryvytskyi]

It should be supported according to this documentation (btw, useful reference for available functions). Try the following code (worked for me):

glCheck(glGenTextures(1, &m_id));

glCheck(glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_id));

glCheck(glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 4, m_internalFormat, width, height, GL_TRUE));

glCheck(glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0));

Also don't forget to set a correct texture type in OpenGL41RenderTarget::InitAttachments -> glFramebufferTexture2D

[MrOnlineCoder]

This was very annoying - emplace_back here caused my compiler to spit out unreadable errors complaining about "construct_at" internal method, and according to some info online, this may be a bug in compiler itself or MacOS SDK. So had to use the old way here.

[denyskryvytskyi]

Approved. Just remove the comment, please.

[denyskryvytskyi]

Well, I've debugged a little bit. This is now causing a "white texture" problem for the models. Because StaticMeshComponent is subscribed on the model-loaded event in the constructor pushing a copy of the created object invalidates this pointer in the event handler. I'm not sure how to fix it for now, but creating a copy is not a good idea here.

[MrOnlineCoder]

Lastly, there are few open points not really related to code:

1. I assume that Platform/Windows should've had Win32-specific implementation of window creation and management, however, because you are using GLFW which is cross-platform by itself, I've re-used it for MacOS and didn't find any Windows-specific usage there. Do you think, in order to avoid further confusion, we should a) rename it to something like "GLFW" b) create a complete copy but explicitly name it "MacOS" c) something else? I also suppose it will work for Linux too out-of-the-box

2. Current shaders code uses GLSL version 450 which is, surely, not supported in 4.1 (410). It also uses binding feature, which is not supported in older GL. I've adjusted the shaders to get the sandbox running, however not sure how we should maintain it - create a copy of shaders/ code that will be copied by CMake depending on the platform, or downgrade existing shaders? (modified shaders are not included in PR because I've did it in build output folder, which is git-ignored)

3. Sprite fragment shader had hardcoded amount of texture samplers:

uniform sampler2D u_Textures[32];

However my system supports only 16 of them (this was printed as a specific message from shader compiler), so I had to lower it to 16. Not sure what we should do about it.

4. Important final note: this branch may be completely broken for you or your setup (compiler/platform/IDE) so if possible, please test it in an somewhat isolated environment to see if it at least compiles on Windows and is still able to use the newer rendering backend.

[MrOnlineCoder]

@denyskryvytskyi I have pushed changes to CMakeLists configuration, and have successfully achieved Sandbox3D app running on Windows 10 (VS 2022), with irrklang too.

During this I have also found that post-build things like copying irrklang dlls are configured in each project separately, i.e. only the Sandbox3D cmakelists are correct now. Can we somehow maybe move this to Engine build script itself, so the engine-projects will only have to copy their assets?

[denyskryvytskyi]

@MrOnlineCoder Here is my thoughts about different moments:

1. Platform-specific Window implementation. You're right, I think we can rename Platform/WindowsWindow.h ->Platform/GLFWWindow.h
2. Shaders. I'm not sure for now what to do with shaders. Let me think a little bit about it.
3. About hardcoded amount of samplers in fragment shader for sprite. Yeah, decreasing it to 16 will be enough for now.
4. About copying DLLs, assets, and other stuff. I think we can use copy-paste for sandboxes and game projects for this PR. I'll think about reducing it in the context of a separate task (or you can make suggestions if you will).
5. GL-specific error on application closing (only for OpenGL 4.1.). I have this error only for the OpenGL 4.1. (you may have the same issue). Actually, I think this is because the window context is destroyed before calling destructors of RHI objects like Buffer, Texture, etc. I'm not sure why this exists only for the 4.1 version, but I think it should be fixed with the architecture in mind - OpenGL objects have to call destructors only before window context destroying. I'll think about how to fix it.
   
6. Also there is a problem with MeshModelSandbox: white textures for OpenGL 4.5. version. According to the log textures aren't loaded at all. Maybe the Assimp texture parsing is broken. Can't say anything, need debugging.

[denyskryvytskyi]

It should be supported according to this documentation (btw, useful reference for available functions). Try the following code (worked for me):

glCheck(glGenTextures(1, &m_id));

glCheck(glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, m_id));

glCheck(glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, 4, m_internalFormat, width, height, GL_TRUE));

glCheck(glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, 0));

Also don't forget to set a correct texture type in OpenGL41RenderTarget::InitAttachments -> glFramebufferTexture2D

[denyskryvytskyi]

Approved. Just remove the comment, please.

[MrOnlineCoder]

@denyskryvytskyi thanks for the link on multisample textures, I'll try implementing that (interesting that I didn't find that on official Khronos wiki for some reason), and will test on my Mac machine

I just have two clarifications:

1. Renaming WindowsWindow to GLFWWindow may cause confusions or name collisions, as there is a struct from glfw itself - GLFWwindow - the difference is just the uppercase "W" letter, which I am just intuitevly not reallty into. What about GLFWWindowImpl or something else? Surely it's more up to you, I am just thinking out loud, as most of Elven code is written inside namespace elv {...} blocks, which makes making a typo like that possible.

2. Do I understand correctly that Core/Input.h is implemented as a set of static functions for convenient usage in other classes (components especially), and not as an abstract class that is defined by, let's say Platform/GLFW? As I understand in that case, changing then the GLFWWindow to some other Window implementation would not be enough to cover window/input handling, you would also need to define other implementations for Input statics, and also disable the compilation of GLFW-ones via macros for example.
   Although I surely doubt the need of writing another window/input handling backend, as glfw is pretty solid and cross-platform solution, and therefore such exception would be fine enough, I am just wondering if you would like to standartize that somehow.

Thanks in advance

[denyskryvytskyi]

• I've submitted a fix for the "GL-specific error on application closing" problem to the main branch 209fb47
• About GLFW-specific implementations. Well, a fair point from you. We would need to make all GLFW usage in the separate Window/Input platform-specific code. But, as GLFW is cross-platform and is used for Opengl and will be used for Vulkan, I would not touch this for now. It could be revisited when integrating DirectX at least. So, Just renaming the WindowsWindow -> GlfwWindowImpl is enough for now. Also, we need to rename WindowsInput ->GlfwInputImpl I think, but I can do this in the main branch.

[MrOnlineCoder]

@denyskryvytskyi I've pushed some changes:

• GraphicsContext now accepts window pointer in Init() function instead of constructor
• I've added support for multi sample textures, however, seems I cannot use them as attachment to framebuffer in RenderTarget. I must note that I "suck" in framebuffer APIs, so maybe there is another way for that, please let me know - OpenGL seems to have multiple ways of doing that. So currently I use regular 2D textures in InitAttachments - this way the sandbox app works
• Renamed WindowsWindow and WindowsInput to GLFWWindowImpl and GLFWInputImpl respectively

[denyskryvytskyi]

Well, I've debugged a little bit. This is now causing a "white texture" problem for the models. Because StaticMeshComponent is subscribed on the model-loaded event in the constructor pushing a copy of the created object invalidates this pointer in the event handler. I'm not sure how to fix it for now, but creating a copy is not a good idea here.

[denyskryvytskyi]

Try to write m_isMultisampled ? GL_TEXTURE_2D_MULTISAMPLE : GL_TEXTURE_2D, here and set info.Multisample. It works for me for OpenGL41 API. If it doesn't work on Mac, then I think we can skip MSAA for opengl41.

[MrOnlineCoder]

AFAIK, I have tried exactly that and didn't succeed. Although the pre-debugcallback error messages are not very descriptive (and not always documented), so maybe I missed something - I will try again, and if not, no MSAA would be available for opengl41 for now.

[denyskryvytskyi]

Thoughts about shaders: it's better to make a separate folder for older version of shaders and adapt code for that.

[MrOnlineCoder]

@denyskryvytskyi

Because StaticMeshComponent is subscribed on the model-loaded event in the constructor pushing a copy of the created object invalidates this pointer in the event handler. I'm not sure how to fix it for now, but creating a copy is not a good idea here.

I am a bit weak on newer C++ features and typing system, so I really can't help there directly. I will try researching for a way to get this compilation error fixed on MacOS (maybe by changing the build toolchain)

Thoughts about shaders: it's better to make a separate folder for older version of shaders and adapt code for that.

Would you like me to try doing that in current PR, or do you have any specific vision on the implementation itself?

[denyskryvytskyi]

Well, I don't think shaders duplication/adaptation has any meaning out of this PR.
There are two options you could do:

1. No duplication. Remove the binding feature from the shaders (and adapt C++ code, we need just bind these textures), downgrade the version, and test it. If it will work that is enough.
2. Duplication [Not desirable because for every new shader or refactoring one needs us to do the same for the old one] if the first point doesn't work and other new features aren't supported. In this case, we need to take the following actions:
   • Make another folder in assets like shadersGL41
   • adapt shaders for OpenGL 4.1
   • Copy the appropriate folder to the binary shader folder according to the platform. I think this is enough for now.

Anyway, both points involve C++ code adaptation, so the first one is better if It doesn't require a lot of downgrade.

[MrOnlineCoder]

@denyskryvytskyi sorry for delay, having a very busy week, I will try to push some changes closer to the weekend