[core] Splitting rcore by platform into submodules

[michaelfiber]

Issue description

As part of my experimenting in pr #3311 to divide rcore into submodules, one of the things I found was that the CoreData structure adds a lot of complexity to that endeavor and MAY benefit from being split up as well. I believe it may benefit because the current CoreData structure can be fairly difficult to understand with the large number of preprocessor directives within the struct itself. This is largely done to accommodate the many different target platforms. But if rcore is divided into different submodules based on platforms there is an opportunity to increase clarity.

Here are a couple of initial ideas for approaches:

1. Maintain a single instance of CoreData but have CoreData only contain data that all submodules use so that it is a simpler struct with no preprocessor directives. A single CORE instance would be defined extern in rcore.h, defined in rcore.c, and accessed from rcore.c and all the submodules. In addition to this each submodule would have a small CoreData like struct of its own, i.e. DesktopData, WebData, etc. The data that is currently in CoreData that is specific to an submodule would be moved to the data struct in that submodule. Each struct definition would become much more legible because it wouldn't be full of preprocessor directives but there would be more structs overall and they'd be more spread out instead of centralized in rcore.

2. Another option could be to have the CoreData struct defined entirely within a submodule specific header file so that each submodule has a complete CoreData struct and rcore.c can include a specific one based on the PLATFORM variable. There'd be more duplication but fewer structs and data would not be divided across multiple structs. Any changes to the data in CoreData that is used by all submodules would require applying the change to each submodule.

Personally I'm leaning towards the first option but am interested in what others think.

Environment

Experiments are being coded in Ubuntu 23.04 and leverage github actions to attempt to run build steps for all platforms.

Code Example

#3311

[ghost]

Under approach 1, what would happen to a member that is used, lets say, by PLATFORM_DESKTOP and PLATFORM_WEB but not PLATFORM_ANDROID nor PLATFORM_DRM.

Would it be under rcore?

Or not in rcore but duplicated on rcore_desktop and on rcore_web?

[raysan5]

I think both approaches have pros and cons.

• 1 keeps a common unified structure for all platforms with just "some" static additional variables per module (duplicated if required, @ubkp) but makes it more difficult to create multi-windows applications in case it was needed, because of those same custom variables, some pieces of data are spread around.
• 2 allows more customization per platform and makes the modules more self-contained per platform, but also increases the complexity dealing with multiple duplicated structs, if a change is needed, it should be maybe propagated to all platforms, for example, recently some new variables were added.

I think the best option could be 1, despite the cons. concerns. I think the number of custom variables is not that big and it should be easier to manage if well organized and documented in the code.

In an ideal scenario, the "extra" required variables could be, maybe, removed or integrated in some way into the main CoreData struct... Actually, that's another possibility, just keep everything in CoreData independently of the platform, beeing used or not. Probably the overload won't be that dramatic...

[raysan5]

EDIT: Copying the info from wishlist for reference:

• REDESIGN: Module subdivision. This module has grown a lot with every new supported platform (~7000 loc), at some point it could be divided into separate modules. Two possible approaches:
  • Divide by platform:
    • rcore_desktop.c - PLATFORM_DESKTOP (Windows, Linux, macOS)
    • rcore_android.c - PLATFORM_ANDROID
    • rcore_native.c - PLATFORM_DRM/PLATFORM_RPI
    • rcore_web.c - PLATFORM_WEB
  • Divide by technology
    • rcore_glfw.c - GLFW library based
    • rcore_sdl.c - SDL library based
    • rcore_native.c - Native implementation

This is not an easy task, still deciding the proper approach for it. There are 45 functions depending on ~100 GLFW functions, those 45 functions should be reproduced separately for the different platforms/technologies and exposed through the same common API.

Also related to this submodules project, some time ago I created a list of functions dependant on GLFW (PLATFORM_DESKTOP), that's probably the list of functions that should be duplicated between platform:

void CloseWindow(void)
    glfwDestroyWindow();
    glfwTerminate();
bool WindowShouldClose(void)
    glfwWaitEvents();
    glfwWindowShouldClose();
void ToggleFullscreen(void)
    glfwGetWindowPos()
    glfwGetMonitors();
    glfwSetWindowMonitor()
    glfwSwapInterval();
void MaximizeWindow(void)
    glfwGetWindowAttrib()
    glfwMaximizeWindow()
void MinimizeWindow(void)
    glfwIconifyWindow()
void RestoreWindow(void)
    glfwRestoreWindow()
void SetWindowState(unsigned int flags)
    glfwSwapInterval();
    glfwSetWindowAttrib()
    glfwHideWindow()
    glfwShowWindow()
void SetWindowIcon(Image image)
    glfwSetWindowIcon()
void SetWindowIcons(Image *images, int count)
    glfwSetWindowIcon()
void SetWindowTitle(const char *title)
    glfwSetWindowTitle()
void SetWindowPosition(int x, int y)
    glfwSetWindowPos()
void SetWindowMonitor(int monitor)
    glfwGetMonitors()
    glfwGetMonitorName()
    glfwGetVideoMode()
    glfwSetWindowMonitor()
void SetWindowMinSize(int width, int height)
    glfwGetVideoMode()
    glfwSetWindowSizeLimits()
void SetWindowSize(int width, int height)
    glfwSetWindowSize()
void SetWindowOpacity(float opacity)
    glfwSetWindowOpacity()
void *GetWindowHandle(void)
    glfwGetWin32Window()
    glfwGetX11Window()
    glfwGetCocoaWindow()
int GetMonitorCount(void)
    glfwGetMonitors();
int GetCurrentMonitor(void)
    glfwGetMonitors();
    glfwGetWindowMonitor()
    glfwGetWindowPos()
    glfwGetMonitorWorkarea()
Vector2 GetMonitorPosition(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPos()
int GetMonitorWidth(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
int GetMonitorHeight(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
int GetMonitorPhysicalWidth(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPhysicalSize()
int GetMonitorPhysicalHeight(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPhysicalSize()
int GetMonitorRefreshRate(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
Vector2 GetWindowPosition(void)
    glfwGetWindowPos()
Vector2 GetWindowScaleDPI(void)
    glfwGetMonitors()
    glfwGetMonitorContentScale()
    glfwGetMonitorWorkarea()
const char *GetMonitorName(int monitor)
    glfwGetMonitors()
    glfwGetMonitorName()
void SetClipboardText(const char *text)
    glfwSetClipboardString()
const char *GetClipboardText(void)
    glfwGetClipboardString()
void ShowCursor(void)
    glfwSetInputMode()
void HideCursor(void)
    glfwSetInputMode()
void EnableCursor(void)
    glfwSetInputMode()
void DisableCursor(void)
    glfwSetInputMode()
double GetTime(void)
    glfwGetTime()
const char *GetGamepadName(int gamepad)
    glfwGetJoystickName();
void SetMousePosition(int x, int y)
    glfwSetCursorPos()
void SetMouseCursor(int cursor)
    glfwSetCursor()
    glfwCreateStandardCursor()
static bool InitGraphicsDevice(int width, int height)
    glfwSetErrorCallback();
    glfwInitHint()
    glfwInit()
    glfwDefaultWindowHints();
    glfwWindowHint()
    glfwSetJoystickCallback();
    glfwGetPrimaryMonitor();
    glfwGetVideoMode(monitor);
    glfwGetVideoModes()
    glfwCreateWindow()
    glfwSetWindowMonitor()
    glfwTerminate();
    glfwSetWindowSizeCallback()
    glfwSetWindowMaximizeCallback()
    glfwSetWindowIconifyCallback()
    glfwSetWindowFocusCallback()
    glfwSetDropCallback()
    glfwSetKeyCallback()
    glfwSetCharCallback()
    glfwSetMouseButtonCallback()
    glfwSetCursorPosCallback()
    glfwSetScrollCallback()
    glfwSetCursorEnterCallback()
    glfwMakeContextCurrent();
    glfwSetInputMode()
    glfwSwapInterval()
    glfwGetFramebufferSize()
    glfwGetProcAddress()
static void SetupViewport(int width, int height)
    glfwGetWindowContentScale()
void SwapScreenBuffer(void)
    glfwSwapBuffers();
void PollInputEvents(void)
    glfwJoystickPresent()
    glfwGetGamepadState()
    glfwWaitEvents()
    glfwPollEvents()
static void KeyCallback(GLFWwindow *window, int key, int scancode, int action, int mods)
    glfwSetWindowShouldClose()

Here the functions list clean:

void CloseWindow(void)
bool WindowShouldClose(void)
void ToggleFullscreen(void)
void MaximizeWindow(void
void MinimizeWindow(void)
void RestoreWindow(void)
void SetWindowState(unsigned int flags)
void SetWindowIcon(Image image)
void SetWindowIcons(Image *images, int count)
void SetWindowTitle(const char *title)
void SetWindowPosition(int x, int y)
void SetWindowMonitor(int monitor)
void SetWindowMinSize(int width, int height)
void SetWindowSize(int width, int height)
void SetWindowOpacity(float opacity)
void *GetWindowHandle(void)
int GetMonitorCount(void)
int GetCurrentMonitor(void)
Vector2 GetMonitorPosition(int monitor)
int GetMonitorWidth(int monitor)
int GetMonitorHeight(int monitor)
int GetMonitorPhysicalWidth(int monitor)
int GetMonitorPhysicalHeight(int monitor)
int GetMonitorRefreshRate(int monitor)
Vector2 GetWindowPosition(void)
Vector2 GetWindowScaleDPI(void)
const char *GetMonitorName(int monitor)
void SetClipboardText(const char *text)
const char *GetClipboardText(void)
void ShowCursor(void)
void HideCursor(void)
void EnableCursor(void)
void DisableCursor(void)
double GetTime(void)
const char *GetGamepadName(int gamepad)
void SetMousePosition(int x, int y)
void SetMouseCursor(int cursor)
static bool InitGraphicsDevice(int width, int height)
static void SetupViewport(int width, int height)
void SwapScreenBuffer(void)
void PollInputEvents(void)

Most of those functions could be not required on target platform and just fallback to some default value.

[ghost]

Actually, that's another possibility, just keep everything in CoreData independently of the platform, beeing used or not. Probably the overload won't be that dramatic...

This would be excellent. Would make a lot simpler for anyone that want/need to extend it. I'd vote for this one.

[michaelfiber]

Actually, that's another possibility, just keep everything in CoreData independently of the platform, beeing used or not. Probably the overload won't be that dramatic...

I like that. It would be super clear. And a shared data structure would probably guide platform specific development to re-use rather than reinvent since every platform would have access to all the data structure anyway.

I hadn't considered multiple windows when I was thinking through it so the approach I was leaning towards before would probably need a lot of work to handle that sanely. And it would lose a lot of clarity.

[raysan5]

@michaelfiber @ubkp I've been thinking about the best approach for code organization and I got to the conclusion that avoiding breaking all raylib current build systems (and potentially many bindings or user codebases) is the best option, my idea is to just keep the rcore.c as the base module, containing all common functions BUT also the inclusion of the required submodules:

// rcore.c

// Required data types and variables

#if defined(PLATFORM_DESKTOP)
    #include "rcore_desktop.c"
#elif defined(PLATFORM_WEB)
    #include "rcore_web.c"
#elif defined(PLATFOM_DRM)
    #include "rcore_drm.c"
#elif defined(PLATFOM_ANDROID)
    #include "rcore_android.c"
#else
    // Software rendering backend, user needs to provide buffer ;)
#endif

// Common functions to all platforms

From an academic point of view it could be ugly but in practice it works very well.

[raysan5]

@michaelfiber @ubkp I'd also like to accelerate the development of this improvement, considering the latest events on the gamedev industry I think it could be beneficial for raylib in multiple ways, for example to allow other contributors to hook additional platforms in an easy way; at this moment I know is quite scary to look at rcore.

Please, let me know if you could help me with this big redesign!

[ghost]

my idea is to just keep the rcore.c as the base module, containing all common functions BUT also the inclusion of the required submodules

1. In that scenario, for example, IsWindowMinimized()(L1154-L1160), would it be under rcore.c with its #ifdefs? Or not in rcore.c but duplicated on rcore_desktop.c and rcore_web.c?
   
   I ask this because, if it's under rcore.c, then I don't think much would change. rcore.c would still have a considerable amount of #ifdefs, thus harder to track. I guess most functions would fall under that case.

#if defined(PLATFORM_DESKTOP)
#include "rcore_desktop.c"
#elif

2. Please correct me if I'm wrong (and I'm probably wrong), but I was under the impression that this way of including would be a bad idea (refs: 1, 2, 3).

for example to allow other contributors to hook additional platforms in an easy way;

3. I'd say the only (actual) missing platform would be iOS. IMHO, I don't think we'd be seeing aditional (e.g.: consoles) platforms even if the conditions were right because of the SDKs and NDAs. Just as a reference, quoting the Godot documentation on a similar subject:

The reason other consoles are not officially supported are:

• To develop for consoles, one must be licensed as a company. As an open source project, Godot has no legal structure to provide console ports.

• Console SDKs are secret and covered by non-disclosure agreements. Even if we could get access to them, we could not publish the platform-specific code under an open source license.

4. I admit I'm having a hard time visualizing it all (separation, building, etc). Would it be a good idea to test it first in a sort of very minimal skeleton/prototype with just one or two structs/functions inside just to see what works or not?

Edit:

I'd also like to accelerate the development of this improvement

5. When the new structure is set in stone, to speed up its implementation, I'd suggest setting a temporary freeze in new PRs (like Linux distros do) to avoid having to play catch-up given the structural nature of the change.

[michaelfiber]

@michaelfiber @ubkp I've been thinking about the best approach for code organization and I got to the conclusion that avoiding breaking all raylib current build systems (and potentially many bindings or user codebases) is the best option, my idea is to just keep the rcore.c as the base module, containing all common functions BUT also the inclusion of the required submodules:

// rcore.c

// Required data types and variables

#if defined(PLATFORM_DESKTOP)
    #include "rcore_desktop.c"
#elif defined(PLATFORM_WEB)
    #include "rcore_web.c"
#elif defined(PLATFOM_DRM)
    #include "rcore_drm.c"
#elif defined(PLATFOM_ANDROID)
    #include "rcore_android.c"
#else
    // Software rendering backend, user needs to provide buffer ;)
#endif

// Common functions to all platforms

From an academic point of view it could be ugly but in practice it works very well.

Doing it this way feels wrong but I completely understand not wanting to break builds. Updating the Makefile was easy but I don't know about updating the others. Right now for testing I'm going to leave the Makefile change and then as I progress through this maybe I'll spot another way that doesn't feel so wrong? With the position of raylib as a great tool for education I'd hate to include something that would have a comment like // You shouldn't do this

Also for the CoreData I realized that if all the data is always there without preprocessor directives then it either needs all dependencies included no matter that target platform OR a lot of void pointers that will have to be cast each time in the platform specific rcore files.

Right now I am leaving the #if defined(PLATFORM_*) directives in the struct as I split out the rest of the functions. The CoreData structure doesn't get any more difficult to read than it did before but it maintains its very difficult to read directives. Another less disruptive approach may be a simple refactor to move platform specific stuff so there's just one directive per platform in CoreData. CORE->handle would become CORE->desktop->handle for instance. It does mean some repeated data structures between platforms but I think the readability would increase dramatically.

TL;DR:

• For now I'm leaving the preprocessor directives in the CoreData struct and will test out a refactor, possibly in a separate PR
• For now I'm leaving the Makefile change to include platform specific core c files as I look for other paths besides #include "rcore_*.c" because I would like to not break the build files so thoroughly and I would also like to avoid including a C file if at all possible.

[raysan5]

@ubkp here the answers for your questions:

1. In those cases the function would be duplicated for all platforms requiring it.

2. It's not recommended and probably bad practice from an academic point of view, still, it can be used and it's really handy. Actually raylib uses that approach for all its external libraries, #define XXXX_IMPLEMENTATION does exactly that.

3. Yes, most noticeable missing platform is PLATFORM_IOS, maybe PLATFORM_SOFTWARE could also be an option... BUT this structure opens the door to the addition of console platforms (by licensed users) in an easier way with minimal raylib modifications. Console homebrew platforms could also follow this same pattern.

4. Yes, it's a very big change in structure. I don't fully visualize it at the moment but I think it could work.

5. Completely agree. Still considering if moving the platform-specific parts from CoreData structure is a better approach, actually CoreData was created quite recently for organization (as a struct of structs), not long ago it was just a bunch of global variables.

Doing it this way feels wrong but I completely understand not wanting to break builds.

@michaelfiber I know, it feels wrong... but it actually works like a charm for this kind of situations.

[michaelfiber]

@raysan5 Ok, I can try that out.

[michaelfiber]

EDIT: Copying the info from wishlist for reference:

* [ ]  **REDESIGN: Module  subdivision**. This module has grown a lot with every new supported platform (~7000 loc), at some point it could be divided into separate modules. Two possible approaches:
  
  * Divide by platform:
    
    * `rcore_desktop.c`  - `PLATFORM_DESKTOP` (Windows, Linux, macOS)
    * `rcore_android.c` - `PLATFORM_ANDROID`
    * `rcore_native.c` - `PLATFORM_DRM`/`PLATFORM_RPI`
    * `rcore_web.c` - `PLATFORM_WEB`
  * Divide by technology
    
    * `rcore_glfw.c` - GLFW library based
    * `rcore_sdl.c` -  SDL library based
    * `rcore_native.c` - Native implementation

This is not an easy task, still deciding the proper approach for it. There are 45 functions depending on ~100 GLFW functions, those 45 functions should be reproduced separately for the different platforms/technologies and exposed through the same common API.

Also related to this submodules project, some time ago I created a list of functions dependant on GLFW (PLATFORM_DESKTOP), that's probably the list of functions that should be duplicated between platform:

void CloseWindow(void)
    glfwDestroyWindow();
    glfwTerminate();
bool WindowShouldClose(void)
    glfwWaitEvents();
    glfwWindowShouldClose();
void ToggleFullscreen(void)
    glfwGetWindowPos()
    glfwGetMonitors();
    glfwSetWindowMonitor()
    glfwSwapInterval();
void MaximizeWindow(void)
    glfwGetWindowAttrib()
    glfwMaximizeWindow()
void MinimizeWindow(void)
    glfwIconifyWindow()
void RestoreWindow(void)
    glfwRestoreWindow()
void SetWindowState(unsigned int flags)
    glfwSwapInterval();
    glfwSetWindowAttrib()
    glfwHideWindow()
    glfwShowWindow()
void SetWindowIcon(Image image)
    glfwSetWindowIcon()
void SetWindowIcons(Image *images, int count)
    glfwSetWindowIcon()
void SetWindowTitle(const char *title)
    glfwSetWindowTitle()
void SetWindowPosition(int x, int y)
    glfwSetWindowPos()
void SetWindowMonitor(int monitor)
    glfwGetMonitors()
    glfwGetMonitorName()
    glfwGetVideoMode()
    glfwSetWindowMonitor()
void SetWindowMinSize(int width, int height)
    glfwGetVideoMode()
    glfwSetWindowSizeLimits()
void SetWindowSize(int width, int height)
    glfwSetWindowSize()
void SetWindowOpacity(float opacity)
    glfwSetWindowOpacity()
void *GetWindowHandle(void)
    glfwGetWin32Window()
    glfwGetX11Window()
    glfwGetCocoaWindow()
int GetMonitorCount(void)
    glfwGetMonitors();
int GetCurrentMonitor(void)
    glfwGetMonitors();
    glfwGetWindowMonitor()
    glfwGetWindowPos()
    glfwGetMonitorWorkarea()
Vector2 GetMonitorPosition(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPos()
int GetMonitorWidth(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
int GetMonitorHeight(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
int GetMonitorPhysicalWidth(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPhysicalSize()
int GetMonitorPhysicalHeight(int monitor)
    glfwGetMonitors()
    glfwGetMonitorPhysicalSize()
int GetMonitorRefreshRate(int monitor)
    glfwGetMonitors()
    glfwGetVideoMode()
Vector2 GetWindowPosition(void)
    glfwGetWindowPos()
Vector2 GetWindowScaleDPI(void)
    glfwGetMonitors()
    glfwGetMonitorContentScale()
    glfwGetMonitorWorkarea()
const char *GetMonitorName(int monitor)
    glfwGetMonitors()
    glfwGetMonitorName()
void SetClipboardText(const char *text)
    glfwSetClipboardString()
const char *GetClipboardText(void)
    glfwGetClipboardString()
void ShowCursor(void)
    glfwSetInputMode()
void HideCursor(void)
    glfwSetInputMode()
void EnableCursor(void)
    glfwSetInputMode()
void DisableCursor(void)
    glfwSetInputMode()
double GetTime(void)
    glfwGetTime()
const char *GetGamepadName(int gamepad)
    glfwGetJoystickName();
void SetMousePosition(int x, int y)
    glfwSetCursorPos()
void SetMouseCursor(int cursor)
    glfwSetCursor()
    glfwCreateStandardCursor()
static bool InitGraphicsDevice(int width, int height)
    glfwSetErrorCallback();
    glfwInitHint()
    glfwInit()
    glfwDefaultWindowHints();
    glfwWindowHint()
    glfwSetJoystickCallback();
    glfwGetPrimaryMonitor();
    glfwGetVideoMode(monitor);
    glfwGetVideoModes()
    glfwCreateWindow()
    glfwSetWindowMonitor()
    glfwTerminate();
    glfwSetWindowSizeCallback()
    glfwSetWindowMaximizeCallback()
    glfwSetWindowIconifyCallback()
    glfwSetWindowFocusCallback()
    glfwSetDropCallback()
    glfwSetKeyCallback()
    glfwSetCharCallback()
    glfwSetMouseButtonCallback()
    glfwSetCursorPosCallback()
    glfwSetScrollCallback()
    glfwSetCursorEnterCallback()
    glfwMakeContextCurrent();
    glfwSetInputMode()
    glfwSwapInterval()
    glfwGetFramebufferSize()
    glfwGetProcAddress()
static void SetupViewport(int width, int height)
    glfwGetWindowContentScale()
void SwapScreenBuffer(void)
    glfwSwapBuffers();
void PollInputEvents(void)
    glfwJoystickPresent()
    glfwGetGamepadState()
    glfwWaitEvents()
    glfwPollEvents()
static void KeyCallback(GLFWwindow *window, int key, int scancode, int action, int mods)
    glfwSetWindowShouldClose()

Here the functions list clean:

void CloseWindow(void)
bool WindowShouldClose(void)
void ToggleFullscreen(void)
void MaximizeWindow(void
void MinimizeWindow(void)
void RestoreWindow(void)
void SetWindowState(unsigned int flags)
void SetWindowIcon(Image image)
void SetWindowIcons(Image *images, int count)
void SetWindowTitle(const char *title)
void SetWindowPosition(int x, int y)
void SetWindowMonitor(int monitor)
void SetWindowMinSize(int width, int height)
void SetWindowSize(int width, int height)
void SetWindowOpacity(float opacity)
void *GetWindowHandle(void)
int GetMonitorCount(void)
int GetCurrentMonitor(void)
Vector2 GetMonitorPosition(int monitor)
int GetMonitorWidth(int monitor)
int GetMonitorHeight(int monitor)
int GetMonitorPhysicalWidth(int monitor)
int GetMonitorPhysicalHeight(int monitor)
int GetMonitorRefreshRate(int monitor)
Vector2 GetWindowPosition(void)
Vector2 GetWindowScaleDPI(void)
const char *GetMonitorName(int monitor)
void SetClipboardText(const char *text)
const char *GetClipboardText(void)
void ShowCursor(void)
void HideCursor(void)
void EnableCursor(void)
void DisableCursor(void)
double GetTime(void)
const char *GetGamepadName(int gamepad)
void SetMousePosition(int x, int y)
void SetMouseCursor(int cursor)
static bool InitGraphicsDevice(int width, int height)
static void SetupViewport(int width, int height)
void SwapScreenBuffer(void)
void PollInputEvents(void)

Most of those functions could be not required on target platform and just fallback to some default value.

I'll probably just go function by function through rcore and move the functions that have platform_specific logic. That will probably align with the list of functions that reference glfw but I'm not sure. Also If you are keeping a list then SetWindowFocused should be on it too.

[michaelfiber]

Immediately as I make this transition I see the opportunity to add some simple javascript functions to the default HTML templates for emscripten that would help with things like reading the clipboard and disabling the cursor without having the ID of the canvas element hard coded into raylib. That makes me think this submodule approach might be a good one. It's much easier to check at a glance how complete each implementation is.

[ghost]

@michaelfiber If you need help with anything, please let me know.

[raysan5]

@michaelfiber thank you very much for this big effort! Undoubtely it opens the door to many per-platform improvements.

This is a lot of work, please, let me and @ubkp help with it.

I created a separate branch to start merging the changes: https://github.com/raysan5/raylib/tree/rcore_platform_split

We can divide the functions to port in chunks between us.

Also, I'm locking any change in rcore module for now, at least until we get a functional version and we can validate if it's a good line to follow for raylib (I'm pretty convinced of it!).

[michaelfiber]

I'm doing a basic split on the platform specific preprocessor directives so it's actually very easy. But once its done the real work of making sure each submodule is correct and builds will be more time consuming I think and that'll be very easy to divvy up.

I should be done splitting rcore today and maybe after that we can share going through the submodules?

[raysan5]

I should be done splitting rcore today and maybe after that we can share going through the submodules?

Perfect, it sounds like a plan. Feel free to send the redesign to rcore_platform_split branch and we can continue from there.

I'm not touching rcore module for now.

[michaelfiber]

@raysan5 I changed PR #3311 to target rcore_platform_split and finished moving the listed functions. There were a couple of functions not on the list that I moved because their logic was highly platform dependent. I have a feeling more such functions are there in rcore but I just haven't had the chance to go through every function yet. When a function like that is found it needs to be moved from rcore to all submodules at the same time. So that might create logistical issues as we work on this.

I did all my work on Linux and it shows as the linux builds are the only ones working at this time. The windows build has been succeeding every time I push even though it was definitely actually failing so that needs more than just the github action to verify if it is working at all.

I think this is a good place to start divvying up the work since different people can work on different submodules simuiltaneously and the only real friction will be when changes need to be made to rcore.c or rcore.h.

I'm going to be away from my computer for a few days now but when I get back I can continue to hack away at some of this. I would probably start working specifically on PLATFORM_DRM since I have some experience with it and a variety of devices already running raylib projects to test with.

[raysan5]

@michaelfiber Thank you very much for the big amount work put into this improvement!

I'm merging it right now and reviewing Windows build in the following days! Thanks!

[ghost]

@raysan5 @michaelfiber I can review PLATFORM_WEB (it's the one I got more experience with).

[orcmid]

@raysan5 @michaelfiber I have cloned the branch and my Windows x64 build using VC/C++ fails with a fatal error in compiling rcore.c.

 rcore_desktop.c(3) fatal error C1083: Cannot open inlude file: 'rcore.h': No such file or directory

Seems to be the case.

2023-09-21T00:02Z @ubkp @raysan5 @michaelfiber That problem is now fixed and a simple conformation test of raylib component compiles works fine.

[ghost]

@orcmid The previous version had rcore.h (f27454c) but @raysan5 moved its content to rcore.c (71a1217). Likely still cleaning up after the change and missed it on rcore_desktop.c (L3).

[raysan5]

@orcmid @ubkp Sorry, I broke it just trying some things, reviewed it now, it should work but still some issues...

I'm afraid this redesign could be more complex than I anticipated... let's see how evolves...

[ghost]

@raysan5 Would it be a bad idea to move rcore.h's contents to raylib.h?

[raysan5]

@ubkp I prefer to avoid that, it will add too much uneeded content in raylib.h and probably a lot of confusion that use raylib.h as their reference cheatsheet. rcore.h is ok.

[michaelfiber]

Yeah I think its more likely rcore.h will go away than be integrated with raylib.h. With the #includes being there there isn't a ton of need for it except that I think the IDE experience might break down a bit without it. I could be wrong though.

[michaelfiber]

@raysan5 I moved raymath.h out of rcore.h because it was clobbering the #define RAYMATH_IMPLEMENTATION directive. Submitted as pr #3331

That fixed my build issues across platform related to not being able to find raymath functions.

EDIT: I closed that down because I was still committing to the branch. I am trying to rush things a bit too much.

I also moved the block of code that includes the submodules to the end of rcore.c because I realized I put it ahead of function definitions originally and that was creating issues compiling examples.

@ubkp did you encounter this as well in your work?

[raysan5]

Please note that I pushed a commit to rcore master branch (a2b3b1e). I'm afraid it was an issue that could cause a stack overflow in several platforms when using CompressData() function, it was actually breaking some of my tools in PLATFORM_WEB because emscripten sets a maximum size for stack of 65KB.

[ghost]

@ubkp did you encounter this as well in your work?

@michaelfiber Not yet. Testing PLATFORM_WEB is going really smoothly here. After fixing the initial compiling errors I jumped to test the examples and (aside from a few that are actually not working even on current master branch) just found two examples that needed review.

I'm still going through all functions but, so far, everything is working really well. You did an incredible job on the split. 👍

[ghost]

@Bigfoot71 Awesome! Thank you very much! My apologies again for deleting the message. I was trying to do two things at once and messed up.

[Bigfoot71]

Or we think that the problem comes from our way of obtaining the mouse delta. We could make sure that event.motion.xrel and event.motion.yrel are returned only when the mouse is moving.

Edit: This way the delta would always be correct and the mouse would not be stuck in the center of the screen when hidden.
Because currently we calculate it ourselves with the position that we obtain ourselves, that's what poses the problem.

Edit 2: But it would be necessary to create a new variable to store the delta.

When trying SDL_WarpMouseInWindow as well, I notice the same behavior, which is logical since the mouse is repositioned each time. I think it would be really important to be able to store relx and rely to be able to return them in GetMouseDelta(), which is rather cumbersome.

Alternatively, we could see how SDL_SetHint(SDL_HINT_MOUSE_RELATIVE_WARP_MOTION, "1"); behaves, but there is a strong chance that it will yield the same result.

[ghost]

Edit 2: I spoke too quickly, it doesn't work and it's quite logical in the end :/

@Bigfoot71 Yeah, you're right. Also good idea using the core_3d_camera_first_person.

Or we think that the problem comes from our way of obtaining the mouse delta. We could make sure that event.motion.xrel and event.motion.yrel are returned only when the mouse is moving.

Edit: This way the delta would always be correct and the mouse would not be stuck in the center of the screen when hidden. Because currently we calculate it ourselves with the position that we obtain ourselves, that's what poses the problem.

This sounds like a good solution.

By the way, I was messing around testing zeroing the CORE.Input.Mouse.previousPosition (L615), which improved it, but caused drifting.

Edit: @Bigfoot71 Yeah, I'm trying to figure out why SDL_SetHint(SDL_HINT_MOUSE_RELATIVE_WARP_MOTION, "1"); is not working here. It should be working on 2.28.4.

[ghost]

@Bigfoot71 Managed to fix it. I'll clean this up and send you a diff. The solution was zeroing indeed.

[ghost]

@Bigfoot71 Could you please test this:

diff --git a/rcore_desktop_sdl.c b/rcore_desktop_sdl_FIXED.c
index cddee74..8657094 100644
--- a/rcore_desktop_sdl.c
+++ b/rcore_desktop_sdl_FIXED.c
@@ -63,6 +63,7 @@ typedef struct {
 
     SDL_Joystick *gamepad;
     SDL_Cursor *cursor;
+    bool cursorRelative;
 } PlatformData;
 
 //----------------------------------------------------------------------------------
@@ -529,7 +530,7 @@ void EnableCursor(void)
 {
     SDL_SetRelativeMouseMode(SDL_FALSE);
     SDL_ShowCursor(SDL_ENABLE);
-
+    platform.cursorRelative = false;
     CORE.Input.Mouse.cursorHidden = false;
 }
 
@@ -537,7 +538,7 @@ void EnableCursor(void)
 void DisableCursor(void)
 {
     SDL_SetRelativeMouseMode(SDL_TRUE);
-
+    platform.cursorRelative = true;
     CORE.Input.Mouse.cursorHidden = true;
 }
 
@@ -612,7 +613,8 @@ void PollInputEvents(void)
     CORE.Input.Mouse.currentWheelMove.y = 0;
 
     // Register previous mouse position
-    CORE.Input.Mouse.previousPosition = CORE.Input.Mouse.currentPosition;
+    if (platform.cursorRelative) CORE.Input.Mouse.currentPosition = (Vector2){ 0.0f, 0.0f };
+    else CORE.Input.Mouse.previousPosition = CORE.Input.Mouse.currentPosition;
 
     // Reset last gamepad button/axis registered state
     CORE.Input.Gamepad.lastButtonPressed = GAMEPAD_BUTTON_UNKNOWN;
@@ -710,8 +712,17 @@ void PollInputEvents(void)
             } break;
             case SDL_MOUSEMOTION:
             {
-                CORE.Input.Mouse.currentPosition.x = (float)event.motion.x;
-                CORE.Input.Mouse.currentPosition.y = (float)event.motion.y;
+                if (platform.cursorRelative)
+                {
+                    CORE.Input.Mouse.currentPosition.x = (float)event.motion.xrel;
+                    CORE.Input.Mouse.currentPosition.y = (float)event.motion.yrel;
+                    CORE.Input.Mouse.previousPosition = (Vector2){ 0.0f, 0.0f };
+                }
+                else
+                {
+                    CORE.Input.Mouse.currentPosition.x = (float)event.motion.x;
+                    CORE.Input.Mouse.currentPosition.y = (float)event.motion.y;
+                }
             } break;
 
             // Check gamepad events

What do you think? If you agree, feel free to add it to #3439.

[Bigfoot71]

@ubkp I try this right after!

Otherwise, do you think it is necessary to check SDL_WasInit() in certain functions related to the screen and the monitor? I probably would have done it but I see that the GLFW version does not check if the window has been initialized.

[ghost]

Otherwise, do you think it is necessary to check SDL_WasInit() in certain functions related to the screen and the monitor? I probably would have done it but I see that the GLFW version does not check if the window has been initialized.

@Bigfoot71 IMHO, I don't think it's necessary right now.

[Bigfoot71]

@ubkp Your solution for the delta works! I'm adding it to the PR!
(and we avoid storing an additional Vector2, even if it is less "explicit" it is rather clever!)

The only issue left is with IsKeyPressed, which always returns true when a key is pressed, just like IsKeyDown

simplescreenrecorder-2023-10-18_20.31.03.mp4

[ghost]

The only issue left is with IsKeyPressed, which always returns true when a key is pressed, just like IsKeyDown

@Bigfoot71 Oh yeah, forgot about that. Let me take a look at it.

[ghost]

@Bigfoot71 I think I found the problem.

Looks like we are polling by event.type (L656) exclusively. But SDL_PRESSED and SDL_RELEASED are provided by event.state on the SDL_KeyboardEvent (doc).

I guess the solution could be checking if (event.key.state == SDL_PRESSED) inside case SDL_KEYDOWN: (L679). And checking if (event.key.state == SDL_RELEASED) inside case SDL_KEYUP: (L691).

What do you think?

Edit: code correction.

Edit 2, 3: tested it and event.state is not provided there. I'll take a deeper look into it. It is event.key.state. Working on a fix.

[Bigfoot71]

@ubkp, look no further, I've found it, and it was quite simple. I had realized that it wasn't working for all the keys, and I immediately guessed that it was the loop for defining the previous keys.

Currently, we are doing this:

for (int i = 0; i < 260; i++)
{
    CORE.Input.Keyboard.previousKeyState[i] = CORE.Input.Keyboard.currentKeyState[i];
    CORE.Input.Keyboard.keyRepeatInFrame[i] = 0;
}

However, with MAX_KEYBOARD_KEYS set to 512, I did this, and it seems to work perfectly:

for (int i = 0; i < MAX_KEYBOARD_KEYS; i++)
{
    CORE.Input.Keyboard.previousKeyState[i] = CORE.Input.Keyboard.currentKeyState[i];
    CORE.Input.Keyboard.keyRepeatInFrame[i] = 0;
}

[ghost]

@Bigfoot71 Great catch! Tested here and that's indeed the fix. Great job!

[Bigfoot71]

@ubkp It was the sneaky error; now we need to see what's missing. I think we've covered almost everything, haven't we? (Except for SetWindowMonitor)

[ghost]

@Bigfoot71 Yup, I think this was indeed mostly it. It was a great sprint! Really fantastic job! 👍

There are a few minor things like ToggleFullscreen setting vsync on rcore_desktop.c (L194-L196) (that I don't know if we should add too? @raysan5). And I'm sure more issues will popup as we use it more, but the basic core operation should be there.

@raysan5 After reviewing and merging #3439, if you want to move everything to a platform directory, as you mentioned, we can probably continue polishing after that.

[Bigfoot71]

@ubkp I just see that only GetWindowScaleDPI and SetWindowIcons are missing (not to be confused with SetWindowIcon).

For DPI, it doesn't seem to work the same way as with GLFW. Here is the SDL function (doc):

int SDL_GetDisplayDPI(int displayIndex, float * ddpi, float * hdpi, float * vdpi);

It returns a diagonal, horizontal, and vertical value, but I'm not sure if you need to divide the value or use it as is. (to see)

I will now see for SetWIndowIcons()

Edit: I have a feeling that SDL won't support SetWIndowIcons(), am I wrong? (with an S, plural)

[ghost]

I just see that only GetWindowScaleDPI and SetWindowIcons are missing (not to be confused with SetWindowIcon).

@Bigfoot71 That's true. I actually thought SetWindowIcons was SetWindowIcon. TBQH, now that you mentioned it, I don't even see how SetWindowIcons is supposed to be used.

It returns a diagonal, horizontal, and vertical value, but I'm not sure if you need to divide the value or use it as is. (to see)

For GetWindowScaleDPI I believe we need the hdpi and vdpi.

And as I type this I just realized I probably used the wrong dpi on GetMonitorPhysicalWidth and GetMonitorPhysicalHeight. I should have used ddpi instead of vdpi. I'll do a quick test here to confirm it.
Edit: the worst part is that when I implemented that I even took a measure tape to confirm the results and they were within tolerance.

Edit 2: Tested with all 3 (ddpi, hdpi, vdpi), their values are almost he same:

Resolution	ddpi	hdpi	vdpi
1600x900	131.486923	131.521042	131.379303
1280x1024	96.244881	96.189346	96.331848

I think the ddpi is the correct one. If anyone knows if we should be using the others instead, please send a PR correcting it.

Edit 3: Changed at #3442.

[raysan5]

@Bigfoot71 @ubkp thank you very much for all the latests reviews and improvements!

Excuse for the late response, for the last two days I've been working on Switch platform and thanks to the platform split, it worked very nicely! Unfortunately, that platform can not be open sourced...

[raysan5]

@michaelfiber @ubkp @Bigfoot71 Hi! Just moved the platforms rcore(s) to a separate directory for better organization. Please let me know if any compilation issue arises because of that.

Also marked InitPlatform() and ClosePlatform() as external (despite technically being in the same module). It shouldn't be a problem; some "special" platforms could require those functions to be external... ;)

[ghost]

Tested current master branch (081fffd) successfully:

1. PLATFORM_DESKTOP, raylib compiles ✔️, examples compile ✔️.
2. PLATFORM_WEB, raylib compiles ✔️, examples compile ✔️.
3. PLATFORM_DESKTOP_SDL, raylib compiles ✔️, examples compile ✔️.
4. PLATFORM_DRM. raylib compiles ✔️, examples compile ✔️.

I'll double check if it's not an issue on my end.
Edit 3: It was an issue on my end. I had a faulty script. Sorry, my bad.

Edit 2: PLATFORM_WEB examples are giving me this error:
PLATFORM_DRM is giving me undefined references errors. So they are probably different issues.

Environment: Linux (Ubuntu 22.04 64-bit); Web (Firefox 115.3.1esr 64-bit and Chromium 117.0.5938.149 64-bit); DRM (Raspberry Pi 3 Model B 1.2 with Raspberry Pi OS 11 Bullseye 32-bit armhf).

Edit 1, 4, 5: corrections.

[Bigfoot71]

No problems reported with the Makefile for PLATFORM_ANDROID (NDK 25.2.9519653) (081fffd)
Nor any problems reported with Android Studio. (Same as above, and everything launches perfectly)

[raysan5]

I think this issue can already by closed.

@ubkp @michaelfiber @Bigfoot71 thank you very much for making it possible! ♥️

I think is about time to prepare raylib 5.0 release. Trying to close some additional issues and squiz some extra features...

[ghost]

@raysan5 Just one last issue. While using rcore_desktop_sdl.c, after resizing the window (be it by dragging the borders, or with SetWindowSize), the window will be resized, but the rendered area won't follow and will stay on the bottom left of the window.

Screenshot

Code example

#include "raylib.h"

int main(void) {
    InitWindow(800, 450, "test");
    SetTargetFPS(60);

    SetWindowState(FLAG_WINDOW_RESIZABLE);

    while (!WindowShouldClose()) {
        BeginDrawing();
        ClearBackground(RAYWHITE);
            DrawRectangleLinesEx((Rectangle){ 0, 0, GetScreenWidth(), GetScreenHeight() }, 2.0f, RED);
            DrawText(TextFormat("Window size: %i %i", GetScreenWidth(), GetScreenHeight()), 20, 20, 20, BLACK);
        EndDrawing();
    }
    CloseWindow();
    return 0;
}

If you can, could you please take a look at this issue. I really wasn't able to figure out why. It's probably something that should be fixed before the 5.0 release.

Edit: updated code example.

[ghost]

@raysan5 I think the issue above is because we're missing something like WindowSizeCallback on rcore_desktop_sdl.c (L1553-L1584) that will SetupViewport and update the CORE.Window.currentFbo.width/height (L1556-L1559) when there's a resize event.

We probably need to poll the SDL_WINDOWEVENT_SIZE_CHANGED (doc) to update the CORE.Window.currentFbo.width/height similar to PollInputEvents. It could be added to PollInputEvents, but since it's not an input event, it could cause some confusion.

Edit: editing.

[Bigfoot71]

Maybe an event watch could help us avoid putting the resizing detection in the input-related function. (doc)

I could take a look at it tomorrow if I have some time, if you'd like.

Edit: Sorry for the link problem, phone...

[ghost]

@Bigfoot71 Very good find! SDL_AddEventWatch is probably the best/correct solution.

By the way, while inspecting the PollInputEvents with attention, I found that we already do that inside it (L993-L1004).

So, as a temporary/stopgap fix, changed it to this (which fixes the issue):

switch (event.window.event)
{
    case SDL_WINDOWEVENT_RESIZED:
    case SDL_WINDOWEVENT_SIZE_CHANGED:
    {
        int width = event.window.data1;
        int height = event.window.data2;
        SetupViewport(width, height);
        CORE.Window.screen.width = width;
        CORE.Window.screen.height = height;
        CORE.Window.currentFbo.width = width;
        CORE.Window.currentFbo.height = height;
        CORE.Window.resizedLastFrame = true;
    } break;
    case SDL_WINDOWEVENT_LEAVE:
    case SDL_WINDOWEVENT_HIDDEN:
    case SDL_WINDOWEVENT_MINIMIZED:
    case SDL_WINDOWEVENT_FOCUS_LOST:
    case SDL_WINDOWEVENT_ENTER:
    case SDL_WINDOWEVENT_SHOWN:
    case SDL_WINDOWEVENT_FOCUS_GAINED:
    case SDL_WINDOWEVENT_MAXIMIZED:
    case SDL_WINDOWEVENT_RESTORED:
    default: break;
}

@Bigfoot71 Since I have this ready, I'll add it to #3450. But feel free to revert it and properly fix it with SDL_AddEventWatch if you want. 👍

[raysan5]

@ubkp @Bigfoot71 Yeah, I prefer to avoid the callback system and just do the polling manually, I just left the event.window.event switch ready to use for this situations, actually the other event should be probably also processed accordingly and mapped to raylib window flags.