[Renderer/raylib] make the renderer a header file

[TimothyHoytBSME]

Having the renderer as a .c file makes it difficult to include it in multiple files. Use case: initialize clay and renderer in main file, have a separate header/file to handle draw calls.(Not an issue of mine directly, but it was brought up in the discord chat.)

[TimothyHoytBSME]

Or, more specifically, what is the correct way to have a project with multiple header files that all reference clay.h and/or the raylib renderer?

[cD1rtX3]

Simple fix: add #pragma once to the top [of the renderer] or wrap like this:

#ifndef _CLAY_RENDERER
#define _CLAY_RENDERER

/* ... */

#endif

[TimothyHoytBSME]

@cD1rtX3 have you gotten that to work in this case? Two of us have tried both methods with different compilers with no luck. I keep getting either undefined or re-defined errors. Nothing I've tried eposes both clay and the renderer's methods in separate headers without errors.

[cD1rtX3]

I mean to the top of the renderer file, the one you're including multiple times. Both methods have worked for me, and are standard to most header files. Why are you including the renderer twice, anyway?

[Cyrus-Harding]

I mean to the top of the renderer file, the one you're including multiple times. Both methods have worked for me, and are standard to most header files. Why are you including the renderer twice, anyway?

Since the renderer requires Clay_Raylib_Initialize to replace the InitWindow function (often in main.cpp). The logic for rendering the GUI (in the case of a video game) might be in a GUI.cpp file.

[Cyrus-Harding]

The include guards do not work, the file needs to be "converted" to a header to work.

Can't seem to be able to upload the patch file so I'll embed it:

Edit: Made it a fork as well.

https://github.com/Cyrus-Harding/clay/tree/make_raylib_render_header

Edit 2: Removed the embedded patch as it's out dated and included a bug.

[TimothyHoytBSME]

@cD1rtX3 an end user should not need to do this. In the specified case, as @Cyrus-Harding has clearly pointed out, a user (in a larger project involving many other rendering operations) may want a separate file for part of the rendering. This would mean two files depend on both clay.h and clay_renderer_raylib.c (since the "render loop" is in the main). So, header guards should be included (as it already is with clay.h). Now, as an aside, the renderer file also does not include clay.h, but I presume that is because it is expected to be included after clay.h every time.

I finally managed to get it to work by putting header guards in every file and then could include both clay and raylib in multiple files (Then included the side headers into the main). The other issue I was running into was compiling with CMake when there were mulitple .cpp files (one with each header), because compiling separate .cpp files that each depend on clay.h and the renderer caused multiple definition errors. I'm sure this is just an error on how I am using CMake. To temporarily resolve this, I made the side files into single-file headers (without cpp files). If any insight could be provided on how to use CMake to compile multiple .cpp files with shared sources, much would be appreciated.

[TimothyHoytBSME]

Okay, so after some research and testing. In it's current form, the renderer file would need to be compiled and linked. However, after renaming the file to a .h, including header guards, inlining the functions, and changing the global declarations to:

#ifdef CLAY_RENDERER_RAYLIB_DEFINE
    Raylib_Font Raylib_fonts[10]; // Define the global here
    Camera Raylib_camera;
#else
    extern Raylib_Font Raylib_fonts[10]; // Declare the global
    extern Camera Raylib_camera;
#endif

Then having the define in the main:

#define CLAY_IMPLEMENTATION
#include "include/clay.h"
#define CLAY_RENDERER_RAYLIB_DEFINE
#include "./include/raylib/clay_renderer_raylib.h"
#include "drawer.hpp"

Everything works as it should and allows proper multiple headers in a project all depending on the same sources of clay.h and clay_renderer_raylib.h

To make this change will be simple enough, but all examples using the renderer will need the additional define and name change. Someone else more familiar with the full clay repo may need to make a PR.

[Cyrus-Harding]

#ifdef CLAY_RENDERER_RAYLIB_DEFINE
    Raylib_Font Raylib_fonts[10]; // Define the global here
    Camera Raylib_camera;
#else
    extern Raylib_Font Raylib_fonts[10]; // Declare the global
    extern Camera Raylib_camera;
#endif

Wouldn't

    inline Raylib_Font Raylib_fonts[10]; // Declare the global
    inline Camera Raylib_camera;

Suffice?

extern inline works as well, but in C++ at least inline automatically means extern inline.

That's what I did in my fork.

[TimothyHoytBSME]

Maybe, these concepts are still a bit new to me. I assumed inlining was for functions only.

[TimothyHoytBSME]

Yea, that worked fine, even if we leave the filename as a .c. That would make a PR easier. However, changing the file to a .h makes the IDE not yell about missing references.

[TimothyHoytBSME]

I made the changes in my fork, and changed the example includes to .h as well. Tried the cmake build, and getting slapped with multiple warning: missing braces around initializer [-Wmissing-braces]. Which I think is a separate issue. So, I will leave this PR to someone more qualified.

[cD1rtX3]

I think the -Wmissing-braces is a bug in GCC. I got that too, and simply ignoring it makes the app work fine.

[TimothyHoytBSME]

I worked out the braces bug with some help. I could do a PR for this now, and just test build with the braces fix, and then change the files back before submitting. The question now, for @nicbarker is should we make this a minor breaking change by changing the filename to clay_renderer_raylib.c to clay_renderer_raylib.h? It's easy enough to change the internal examples, but it would affect everyone's external projects using the renderer. They would just need to change the name in the include. Otherwise, we can leave it as .c, but in my case, intellisense yells at me.

[peterc-s]

I have a PR from a while ago (#121) that moves the renderer to a header file, though I think it's a bit out of date now with the breaking changes made since I made it.

[TimothyHoytBSME]

@peterc-s Cyrus and I both have working fixes, just need to know whether to change the filename I think.

[cD1rtX3]

Could one not simply generate a header file to support your use-case but not break existing code which uses the .c file?

[TimothyHoytBSME]

The only breaking change would be the filename change. The rest of the file would function as usual. And, as I said, we could leave the filename alone, but it's not ideal. However, long-term, many more use-cases will require it to be a header, as it needs to be. Better to make a few people change one single character now than to have thousands change it later. The clay.h file is already written this way for the same reasons: to allow modularity and flexibly for a broad set of use-cases.

[peterc-s]

Could one not simply generate a header file to support your use-case but not break existing code which uses the .c file?

In my PR all I did was move it all to a header file with an extra guard. The only breakage is changing .c to .h wherever it's referenced, and adding a #define above the include for the implementation, similar to using clay.h.

However, since I opened that PR a while ago, there are now merge conflicts and probably new examples (I haven't been following) that use the raylib renderer which I haven't modified to use the header version.

[nicbarker]

I'm totally fine to have breaking changes if they're better for the project in the long run, we're still in pre 1.0 🙂
Sounds to me like using .h files for the renderers is going to be a good idea going forward - originally they were just designed to be examples that you might copy paste and adapt to your own use case, but it seems people are using them out of the box, so we may as well make them nicer to use!

[nicbarker]

@Cyrus-Harding

#ifdef CLAY_RENDERER_RAYLIB_DEFINE
Raylib_Font Raylib_fonts[10]; // Define the global here
Camera Raylib_camera;
#else
extern Raylib_Font Raylib_fonts[10]; // Declare the global
extern Camera Raylib_camera;
#endif

Wouldn't

inline Raylib_Font Raylib_fonts[10]; // Declare the global
inline Camera Raylib_camera;

Suffice?

extern inline works as well, but in C++ at least inline automatically means extern inline.

That's what I did in my fork.

We can actually avoid this altogether given that we can now pass user data into the measuretext function - I just haven't had time to actually write up the change. Will be nice to get rid of the fonts being global 🙂

Take a look at the SDL2 implementation for an example:
https://github.com/nicbarker/clay/blob/main/renderers/SDL2/clay_renderer_SDL2.c#L18

[TimothyHoytBSME]

Thanks for the confirmation @nicbarker. In that case, I will get a PR submitted for the basic header conversion sometime today. Then later we can probably do a separate issue/PR to remove the globals.

[TimothyHoytBSME]

@nicbarker I ran into several other issues when trying to implement this change. For some reason, the tricks that worked in an isolated example are conflicting in the main repo, particularly inlining the globals. So maybe it should wait until we rewrite that as well.