[models] Question about how to add hooks

[bztsrc]

Hi,

First I'll try to explain the problem in a nutshell.

All model formats I have seen so far simply store the filepath of the assets in the model file. I find that solution conceptually broken for several reasons, and to my experience there's lot of problems with models because of this. For example:

1. people tend to store absolute paths in them, which is just plainly wrong, my directory structure is NOT the same as the model creator's
2. even if it's a relative path, there's always portability issues (Win directory separator is \, while Linux uses /, VMS uses [dir.dir]file etc.)
3. not all engines store the assets in separate files. Many use some kind of module or DLC system (for example, using ZIP or in-house developed archives)
4. what if the asset is inlined in the model file?
5. what if the model creator have used .bmp for textures, but an engine doesn't want to use that format (for obvious reasons)?

So when I designed M3D, I took a different approach, only the asset's unique name is stored in the model file (kinda like an ID), and the application using the M3D SDK supposed to provide two hooks: the first one receives the name of the asset and returns a pointer to a buffer, and the second optional one frees that buffer if it was allocated in the first hook.

However the M3D SDK is hidden with raylib (which is good practice), and is integrated in raylib is as follows (actually I wanted to use the raylib functions as-is, but there's a prototype difference with the arguments, char* vs const char* and void* vs. unsigned char*, hence the need for these wrapper functions):

unsigned char *m3d_loaderhook(char *fn, unsigned int *len) { return LoadFileData((const char *)fn, len); }
void m3d_freehook(void *data) { UnloadFileData((unsigned char *)data); }

Obviously this simple solution only works for the most trivial use-case, when assets are stored in separate files in the same current working directory (or all of them stored in the same archive). I'd like to change that, and provide a way for the application to specify these hooks and therefore implement whatever asset storing mechanism they'd like.

(FYI, I've always wanted to use LoadFileData because it has SetLoadFileDataCallback, but when I actually have tried this, I run into issues: it's common to all file operations, both for the model itself and the assets, and not just for model's assets. The model is passed as a filename, but assets as a unique name, and assets could be stored in a different place (like in a "skin DLC" for example). And then I'm not sure what about free as a custom storage mechanism probably provides its own freeing, therefore the asset buffers shouldn't be freed when the model is unloaded, otherwise that would result in double free (or triple or quadriple if several models are using the same texture asset). Currently there's no way of turning the UnloadFileData call off in m3d_freehook.)

My question is, since only model's filename is passed to LoadModel and not the asset's hooks, what would be the preferred, raylib philosophy compatible way here? I am thinking about using two static function pointers, which are initialized as m3d_loaderhook and m3d_freehook. Then the app can change these before the first LoadModel call if it wishes to implement a custom asset storage mechanism. What do you think about this? If using two static function pointer variables is okay, what would be your preference for the names for these two that aligns with raylib? My guess is, they should somehow reflect being used in LoadModel, so maybe LoadModelAssetGet and LoadModelAssetFree? Or should a SetLoadModelAssetCallback be added?

If you don't like the idea of two static variables, then I'm open to any suggestion. Any solution you prefer and allows the application to provide its own way of loading the assets is okay. (Again, an app might wish to use different hooks for loading the model and the assets, and on unload the model's buffer must be freed but the asset's not necessarily.)

Thank you for your answer,
bzt

[bztsrc]

Here's the proposed solution. Let me know if you need any change. I've deliberately not used m3d related variable names, because separate asset loaders might come in handy for other formats as well.

I've added a single SetLoadAssetCallbacks API call, because I saw no point in setting the load/free hooks independently. If these aren't set, then the behaviour is 100% backward compatible (eg. uses LoadFileData and UnloadFileData).

diff --git a/src/raylib.h b/src/raylib.h
index 1abce717..f33b9a56 100644
--- a/src/raylib.h
+++ b/src/raylib.h
@@ -906,6 +906,8 @@ typedef unsigned char *(*LoadFileDataCallback)(const char *fileName, unsigned in
 typedef bool (*SaveFileDataCallback)(const char *fileName, void *data, unsigned int bytesToWrite);  // FileIO: Save binary data
 typedef char *(*LoadFileTextCallback)(const char *fileName);            // FileIO: Load text data
 typedef bool (*SaveFileTextCallback)(const char *fileName, char *text); // FileIO: Save text data
+typedef unsigned char *(*LoadFileAssetCallback)(const char *fileName, unsigned int *bytesRead);      // FileIO: Load asset data
+typedef void (*FreeFileAssetCallback)(unsigned char *data);             // FileIO: Free asset buffer
 
 //------------------------------------------------------------------------------------
 // Global Variables Definition
@@ -1053,6 +1055,7 @@ RLAPI void SetLoadFileDataCallback(LoadFileDataCallback callback); // Set custom
 RLAPI void SetSaveFileDataCallback(SaveFileDataCallback callback); // Set custom file binary data saver
 RLAPI void SetLoadFileTextCallback(LoadFileTextCallback callback); // Set custom file text data loader
 RLAPI void SetSaveFileTextCallback(SaveFileTextCallback callback); // Set custom file text data saver
+RLAPI void SetLoadAssetCallbacks(LoadFileAssetCallback loadcallback, FreeFileAssetCallback freecallback); // Set custom file binary data loader
 
 // Files management functions
 RLAPI unsigned char *LoadFileData(const char *fileName, unsigned int *bytesRead);       // Load file data as byte array (read)
diff --git a/src/rmodels.c b/src/rmodels.c
index 290d6e29..72f87e02 100644
--- a/src/rmodels.c
+++ b/src/rmodels.c
@@ -5215,8 +5215,8 @@ static Model LoadVOX(const char *fileName)
 
 #if defined(SUPPORT_FILEFORMAT_M3D)
 // Hook LoadFileData()/UnloadFileData() calls to M3D loaders
-unsigned char *m3d_loaderhook(char *fn, unsigned int *len) { return LoadFileData((const char *)fn, len); }
-void m3d_freehook(void *data) { UnloadFileData((unsigned char *)data); }
+extern LoadFileAssetCallback loadFileAsset;
+extern FreeFileAssetCallback freeFileAsset;
 
 // Load M3D mesh data
 static Model LoadM3D(const char *fileName)
@@ -5231,7 +5231,7 @@ static Model LoadM3D(const char *fileName)
 
     if (fileData != NULL)
     {
-        m3d = m3d_load(fileData, m3d_loaderhook, m3d_freehook, NULL);
+        m3d = m3d_load(fileData, (m3dread_t)loadFileAsset, (m3dfree_t)freeFileAsset, NULL);
 
         if (!m3d || M3D_ERR_ISFATAL(m3d->errcode))
         {
@@ -5529,7 +5529,7 @@ static ModelAnimation *LoadModelAnimationsM3D(const char *fileName, unsigned int
 
     if (fileData != NULL)
     {
-        m3d = m3d_load(fileData, m3d_loaderhook, m3d_freehook, NULL);
+        m3d = m3d_load(fileData, (m3dread_t)loadFileAsset, (m3dfree_t)freeFileAsset, NULL);
 
         if (!m3d || M3D_ERR_ISFATAL(m3d->errcode))
         {
diff --git a/src/utils.c b/src/utils.c
index b165449b..44bc680c 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -67,6 +67,8 @@ static LoadFileDataCallback loadFileData = NULL;    // LoadFileData callback fun
 static SaveFileDataCallback saveFileData = NULL;    // SaveFileText callback funtion pointer
 static LoadFileTextCallback loadFileText = NULL;    // LoadFileText callback funtion pointer
 static SaveFileTextCallback saveFileText = NULL;    // SaveFileText callback funtion pointer
+LoadFileAssetCallback loadFileAsset = LoadFileData;    // LoadFileAsset funtion pointer
+FreeFileAssetCallback freeFileAsset = UnloadFileData;  // FreeFileAsset funtion pointer
 
 //----------------------------------------------------------------------------------
 // Functions to set internal callbacks
@@ -76,7 +78,9 @@ void SetLoadFileDataCallback(LoadFileDataCallback callback) { loadFileData = cal
 void SetSaveFileDataCallback(SaveFileDataCallback callback) { saveFileData = callback; }  // Set custom file data saver
 void SetLoadFileTextCallback(LoadFileTextCallback callback) { loadFileText = callback; }  // Set custom file text loader
 void SetSaveFileTextCallback(SaveFileTextCallback callback) { saveFileText = callback; }  // Set custom file text saver
-
+void SetLoadAssetCallbacks(LoadFileAssetCallback loadcallback, FreeFileAssetCallback freecallback) { // Set custom file asset loader
+    loadFileAsset = loadcallback; freeFileAsset = freecallback;
+}
 
 #if defined(PLATFORM_ANDROID)
 static AAssetManager *assetManager = NULL;          // Android assets manager pointer

Cheers,
bzt

[raysan5]

@bztsrc Hi! Excuse the late response! I think this is a complex topic, I've also found that issue when dealing with OBJ models and I couldn't find a good solution, I neither like the idea of leting the used library (tinyobjloader-c) to make the files loading for me, I would prefer to just get the path and manage it myself... but that implies other issues... as I said, no easy solution to the problem and I neither have a best-approach answer.

Not sure if I understand the issue and proposed solution correctly, current implementation using LoadFileData()/UnloadFileData() hooks seem to work fine... If I understand it correctly, the problem is leting the user to customize how the filename-id provided is processed to load the required data, is that right?

In any case, the proposed solution looks good, it's simple, clean and not intrusive.

[bztsrc]

Not sure if I understand the issue and proposed solution correctly, current implementation using LoadFileData()/UnloadFileData() hooks seem to work fine...

Except I keep getting issues about letting to load the model from a different directory than the assets (for example).

If I understand it correctly, the problem is leting the user to customize how the filename-id provided is processed to load the required data, is that right?

Yep, something like that. Ideally the asset-specific hook should be simple as

unsigned char *mySpecificAssetLoader(const char *name, unsigned int *bytesRead)
{
  static char fileName[1024];
  sprintf(fileName, "skins/%s/textures/%s.png", user->current_theme, name);
  return LoadFileData(fileName, bytesRead);
}

What path is used, always depends on the app, and therefore should not be stored in the models (that latter ideally should only store the file name, without any path). Furthermore, M3D does not store the extension either, because texture is just a unique name (an ID), and if the app adds a path anyway, it's cheap to add the desired extension too. (PNG is only mandated if the texture is inlined the model file.)

In any case, the proposed solution looks good, it's simple, clean and not intrusive.

Thanks! I'm glad you liked it. I definitely wanted to make it non-intrusive.

Cheers,
bzt

[raysan5]

@bztsrc Ok, I see. But, is that new hook mechanism required because the m3d file-format is changing? I mean, avoiding the use of filepaths directly inside the .m3d file, replacing it by that kind of filename-id (with no path and no extension).

But... the current callback will be specific to m3d file format, no other asset file will require that approach, right? I think it could be confusing for users... I mean, LoadFileAssetCallback sounds like all assets laoding will use that function...

Sorry for my insistence, I think I understand the issue but I'm still thinking about the best approach to it...

[bztsrc]

But, is that new hook mechanism required because the m3d file-format is changing?

Nope, the format hasn't changed. It was always designed with IDs and a hook in mind, but so far that hook was "hardwired" in raylib. Which works, but not changeable by the app ATM.

I mean, avoiding the use of filepaths directly inside the .m3d file, replacing it by that kind of filename-id (with no path and no extension).

Using an ID was always the case. This means that with the current "hardwired" hook an app couldn't change the path where the assets should be looked up, but it still can use it load external assets (provided the filenames are the same as the IDs).

But... the current callback will be specific to m3d file format, no other asset file will require that approach, right?

Yes and no. Currently only m3d uses a separate asset hook, but I'm planning to add a patch for other formats as well. An app might want to use separate asset directory with other formats too, but storing paths in model files isn't portable, and replacing the paths in all model files can be problematic, so I believe other formats could benefit from this new hook too greatly.

(For example it is possible that an OBJ file only stores filenames, and an app then could use this asset hook to add whatever path it uses for the material libraries and textures.)

I mean, LoadFileAssetCallback sounds like all assets laoding will use that function...

Yes, that's the plan. I wanted to add it in a way that other loaders could benefit from it as well in the future.

Sorry for my insistence, I think I understand the issue but I'm still thinking about the best approach to it...

No worries! I believe the best approach here is to introduce an API which could later be used by other formats. But you can rename it to something m3d specific of course if you think otherwise. If you decide to use a common name, then I'm planning to add a patch for the other formats as well (which defaults to LoadFileData/UnloadFileData therefore would be backward compatible). This would give the choice to apps to use it if they want dynamic asset paths.

Cheers,
bzt

[RobLoach]

While somewhat unrelated to what you're trying to accomplish, I had put together PhysFS integeration for raylib to allow loading assets from .zip files: https://github.com/RobLoach/raylib-physfs .

PhysFS is an abstracted file system API that allows "mounting" .zip files, or directories. In theory, if you're looking to compile the model asset directly in your application, you could...

1. Bundle it into a .zip file, compile the binary directly into your application
2. Use MountPhysFSFromMemory() to mount the compiled .zip binary directly from memory
3. Run SetPhysFSCallbacks() to tell raylib to use the PhysFS file callbacks
4. Load the model data as normal

[chriscamacho]

I'd also recommend physfs - a few notes https://bedroomcoders.co.uk/physfs-and-raylib/

[bztsrc]

While somewhat unrelated to what you're trying to accomplish

Nope, not unrelated, exactly in my street. I just want to have a way to specify the asset loader callback, so that it could add path and extension to the string (and pass that to PhysFS for example).

Cheers,
bzt