ConfettiFX / The-Forge

The Forge Cross-Platform Rendering Framework PC Windows, Steamdeck (native), Ray Tracing, macOS / iOS, Android, XBOX, PS4, PS5, Switch, Quest 2
Apache License 2.0
4.71k stars 493 forks source link

CMakeLists.txt really needed #158

Closed Hideman85 closed 4 years ago

Hideman85 commented 4 years ago

Well first of all your library look awesome and I like samples you wrote that bring a view of how to use your lib.

I like also also the support of different IDE like VisualStudio ode Codelite but personally I use CLion and also like after cloning a lib like this just do mkdir build && cd build && cmake .. && make -j12 for quickly test the project but your project doesn't support it.

Cmake is a very powerful tool for building a project, managing dependencies, use your lib as dependencies (that we can also integrate as git submodule of in our project), cross compilation via toolchains or also generate project for other IDE.

The following generators are available on this platform (* marks default):
* Unix Makefiles               = Generates standard UNIX makefiles.
  Green Hills MULTI            = Generates Green Hills MULTI files
                                 (experimental, work-in-progress).
  Ninja                        = Generates build.ninja files.
  Watcom WMake                 = Generates Watcom WMake makefiles.
  CodeBlocks - Ninja           = Generates CodeBlocks project files.
  CodeBlocks - Unix Makefiles  = Generates CodeBlocks project files.
  CodeLite - Ninja             = Generates CodeLite project files.
  CodeLite - Unix Makefiles    = Generates CodeLite project files.
  Sublime Text 2 - Ninja       = Generates Sublime Text 2 project files.
  Sublime Text 2 - Unix Makefiles
                               = Generates Sublime Text 2 project files.
  Kate - Ninja                 = Generates Kate project files.
  Kate - Unix Makefiles        = Generates Kate project files.
  Eclipse CDT4 - Ninja         = Generates Eclipse CDT 4.0 project files.
  Eclipse CDT4 - Unix Makefiles= Generates Eclipse CDT 4.0 project files.

I can also found related issues (https://github.com/ConfettiFX/The-Forge/issues/66, https://github.com/ConfettiFX/The-Forge/issues/81, https://github.com/ConfettiFX/The-Forge/issues/138) on your project that request also a cmake version of your lib.

I hope this new request will be solve by the presence of a CMakeLists.txt in your repo.

wolfgangfengel commented 4 years ago

Hey @Hideman85, we don't want to support more IDEs. It is already pretty tough to support the ones we have with our automated testing. I think we need to spend time in automated testing for things that are more important. You have to imagine that every time someone wants to merge code, the automated test kicks off and tests all platforms. In case we would support more IDEs, we would have to add those. For us there is no benefit to do this. More the opposite, we would be loosing a lot of time testing all this. Please go ahead fork our repository and create your CMake support. We will redirect everyone to your fork in that case ...

Hideman85 commented 4 years ago

Hi @wolfgangfengel I completely understand your point but in the other hand it's very hard to integrate your library in a projet. I don't mean your unit tests that have one file that describe what it just need to describe (the goal of a unit test) but for project that need to more that just a graphic lib it would be very great to have a CMakeLists.txt that setup your lib and build it as shared or static lib.

I don't know how you proceed your automated tests on different platform but toolchain feature of CMake that allow cross compiling is also a very good point for automated testing.

Hideman85 commented 4 years ago

HI @wolfgangfengel after working on implementing a CMake solution inside your project I can give you this feedback.

The first bad things that I saw on your project is the way that libraries are imported:

#include "../../ThirdParty/OpenSource/EASTL/array.h"
#include "../../ThirdParty/OpenSource/gainput/lib/include/gainput/gainput.h"

Why you using full relative path? The project should have dependencies include directories link to it and use as:

#include <EASTL/array.h>
#include <gainput/gainput.h>

This is the same issue when using your library. Look at one of your samples:

//Interfaces
#include "../../../../Common_3/OS/Interfaces/ICameraController.h"
#include "../../../../Common_3/OS/Interfaces/IApp.h"
#include "../../../../Common_3/OS/Interfaces/ILog.h"
#include "../../../../Common_3/OS/Interfaces/IInput.h"
// Should be
//Interfaces
#include <TheForge/OS/Interfaces/ICameraController.h>
#include <TheForge/OS/Interfaces/IApp.h>
#include <TheForge/OS/Interfaces/ILog.h>
#include <TheForge/OS/Interfaces/IInput.h>

For that we should have a include directory at root directory (or at least built during the process by copying header files in proper location). You can also expose includes from your deps libraries if needed.

include/
├── assimp
│   ├── ai_assert.h
│   ├── anim.h
│   ├── BaseImporter.h
│   └── ...
├── EASTL
│   ├── algorithm.h
│   ├── allocator_forge.h
│   ├── allocator.h
│   ├── allocator_malloc.h
│   └── ...
├── gainput
│   ├── GainputAllocator.h
│   ├── GainputContainers.h
│   ├── GainputDebugRenderer.h
│   ├── gainput.h
│   └── ...
└── TheForge
    ├── OS
    │   ├── Core
    │   │   ├── Atomics.h
    │   │   ├── Compiler.h
    │   │   ├── DLL.h
    │   │   ├── GPUConfig.h
    │   │   ├── RingBuffer.h
    │   │   └── ThreadSystem.h
    │   ├── FileSystem
    │   ├── Image
    │   └── ...
    └── Renderer
        ├── Direct3D11
        │   ├── Direct3D11CapBuilder.h
        │   └── Direct3D11Commands.h
        └── ...

Imagine in a real project like this:

MySuperProject/
├── Libs
│   ├── The-Forge // git submodule
│   └── ...
└── src
    ├── Core
    │   ├── Renderer
    │   │   ├── MyRenderer.h
    │   │   └── MyRenderer.cpp
    │   └── ...
    └── ...

The MyRenderer.h file will look like:

// In your case would be
#include "../../../Libs/The-Forge/Common_3/Renderer/IRenderer.h"
#include "../../../Libs/The-Forge/Common_3/ThirdParty/OpenSource/gainput/lib/include/gainput/gainput.h"
// Instead of
#include <TheForge/Renderer/IRenderer.h>
#include <gainput/gainput.h>

The second aspect in your project is you modified the code of your deps like:

// Common_3/ThirdParty/OpenSource/EASTL/allocator_forge.cpp
#include "../../../OS/Interfaces/IMemory.h"

The really bad things is you lost track of updates made by authors of these libraries. Imagine tomorrow one of these libs release a fix of a infinite loop that impact several files you need to manually track these updates into your copy of that lib. Two good ways to do this:

Last things, I also saw a include like this:

#include "../../../../../../PS4/Common_3/ThirdParty/OpenSource/hlslparser/OrbisGenerator.cpp"

So I suppose you have a private repo for PS4, XBox, etc that you add "accredited developers" in read mode to these repos. These extensions can also be managed as git submodule and have option to enable theirs compilation in CMakeLists.txt

If you look to similar libraries in the same fields they have CMake for compilation, cmake toolchains for cross compilation and include directory (or at least build it during the process) for correctly using theirs libs:

And to finish I started to implement CMake build system but I'm encountered some issues during the linking phase that need more investigate by comparing with what you have set for UbuntuCodelite projects. https://github.com/Hideman85/The-Forge

I don't know if you will care about my feedback but it is really not trivial to use your lib in a external project. I hope it will change your mind on certain aspect.

wolfgangfengel commented 4 years ago

Just saw this. Let me know when you have the CMake version up and we will add it to the Wiki page ... so we will direct people your way. As soon as you are finished, we would use

https://github.com/Hideman85/The-Forge

in that case?

Hideman85 commented 4 years ago

That's what I was afraid of, you just want to refer me for all people who want a cmake support instead of taking in consideration my feedback. In fact, I don't want to maintain a conversion project for your library; I prefer to show you how to improve your current structure in order to be able to use your library in a conceivable way. I will work on it this weekend because I will have more time for.

Hideman85 commented 4 years ago

No way linking errors that I have is always X11 & Vulkan

/usr/bin/ld: LinuxBase.cpp:(.text+0x3bc): undefined reference to `XOpenDisplay'
/usr/bin/ld: LinuxBase.cpp:(.text+0x43f): undefined reference to `XGetVisualInfo'
/usr/bin/ld: LinuxBase.cpp:(.text+0x483): undefined reference to `XCreateColormap'
/usr/bin/ld: LinuxBase.cpp:(.text+0x572): undefined reference to `XCreateWindow'
/usr/bin/ld: LinuxBase.cpp:(.text+0x5a8): undefined reference to `XStoreName'
/usr/bin/ld: LinuxBase.cpp:(.text+0x60d): undefined reference to `XSetClassHint'
/usr/bin/ld: LinuxBase.cpp:(.text+0x632): undefined reference to `XSelectInput'
/usr/bin/ld: LinuxBase.cpp:(.text+0x652): undefined reference to `XMapWindow'
/usr/bin/ld: LinuxBase.cpp:(.text+0x664): undefined reference to `XFlush'
/usr/bin/ld: LinuxBase.cpp:(.text+0x682): undefined reference to `XInternAtom'
/usr/bin/ld: LinuxBase.cpp:(.text+0x6ba): undefined reference to `XSetWMProtocols'
/usr/bin/ld: LinuxBase.cpp:(.text+0x6bf): undefined reference to `XAllocSizeHints'
/usr/bin/ld: LinuxBase.cpp:(.text+0x708): undefined reference to `XSetWMNormalHints'
/usr/bin/ld: LinuxBase.cpp:(.text+0x714): undefined reference to `XFree'

/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4c6f): undefined reference to `cmdSetScissor(Cmd*, unsigned int, unsigned int, unsigned int, unsigned int)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4c85): undefined reference to `cmdBindPipeline(Cmd*, Pipeline*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4ca6): undefined reference to `cmdBindIndexBuffer(Cmd*, Buffer*, unsigned long)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4cd2): undefined reference to `cmdBindVertexBuffer(Cmd*, unsigned int, Buffer**, unsigned long*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4cf8): undefined reference to `cmdBindDescriptorSet(Cmd*, unsigned int, DescriptorSet*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4e5f): undefined reference to `cmdSetScissor(Cmd*, unsigned int, unsigned int, unsigned int, unsigned int)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4fce): undefined reference to `updateDescriptorSet(Renderer*, unsigned int, DescriptorSet*, unsigned int, DescriptorData const*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x4ff0): undefined reference to `cmdBindDescriptorSet(Cmd*, unsigned int, DescriptorSet*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x5054): undefined reference to `cmdBindDescriptorSet(Cmd*, unsigned int, DescriptorSet*)'
/usr/bin/ld: ImguiGUIDriver.cpp:(.text+0x5072): undefined reference to `cmdDrawIndexed(Cmd*, unsigned int, unsigned int, unsigned int)'

I tried to link with:

##########################################
##      System deps
##########################################
find_package(Vulkan REQUIRED)
include_directories(${Vulkan_INCLUDE_DIR})
link_libraries(${Vulkan_LIBRARIES})

if (UNIX)
    find_package(X11 REQUIRED)
    find_package(ALSA REQUIRED) # sudo apt install libclalsadrv-dev
    include_directories(${X11_INCLUDE_DIR} ${ALSA_INCLUDE_DIR})
    link_libraries(${X11_LIBRARIES} ${ALSA_LIBRARIES} -pthread)
endif()

Or just by

link_libraries(-lX11 -lvulkan -pthread)

Nothing works :( any idea?

EDIT: For vulkan I found I miss to add the definition VULKAN that you used in code for importing headers so solved by:

add_definitions(-DVULKAN=TRUE)

Still looking what's wrong with X11

boberfly commented 4 years ago

Hi @Hideman85

Not sure if this helps but this works ok I used Linux+X11+Vulkan as my primary test machine: https://github.com/boberfly/The-Forge-Lite/blob/master/CMakeLists.txt

It's an old version of The-Forge though, I haven't maintained it but the CMake should work...

Cheers.

Hideman85 commented 4 years ago

Thanks for your resources, your repo looks good but as you said not updated. I still think it would be good if they make some effort to change certain things that has already reported many times. In my case I just don't know why I have issues on X11 because it is found by cmake and linked to the executable. If you have some time for looking into my repo and helps me to solves these linking issues I would be grateful ;)

boberfly commented 4 years ago

Maybe try re-ordering your target links? The linker is sensitive to ordering afaik.

Hideman85 commented 4 years ago

Your right I didn't figure out that all shared libs must be after static ones. That solve my issue no it is easier to fix the rest. Thanks a lot for your suggestion ;)

Hideman85 commented 4 years ago

Only one last error that is not comprehensible ...

/usr/bin/ld: /tmp/Visibility_Buffer.8q0oyR.ltrans39.ltrans.o: in function `Assimp::IRRImporter::InternReadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, aiScene*, Assimp::IOSystem*)':
The-Forge/Common_3/ThirdParty/OpenSource/assimp/4.1.0/code/IRRLoader.cpp:913: undefined reference to `irr::io::createIrrXMLReader(irr::io::IFileReadCallBack*)'
/usr/bin/ld: /tmp/Visibility_Buffer.8q0oyR.ltrans40.ltrans.o: in function `Assimp::IRRMeshImporter::InternReadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, aiScene*, Assimp::IOSystem*)':
The-Forge/Common_3/ThirdParty/OpenSource/assimp/4.1.0/code/IRRMeshLoader.cpp:148: undefined reference to `irr::io::createIrrXMLReader(irr::io::IFileReadCallBack*)'
/usr/bin/ld: /tmp/Visibility_Buffer.8q0oyR.ltrans43.ltrans.o: in function `Assimp::ColladaLoader::InternReadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, aiScene*, Assimp::IOSystem*)':
The-Forge/Common_3/ThirdParty/OpenSource/assimp/4.1.0/code/ColladaParser.cpp:101: undefined reference to `irr::io::createIrrXMLReader(irr::io::IFileReadCallBack*)'
collect2: error: ld returned 1 exit status

nm lib/libIrrXML.a | grep createIrrXMLReader
00000000 T _ZN3irr2io18createIrrXMLReaderEP8_IO_FILE
00000000 T _ZN3irr2io18createIrrXMLReaderEPKc
00000000 T _ZN3irr2io18createIrrXMLReaderEPNS0_17IFileReadCallBackE
00000000 T _ZN3irr2io23createIrrXMLReaderUTF16EP8_IO_FILE
00000000 T _ZN3irr2io23createIrrXMLReaderUTF16EPKc
00000000 T _ZN3irr2io23createIrrXMLReaderUTF16EPNS0_17IFileReadCallBackE
00000000 T _ZN3irr2io23createIrrXMLReaderUTF32EP8_IO_FILE
00000000 T _ZN3irr2io23createIrrXMLReaderUTF32EPKc
00000000 T _ZN3irr2io23createIrrXMLReaderUTF32EPNS0_17IFileReadCallBackE

nm lib/libassimp.a | grep createIrrXMLReader
U _ZN3irr2io18createIrrXMLReaderEPNS0_17IFileReadCallBackE

nm libTheForge.a | grep createIrrXMLReader
U _ZN3irr2io18createIrrXMLReaderEPNS0_17IFileReadCallBackE

IrrXML is a dep of Assimp it self a dep of TheForge that is dep of the final executable. Assimp provide a proper CMakeLists.txt that is used by add_subdirectory(assimp/4.1.0). What can I do?

Hideman85 commented 4 years ago

Just need to link manually IrrXML target I don't know why but it solves this issue ... So fine all test cases and Visibility buffer finally compile correctly. I will look for cross compiling and improving compilation by adding option for compiling part of the lib. But you can have a looks to current changes how they impact changes.

Hideman85 commented 4 years ago

I'm starting to try cross compilation process and my first try is with MingW64 and I already fix some issues mainly due to bad imports but I have some issues related with The-Forge itself:

[ 67%] Building CXX object CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsThread.cpp.obj
/work/Common_3/OS/FileSystem/SystemFileStream.cpp: In function 'ssize_t ftello(FILE*)':
/work/Common_3/OS/FileSystem/SystemFileStream.cpp:36:28: error: ambiguating new declaration of 'ssize_t ftello(FILE*)'
 ssize_t ftello(FILE* stream) { return (ssize_t)_ftelli64(stream); }
                            ^
In file included from /work/Common_3/OS/FileSystem/../Interfaces/../Interfaces/IOperatingSystem.h:62:0,
                 from /work/Common_3/OS/FileSystem/../Interfaces/IFileSystem.h:28,
                 from /work/Common_3/OS/FileSystem/FileSystemInternal.h:28,
                 from /work/Common_3/OS/FileSystem/SystemFileStream.h:25,
                 from /work/Common_3/OS/FileSystem/SystemFileStream.cpp:27:
/usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/stdio.h:636:10: note: old declaration '_off_t ftello(FILE*)'
   _off_t ftello(FILE * stream);
          ^
/work/Common_3/OS/FileSystem/SystemFileStream.cpp: In constructor 'SystemFileStream::SystemFileStream(FILE*, FileMode, bool)':
/work/Common_3/OS/FileSystem/SystemFileStream.cpp:56:46: error: call of overloaded 'fseeko(FILE*&, int, int)' is ambiguous
     if (ownsFile && fseeko(pFile, 0, SEEK_END) == 0)
                                              ^
In file included from /work/Common_3/OS/FileSystem/../Interfaces/../Interfaces/IOperatingSystem.h:62:0,
                 from /work/Common_3/OS/FileSystem/../Interfaces/IFileSystem.h:28,
                 from /work/Common_3/OS/FileSystem/FileSystemInternal.h:28,
                 from /work/Common_3/OS/FileSystem/SystemFileStream.h:25,
                 from /work/Common_3/OS/FileSystem/SystemFileStream.cpp:27:
/usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/stdio.h:634:7: note: candidate: int fseeko(FILE*, _off_t, int)
   int fseeko(FILE* stream, _off_t offset, int whence);
       ^
/work/Common_3/OS/FileSystem/SystemFileStream.cpp:34:5: note: candidate: int fseeko(FILE*, ssize_t, int)
 int fseeko(FILE* stream, ssize_t offset, int origin) { return _fseeki64(stream, offset, origin); }
     ^
CMakeFiles/TheForge-static.dir/build.make:133: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/FileSystem/SystemFileStream.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/FileSystem/SystemFileStream.cpp.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
/work/Common_3/OS/Windows/WindowsLog.cpp: In function 'void _FailedAssert(const char*, int, const char*)':
/work/Common_3/OS/Windows/WindowsLog.cpp:83:5: error: '_asm' was not declared in this scope
     _asm int 0x03;  
     ^
/work/Common_3/OS/Windows/WindowsThread.cpp: In member function 'bool ConditionVariable::Init(const char*)':
/work/Common_3/OS/Windows/WindowsThread.cpp:62:58: error: 'InitializeConditionVariable' was not declared in this scope
  InitializeConditionVariable((PCONDITION_VARIABLE)pHandle);
                                                          ^
/work/Common_3/OS/Windows/WindowsThread.cpp: In member function 'void ConditionVariable::Wait(const Mutex&, uint32_t)':
/work/Common_3/OS/Windows/WindowsThread.cpp:73:94: error: 'SleepConditionVariableCS' was not declared in this scope
  SleepConditionVariableCS((PCONDITION_VARIABLE)pHandle, (PCRITICAL_SECTION)&mutex.mHandle, ms);
                                                                                              ^
/work/Common_3/OS/Windows/WindowsThread.cpp: In member function 'void ConditionVariable::WakeOne()':
/work/Common_3/OS/Windows/WindowsThread.cpp:78:52: error: 'WakeConditionVariable' was not declared in this scope
  WakeConditionVariable((PCONDITION_VARIABLE)pHandle);
                                                    ^
/work/Common_3/OS/Windows/WindowsThread.cpp: In member function 'void ConditionVariable::WakeAll()':
/work/Common_3/OS/Windows/WindowsThread.cpp:83:55: error: 'WakeAllConditionVariable' was not declared in this scope
  WakeAllConditionVariable((PCONDITION_VARIABLE)pHandle);
                                                       ^
CMakeFiles/TheForge-static.dir/build.make:693: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsLog.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsLog.cpp.obj] Error 1
CMakeFiles/TheForge-static.dir/build.make:889: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsThread.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsThread.cpp.obj] Error 1
/work/Common_3/OS/Input/InputSystem.cpp:861:24: error: enclosing class of constexpr non-static member function 'bool InputSystemImpl::IsPointerType(gainput::DeviceId)' is not a literal type
  inline constexpr bool IsPointerType(gainput::DeviceId device)
                        ^
/work/Common_3/OS/Input/InputSystem.cpp:101:8: note: 'InputSystemImpl' is not literal because:
 struct InputSystemImpl : public gainput::InputListener
        ^
/work/Common_3/OS/Input/InputSystem.cpp:101:8: note:   'InputSystemImpl' has a non-trivial destructor
CMakeFiles/TheForge-static.dir/build.make:203: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/Input/InputSystem.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/Input/InputSystem.cpp.obj] Error 1
/work/Common_3/OS/Windows/WindowsBase.cpp: In function 'void openWindow(const char*, WindowsDesc*)':
/work/Common_3/OS/Windows/WindowsBase.cpp:371:52: error: invalid conversion from 'const char*' to 'size_t {aka long long unsigned int}' [-fpermissive]
  mbstowcs_s(&charConverted, app, app_name, MAX_PATH);
                                                    ^
/work/Common_3/OS/Windows/WindowsBase.cpp:371:52: error: invalid conversion from 'int' to 'const char*' [-fpermissive]
/work/Common_3/OS/Windows/WindowsBase.cpp:371:52: error: too few arguments to function 'errno_t mbstowcs_s(size_t*, wchar_t*, size_t, const char*, size_t)'
In file included from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/stdlib.h:740:0,
                 from /usr/src/mxe/usr/lib/gcc/x86_64-w64-mingw32.static.posix/5.5.0/include/mm_malloc.h:27,
                 from /usr/src/mxe/usr/lib/gcc/x86_64-w64-mingw32.static.posix/5.5.0/include/xmmintrin.h:34,
                 from /usr/src/mxe/usr/lib/gcc/x86_64-w64-mingw32.static.posix/5.5.0/include/x86intrin.h:31,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/winnt.h:1554,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/minwindef.h:163,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/windef.h:8,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/windows.h:69,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/rpc.h:16,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/objbase.h:7,
                 from /usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/shlwapi.h:16,
                 from /work/Common_3/OS/Windows/WindowsBase.cpp:33:
/usr/src/mxe/usr/x86_64-w64-mingw32.static.posix/include/sec_api/stdlib_s.h:23:27: note: declared here
   _CRTIMP errno_t __cdecl mbstowcs_s(size_t *_PtNumOfCharConverted,wchar_t *_DstBuf,size_t _SizeInWords,const char *_SrcBuf,size_t _MaxCount);
                           ^
CMakeFiles/TheForge-static.dir/build.make:665: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsBase.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsBase.cpp.obj] Error 1
/work/Common_3/OS/Windows/WindowsFileSystem.cpp: In function 'Path* fsCopyPreferencesDirectoryPath(const char*, const char*)':
/work/Common_3/OS/Windows/WindowsFileSystem.cpp:312:48: error: 'KF_FLAG_CREATE' was not declared in this scope
  SHGetKnownFolderPath(FOLDERID_RoamingAppData, KF_FLAG_CREATE, NULL, &preferencesDir);
                                                ^
/work/Common_3/OS/Windows/WindowsFileSystem.cpp:312:85: error: 'SHGetKnownFolderPath' was not declared in this scope
  SHGetKnownFolderPath(FOLDERID_RoamingAppData, KF_FLAG_CREATE, NULL, &preferencesDir);
                                                                                     ^
/work/Common_3/OS/Windows/WindowsFileSystem.cpp: In function 'Path* fsCopyUserDocumentsDirectoryPath()':
/work/Common_3/OS/Windows/WindowsFileSystem.cpp:336:66: error: 'SHGetKnownFolderPath' was not declared in this scope
  SHGetKnownFolderPath(FOLDERID_Documents, 0, NULL, &userDocuments);
                                                                  ^
CMakeFiles/TheForge-static.dir/build.make:679: recipe for target 'CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsFileSystem.cpp.obj' failed
make[2]: *** [CMakeFiles/TheForge-static.dir/Common_3/OS/Windows/WindowsFileSystem.cpp.obj] Error 1
CMakeFiles/Makefile2:96: recipe for target 'CMakeFiles/TheForge-static.dir/all' failed
make[1]: *** [CMakeFiles/TheForge-static.dir/all] Error 2
Makefile:151: recipe for target 'all' failed
make: *** [all] Error 2

Do you have some ideas for how to fix that?

hydexon commented 4 years ago

Hey @Hideman85 i've seen your last compiler error issue:

jwatte commented 4 years ago

In general, trying to support IDEs is an uphill battle. It's better to use a single tool, that can generate project files for all the IDEs. cmake and premake are tools in that category -- once you have cmake files, you can generate XCode, visual studio, makefile, or other supported output configurations, all from the same cmakefile.txt.

In fact, I don't think the existing codelite project is up to date. It references a file that doesn't exist (EphemerisExample/Example.cpp) and it doesn't seem to have runnable configurations for the unit tests. (Or maybe it does? I know next to nothing about codelite itself.)

Physical project structure that works (flexibly) for many use cases is something that's important for re-usable libraries, and many of the suggestions in this thread are good. Use include file paths, not relative includes. Use a single build tool and generate whatever project files you need from that. Reference third party libraries as git submodules. If you need to patch them, use a patch file that you can apply as part of a pre-build step. This is a bit of work to get up to date when you first go into the effort, but it really is worth it once you're on a working system -- having to just change things in one place, and all the platforms update, is very powerful. (And helps with the "make everything work with integration testing" thing, too.)

I know nothing about The Forge (at least yet,) but I'm happy to help in some small way if you're willing to embark on this conversion.

OvermindDL1 commented 4 years ago

And there are a multitude of IDE's now that use cmake 'as' their build system, KDevelop and CLion are two such things (I think even visual studio can use a cmake project as a project now?)

jwatte commented 4 years ago

Visual Studio may or may not be able to use cmake, but cmake can generate Visual Studio projects.

OvermindDL1 commented 4 years ago

MS added native support in VS 2017 15.4+:

https://devblogs.microsoft.com/cppblog/cmake-support-in-visual-studio/ https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

Though I've not used them so I cannot attest to the quality as I don't use Windows.

wolfgangfengel commented 4 years ago

@jwatte one of our challenges on PC (we have to limit this discussion to PC because it is public) is that we really only want to support one version of Visual Studio 2017. We need to limit support to a narrow range of Visual Studio 2017 versions so that we can be sure everyone can actually run the code base. If CMake allows to use Visual Studio 2015 or 2019 oir even an older version of Visual Studio 2017 our code base might not compile or run. We are testing that narrow range of Visual Studio 2017 but we don't test older Visual Studio 2017 versions or 2015, 2019 or any older IDE. I know that our code doesn't compile with an older version of Visual Studio 2017.

If we would allow an end-user on PC to generate a Visual Studio 2015 version we would have to test it and make sure the code actually compiles on 2015. This would increase the time of automatic testing exponentially and people would have to wait every time they want to commit code. Apart from all the other challenges that CMake would add to a practical team development environment that challenge just by itself is not acceptable. Obviously our most important development platforms are consoles ... not even sure what kind of problems we would face there.

wolfgangfengel commented 4 years ago

iOS is another important development platform for us. I don't know how CMake works there but the same train of thought applies. In case CMake would allow a user to use a different IDE compared to the one we use, we would have to test this IDE. Unfortunately we are pretty cutting edge there and we have a lot of challenges with beta IDEs that work or do not work ... we will settle on an older version over time to stabilize that.

Hideman85 commented 4 years ago

The things is you don't need to support all IDE. For example as you said for Visual Studio you can put in readme support 2017+ version.

The really good aspect of CMake is cross-compilation, you can compile for all different platforms on a single OS or by using docker container if needed.

Usually when you use CMake as build system you generate project files in the IDE you want to enable you working on the code but you use make as build system (by yourself or thanks to your IDE).

wolfgangfengel commented 4 years ago

Cross-compilation on one platform: whatever we deliver should compile in that scenario. With a wide range of variables (different compilers, different compiler versions etc.) I can't think of a good way to test this? Honestly that would sound like a support nightmare.

Hideman85 commented 4 years ago

Supporting IDEs is even worse... IDEs is good for developing but not really for building for all platforms, for that we need some sort of cross compiling and even for that you don't have to support all compilers you can say in the readme that you test with GCC6.x, clang6.x, mingw6.x and suppose that a new version can work (and if you use mainly cpp standards, compilers version don't really impact the compilation success).

The question is how you currently compile your code to all your supported platforms? Maybe also to don't have to support several compilers you can make some docker containers for compiling for each platforms like we can found there: https://github.com/dockcross/dockcross

juteman commented 4 years ago

@Hideman85 I think that a building system like CMake is good for this project. The first time I look this project sample, think it was really cool.But where is the building system? After read about wiki I find it was organized by sln, and even can't build on vs2019. It is so strange that a mordern cross platform project use sln on windows,and other building system on other platform. I watch about your work on CMake.I think that more work should do , like the Thirdparty library.Most of them can find on github, and some of them have use the cmake.Why we not clone directly, so the others who want to use this framework can just use the command like

git clone --recurse-submodules

to get all the code. And I think the folder should be reorganized too, make it looks like standerd opensource project. The work that should do is not easy, but i think it will be new project after that.

Hideman85 commented 4 years ago

@juteman Thanks for your feedback. Yes for me this really important to setup a build system and CMake is really good for that kind of project and particularly for a cross platform project.

Yes git submodules is a great way to organize the project unfortunately like I said previously they did some changes on the libraries that they use and so for that they need to change there way to do it (Adapter pattern or repo fork).

The project architecture is also weird but they have other related repo I think for console platforms so they also need to update there architecture but I think this is not there priority right now that I can understand.

juteman commented 4 years ago

@Hideman85 I use the tool "diff" compare some ThirdParty code. It seems that they just change structure of the ThirdParty lib,but not change the api they use. So I think it is possible to use git submodules to organize the project.

I am plan to do some work as I mention.but I do not guarantee to complete the task because I do not have much free time.

Hideman85 commented 4 years ago

Same here I started to do some stuff but now I'm completely busy on an other project.

silentorb commented 3 years ago

This looks like the sort of 3D rendering framework the game industry needs and could rally behind. However, it's current approach to building and integration will add friction to any adoption.

CMake is an excellent tool. It decouples the functionality of a library from how it is used.

It's great to see a high value placed on unit testing, but if the Forge is ever going to reach critical mass, it's going to need to be able to say both "Here's the formally supported and tested use cases" and "Here, we've made it easy for you to do whatever you want and shoot yourself in the foot".