raysan5 / raylib

A simple and easy-to-use library to enjoy videogames programming
http://www.raylib.com
zlib License
22.21k stars 2.24k forks source link

[models] MTL files fail to load automatically on Android #3139

Closed Bigfoot71 closed 1 year ago

Bigfoot71 commented 1 year ago

Issue description

The MTL files linked to an OBJ file fail to load automatically on Android when loading the OBJ file, whereas for the same example, it works on PC.

Android logs:

2023-06-28 15:05:28.324 22364-22505 raylib                  com.raylib.objmtltest                I  FILEIO: [suv.obj] Text file loaded successfully
2023-06-28 15:05:28.342 22364-22505 raylib                  com.raylib.objmtltest                I  MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/0 materials
2023-06-28 15:05:28.342 22364-22505 raylib                  com.raylib.objmtltest                I  MODEL: No materials, putting all meshes in a default material
2023-06-28 15:05:28.344 22364-22505 raylib                  com.raylib.objmtltest                I  VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
2023-06-28 15:05:28.344 22364-22505 raylib                  com.raylib.objmtltest                W  MATERIAL: [suv.obj] Failed to load material data, default to white material

Desktop logs:

INFO: FILEIO: [suv.obj] Text file loaded successfully
INFO: MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/8 materials
INFO: MODEL: model has 8 material meshes
INFO: VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 3] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 4] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 5] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 6] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 7] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 8] Mesh uploaded successfully to VRAM (GPU)
INFO: VAO: [ID 9] Mesh uploaded successfully to VRAM (GPU)

Issue Screenshot

Android: android-screenshot

Desktop: desktop-screenshot

Code Example

Example Android/Desktop (.zip)

#include "raylib.h"
#include "raymath.h"

int main()
{
    InitWindow(800, 450, "raylib - OBJ/MTL test");

    Camera camera = { };
    camera.position = { 10.0f, 10.0f, 10.0f };
    camera.target = { 0.0f, 0.0f, 0.0f };
    camera.up = { 0.0f, 1.0f, 0.0f };
    camera.fovy = 45.0f;
    camera.projection = CAMERA_PERSPECTIVE;

    Model models = LoadModel("suv.obj");
    {
        BoundingBox bb = GetModelBoundingBox(models);
        Vector3 center = { };
        center.x = bb.min.x  + (((bb.max.x - bb.min.x)/2));
        center.z = bb.min.z  + (((bb.max.z - bb.min.z)/2));
        Matrix matTranslate = MatrixTranslate(-center.x, 0, -center.z);
        models.transform = matTranslate;
    }

    SetTargetFPS(60);

    while (!WindowShouldClose())
    {
        UpdateCamera(&camera, CAMERA_ORBITAL);

        BeginDrawing();
        ClearBackground(RAYWHITE);
            BeginMode3D(camera);
                DrawModel(models, { 0, 0, 0 }, 1.0f, WHITE);
                DrawGrid(10, 1.0);
            EndMode3D();
        EndDrawing();
    }

    UnloadModel(models);
    CloseWindow();

    return 0;
}
raysan5 commented 1 year ago

@Bigfoot71 This issue could be related to https://github.com/raysan5/raylib/issues/3126

It requires debugging it to find why mtl loading fails...

Bigfoot71 commented 1 year ago

sorry i forgot to update, the error returned is that the file was not found when it can be loaded manually. I tried things with tinyobj_loader_c.h as replaced fopen with android_fopen but no results (and it seemed to call it via the correct macro anyway). I'm still trying to understand why, if I ever manage to solve the problem I'll submit a PR, I'll get back to it as soon as I have time

I also tried to modify the obj file by indicating relative, absolute, etc paths to the mtl in doubt but the same

raysan5 commented 1 year ago

@Bigfoot71 It seems it could be a paths issue, try printing the full model path and compare it with the .mtl path. You can try with GetApplicationDirectory() and GetWorkingDirectory().

Bigfoot71 commented 1 year ago

@raysan5 I have obviously already checked this without a positive conclusion on my part.

The name of the MTL file is obtained here in tinyobj_loader_c.h, and upon checking it, it was completely correct: tinyobj_loader_c.h#L1319

So I suspected the working directory change here in LoadOBJ: rmodels.c#L3922 So, I first added +2 to the pointer to ignore the ./ which could potentially cause an issue when loading on Android assets, but the result remained the same. Then, I changed the directory structure in the assets so that the model and the material are in the root of the assets folder, and I simply removed the change of working directory from LoadOBJ. However, the result was still the same.

To be precise, the exception occurs here: tinyobj_loader_c.h#L749 Right after that, the error returned is No such file or directory. errno (2) with a file path that appears to be correct when displayed here. (and a working directory which also seems correct to reach the files)

raysan5 commented 1 year ago

@Bigfoot71 As an idea, you can try loading the .mtl file directly from your code and see if the path is found.

Bigfoot71 commented 1 year ago

@raysan5 Yes, of course, if I use LoadFileData, the file is loaded correctly.

I have just performed some tests with LoadMaterials, and the return from tinyobj_parse_mtl_file here is indeed TINYOBJ_SUCCESS. However, I'm getting this error: FILEIO: [1.png] Failed to open file (this is the texture linked to the MTL file, which I can load correctly with LoadTexture).

It seems that there is an issue with tinyobj_loader_c.h loading files itself via the Android assets manager. By the way, I don't know why it didn't occur to me earlier, but changing the working directory to an Android application's assets directory doesn't make any sense, and it's normal that it fails.

It seems that we need to rewrite tinyobj_loader_c.h to make it compatible with Android, despite the fact that it appears (according to my IDE) to be using the correct fopen macro, which actually calls the android_fopen function in utils.c (here). (In practice, this doesn't seem to be the case. I would need to perform more thorough tests and attempt to rewrite all of it if I have time in the next few days.)

raysan5 commented 1 year ago

Yes, a new C obj loader library is required with improvements over tinyobj_loader_c.h but implementation from scratch requires a considerable amount of work. Requirements:

Creating and maintaining that library requires some work...

Related comment: https://github.com/raysan5/raylib/issues/2219#issuecomment-1004250177

raysan5 commented 1 year ago

@Bigfoot71 A small change was pushed to tinyobj_loader_c.h, please, could you try again?

Bigfoot71 commented 1 year ago

@raysan5 Unfortunately, the material file is still not loaded. Here is what was displayed in logcat:

2023-08-04 19:24:37.587 10347-10477 raylib com.raylib.objmtltest I FILEIO: [suv.obj] Text file loaded successfully
2023-08-04 19:24:37.606 10347-10477 raylib com.raylib.objmtltest I MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/0 materials
2023-08-04 19:24:37.606 10347-10477 raylib com.raylib.objmtltest I MODEL: No materials, putting all meshes in a default material
2023-08-04 19:24:37.609 10347-10477 raylib com.raylib.objmtltest I VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)

While the file is present: Capture

I believe the problem is indeed that tinyobj_loader_c.h is not searching in the assets via the AssetManager.

Here is my test project: android.zip

In any case, I will start the update of the raymob repository tomorrow, and I will also conduct tests regarding this issue to see if my assumption is correct.

Update: By the way, if the problem indeed lies in the fact that it does not search within the assets, the fact that everything "external" is completely detached from raylib could be annoying because it would require completely rewriting the file loading logic with the AssetManager in this header.

Alternatively, we could consider that it is not possible, in its current state, to automatically load material files from tinyobj_loader_c.h, and that it is up to the user to do it manually.

I will conduct the tests to see if that is indeed the issue, and if it turns out to be the case, you will decide whether the solution is acceptable or not.

Bigfoot71 commented 1 year ago

I did some tests and it finally seems that tinyobj_loader_c.h indeed has access to the macro #define fopen(name, mode) android_fopen(name, mode) defined in utils.h and which gives it access to the AssetManager .

So I added trace logs everywhere (in android_fopen and in tinyobj_loader_c.h) and here is the result:

2023-08-07 13:48:46.403 10828-10908 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.obj
2023-08-07 13:48:46.403 10828-10908 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - SUCCESS: suv.obj
2023-08-07 13:48:46.406 10828-10908 raylib                  com.raylib.objmtltest                I  FILEIO: [suv.obj] Text file loaded successfully
2023-08-07 13:48:46.420 10828-10908 raylib                  com.raylib.objmtltest                I  TINYOBJ LOADER - .mtl path: suv.mtl
2023-08-07 13:48:46.420 10828-10908 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.mtl
2023-08-07 13:48:46.420 10828-10908 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - FAIL: suv.mtl
2023-08-07 13:48:46.424 10828-10908 raylib                  com.raylib.objmtltest                I  MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/0 materials
2023-08-07 13:48:46.424 10828-10908 raylib                  com.raylib.objmtltest                I  MODEL: No materials, putting all meshes in a default material
2023-08-07 13:48:46.426 10828-10908 raylib                  com.raylib.objmtltest                I  VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
2023-08-07 13:48:46.426 10828-10908 raylib                  com.raylib.objmtltest                W  MATERIAL: [suv.obj] Failed to load material data, default to white material

Capture

So tinyobj_loader_c.h gets the name of the mtl file to load, it calls the right function to load it from the assets but the file is not found in the assets. Whereas if we load suv.mtl ourselves with LoadFileData() it is indeed found. Surely there is a problem with the working directory, I still try things but nothing is conclusive for the moment.

Update

Even removing everything related to the working directory change in the LoadOBJ() function of rmodels.c the file still fails to load from assets, while the working directory still remains "/ ".

Small example with the bonus of the "proof" that this same file can be loaded manually without any problem:

2023-08-07 14:05:03.082 13871-14051 raylib                  com.raylib.objmtltest                I  WORKING DIR: /
2023-08-07 14:05:03.082 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.obj
2023-08-07 14:05:03.082 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - SUCCESS: suv.obj
2023-08-07 14:05:03.084 13871-14051 raylib                  com.raylib.objmtltest                I  FILEIO: [suv.obj] Text file loaded successfully
2023-08-07 14:05:03.098 13871-14051 raylib                  com.raylib.objmtltest                I  TINYOBJ LOADER - .mtl path: suv.mtl
2023-08-07 14:05:03.098 13871-14051 raylib                  com.raylib.objmtltest                I  WORKING DIR: /
2023-08-07 14:05:03.098 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.mtl
2023-08-07 14:05:03.098 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - FAIL: suv.mtl
2023-08-07 14:05:03.101 13871-14051 raylib                  com.raylib.objmtltest                I  MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/0 materials
2023-08-07 14:05:03.101 13871-14051 raylib                  com.raylib.objmtltest                I  MODEL: No materials, putting all meshes in a default material
2023-08-07 14:05:03.104 13871-14051 raylib                  com.raylib.objmtltest                I  VAO: [ID 2] Mesh uploaded successfully to VRAM (GPU)
2023-08-07 14:05:03.104 13871-14051 raylib                  com.raylib.objmtltest                W  MATERIAL: [suv.obj] Failed to load material data, default to white material

[...] (manual loading)

2023-08-07 14:05:03.105 13871-14051 raylib                  com.raylib.objmtltest                I  WORKING DIR: /
2023-08-07 14:05:03.105 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.mtl
2023-08-07 14:05:03.105 13871-14051 raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - SUCCESS: suv.mtl
2023-08-07 14:05:03.105 13871-14051 raylib                  com.raylib.objmtltest                I  FILEIO: [suv.mtl] File loaded successfully

Ideas ?

raysan5 commented 1 year ago

@Bigfoot71 this is really strange... no idea... 🤔

raysan5 commented 1 year ago

I'm thinking about it... one idea is that internal loading is called from raylib library location while user loading is called from main program location... maybe getting and printing the current full path for the call?

Bigfoot71 commented 1 year ago

@raysan5 Well, unless I am mistaken, the full path is the one indicated in the logs that I shared (in addition to the working directory but which does not change, and which is not supposed to have any impact on the AssetManager).

And this difference between the location of the library and that of the program does not make too much sense on Android, moreover in my implementation, the game and raylib are the same native library. (identical problem also for me with the "makefile" version of the official template, which requires raylib precompiled in static library).

Here is the android_fopen function which gives these logs:

// Replacement for fopen()
// Ref: https://developer.android.com/ndk/reference/group/asset
FILE *android_fopen(const char *fileName, const char *mode)
{
    TraceLog(LOG_INFO, "WORKING DIR: %s", GetWorkingDirectory());
    TraceLog(LOG_INFO, "ANDROID FOPEN: %s", fileName);

    if (mode[0] == 'w')
    {
        // fopen() is mapped to android_fopen() that only grants read access to
        // assets directory through AAssetManager but we want to also be able to
        // write data when required using the standard stdio FILE access functions
        // Ref: https://stackoverflow.com/questions/11294487/android-writing-saving-files-from-native-code-only
        #undef fopen
        return fopen(TextFormat("%s/%s", internalDataPath, fileName), mode);
        #define fopen(name, mode) android_fopen(name, mode)
    }
    else
    {
        // NOTE: AAsset provides access to read-only asset
        AAsset *asset = AAssetManager_open(assetManager, fileName, AASSET_MODE_UNKNOWN);

        if (asset != NULL)
        {
            TraceLog(LOG_INFO, "ANDROID FOPEN - SUCCESS: %s", fileName);

            // Get pointer to file in the assets
            return funopen(asset, android_read, android_write, android_seek, android_close);
        }
        else
        {
            TraceLog(LOG_INFO, "ANDROID FOPEN - FAIL: %s", fileName);

            #undef fopen
            // Just do a regular open if file is not found in the assets
            return fopen(TextFormat("%s/%s", internalDataPath, fileName), mode);
            #define fopen(name, mode) android_fopen(name, mode)
        }
    }
}
kusius commented 1 year ago

Hi everyone.

Took a look with the android debugger and it seems like indeed there's an issue with how tinyobj_loader parses that particular obj given in your example @Bigfoot71 . Here's what has been parsed in tinyobj_parse_obj()

image

So, the resulting filename contains a trailing CR (\r) which is in turn given to android_fopen and then to android's AAssetManager_open function which seems to fail because of it.

Doing quick and dirty removal of this trailing CR loads the file successfully, but app crashes somewhere else which I did not investigate why. Maybe there's something horribly wrong with that obj file? or the obj loader needs refinement. For now, I do not know.

Bigfoot71 commented 1 year ago

@kusius Hehehe bingo!

I also made a dirty modification by changing slen = my_strnlen(s, len); to slen = my_strnlen(s, len) - 1; in my_strndup from tinyobj_loader_c.h, and the material file is loaded! (L486)

However, indeed, it crashes right after. Here are the logs from my end:

2023-08-09 02:07:04.971  3943-4100  raylib                  com.raylib.objmtltest                I  WORKING DIR: /
2023-08-09 02:07:04.971  3943-4100  raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.obj
2023-08-09 02:07:04.971  3943-4100  raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - SUCCESS: suv.obj
2023-08-09 02:07:04.974  3943-4100  raylib                  com.raylib.objmtltest                I  FILEIO: [suv.obj] Text file loaded successfully
2023-08-09 02:07:04.987  3943-4100  raylib                  com.raylib.objmtltest                I  TINYOBJ LOADER - .mtl path: suv.mtl
2023-08-09 02:07:04.987  3943-4100  raylib                  com.raylib.objmtltest                I  WORKING DIR: /
2023-08-09 02:07:04.987  3943-4100  raylib                  com.raylib.objmtltest                I  ANDROID FOPEN: suv.mtl
2023-08-09 02:07:04.987  3943-4100  raylib                  com.raylib.objmtltest                I  ANDROID FOPEN - SUCCESS: suv.mtl
2023-08-09 02:07:04.990  3943-4100  raylib                  com.raylib.objmtltest                I  MODEL: [suv.obj] OBJ data loaded successfully: 5 meshes/8 materials
--------- beginning of crash
2023-08-09 02:07:04.990  3943-4100  raylib                  com.raylib.objmtltest                I  MODEL: model has 8 material meshes
2023-08-09 02:07:04.995  3943-4100  libc                    com.raylib.objmtltest                A  Fatal signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0xd7fb35c0 in tid 4100 (ylib.objmtltest), pid 3943 (ylib.objmtltest)
2023-08-09 02:07:05.113  4107-4107  DEBUG                   pid-4107                             A  pid: 3943, tid: 4100, name: ylib.objmtltest  >>> com.raylib.objmtltest <<<
2023-08-09 02:07:05.118  4107-4107  DEBUG                   pid-4107                             A        #00 pc 0006ccf0  /data/app/com.raylib.objmtltest-R6PVd0o2fm85YasZ3JwMXw==/base.apk!libobjmtltest.so (offset 0x15000) (BuildId: c53adf52678fae57b76e005d59f6ef3ae7387988)
2023-08-09 02:07:05.119  4107-4107  DEBUG                   pid-4107                             A        #01 pc 0006c70f  /data/app/com.raylib.objmtltest-R6PVd0o2fm85YasZ3JwMXw==/base.apk!libobjmtltest.so (offset 0x15000) (LoadModel+70) (BuildId: c53adf52678fae57b76e005d59f6ef3ae7387988)
2023-08-09 02:07:05.119  4107-4107  DEBUG                   pid-4107                             A        #02 pc 0003d6ff  /data/app/com.raylib.objmtltest-R6PVd0o2fm85YasZ3JwMXw==/base.apk!libobjmtltest.so (offset 0x15000) (main+146) (BuildId: c53adf52678fae57b76e005d59f6ef3ae7387988)
2023-08-09 02:07:05.119  4107-4107  DEBUG                   pid-4107                             A        #03 pc 0004e849  /data/app/com.raylib.objmtltest-R6PVd0o2fm85YasZ3JwMXw==/base.apk!libobjmtltest.so (offset 0x15000) (android_main+64) (BuildId: c53adf52678fae57b76e005d59f6ef3ae7387988)
2023-08-09 02:07:05.119  4107-4107  DEBUG                   pid-4107                             A        #04 pc 00136e87  /data/app/com.raylib.objmtltest-R6PVd0o2fm85YasZ3JwMXw==/base.apk!libobjmtltest.so (offset 0x15000) (BuildId: c53adf52678fae57b76e005d59f6ef3ae7387988)
raysan5 commented 1 year ago

@kusius @Bigfoot71 Did you try with latest raylib from GitHub master branch? I just pushed a commit related trying to address this issue (#3126) some days ago: https://github.com/raysan5/raylib/commit/6094869e3e845e90e1e8ae41b98e889fb3e13e78

kusius commented 1 year ago

@kusius @Bigfoot71 Did you try with latest raylib from GitHub master branch? I just pushed a commit related trying to address this issue (#3126) some days ago: https://github.com/raysan5/raylib/commit/6094869e3e845e90e1e8ae41b98e889fb3e13e78

Hey @raysan5 sort of yes.. I tried manually changing that line it in the example project but it did not have any effect. I'll double check with the latest master.

raysan5 commented 1 year ago

@kusius Afaik, reding the text file as "rt" should process the line-breaks properly. Another possible test to veify this issue is just open the .mtl file with a text editor and convert the line endings to CRLF.

Bigfoot71 commented 1 year ago

@raysan5 I am using the repository precisely starting from this commit.

However, I have conducted tests and it appears that the crash occurs within this loop of LoadOBJ in rmodels.c: L4011

Strangely though, it never reaches the same iteration count for af. And one out of ten times, there is no crash and everything seems to have loaded, but no model is displayed.

I also recall seeing a log at one point stating that a null pointer was returned after a malloc call, but I didn't keep track of that moment and I haven't been able to retrieve the same log again, so this information should be taken with caution.

Bigfoot71 commented 1 year ago

@kusius Afaik, reding the text file as "rt" should process the line-breaks properly. Another possible test to veify this issue is just open the .mtl file with a text editor and convert the line endings to CRLF.

Done, I tried converting LF to CRLF using VSCode, but it didn't work. Then I used the dos2unix package via Mingw, and that didn't work either.

I had tried other OBJs from different sources, and the problem was the same. I also tried re-exporting it via Blender, but there was no conclusive result.

orcmid commented 1 year ago

@Bigfoot71

Done, I tried converting LF to CRLF using VSCode, but it didn't work. Then I used the dos2unix package via Mingw, and that didn't work either.

Based on the fact that a \r showed up in a filename in the model-loading process, I am unclear how converting all LF to CRLF is going to help with that. It seems to me that you might want to do the opposite and use dos2unix rather than unix2dos.

It looks like there is a parsing problem in the model loading code. A sane solution, generally, is to treat all \r' and\n` codes as white space and be flexible about treating runs of whitespace as a single whitespace. I assume the "parser" in model loading is more rigid than that. My guesswork only.

Runs of the failing program having varying success/failure patterns from one run to the next signals that there may be an uninitialized variable or an over-run value somewhere. This could happen with a pointer misuse too (e.g., using the pointer instead of what it is pointed at, or even vice versa). It might even matter whether char is handled as a signed or unsigned small int.

Final observation. In the distant past, there was a time when \r alone was used the same as \n is in Unix, not in a pair. Also, use of \r alone can be intentional to overwrite a line in output to a stdout terminal, as when presenting a countdown, or other busy behavior, without scrolling down the screen. That usually works independent of whether \n is converted to a single LF or a CRLF in the stream to the device. Here, I suppose, it might be important to know whether the compiler used handles \r properly if at all or whether there is an ill-escaped usage.

Bigfoot71 commented 1 year ago

@orcmid Oh yes, indeed, I misunderstood Ray's message and acted without thinking. Otherwise, yes, I did use the unix2dos command from the dos2unix package, sorry twice for these mistakes.

BUT 'SOLUTION' FOUND

I must have been too tired earlier, the OBJ file was already in CRLF, and here's what the debugger was showing: CRLF

And the loading was failing, then I converted the OBJ file to LF and the loading works! LF Screenshot_2023-08-09-22-05-54-429_com raylib objmtltest

So we have to understand why it only happens on Android now...

raysan5 commented 1 year ago

So we have to understand why it only happens on Android now...

Maybe because Android is a Linux system and expects text files line-endings as LF instead of CRLF.

raysan5 commented 1 year ago

In any case, it seems this issue is out of the domain of raylib... so I'm closing it.