Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[models] Display problem of vertex colors in the Raylib library for M3D models #3109

Closed
2 of 3 tasks
Bigfoot71 opened this issue Jun 13, 2023 · 11 comments
Closed
2 of 3 tasks
Labels
external This issue depends on external lib

Comments

@Bigfoot71
Copy link
Contributor

Bigfoot71 commented Jun 13, 2023

  • I tested it on latest raylib version from master branch
  • I checked there is no similar issue already reported - (see issue #2855)
  • My code has no errors or misuse of raylib

Issue description

I'm having a problem using raylib to display M3D models with vertex colors. The vertex colors are not displayed correctly in my scene, while the official m3dview viewer manages to display the vertex colors correctly.

Environment

Desktop, Windows 10, OpenGL 3.3:

INFO: GL: OpenGL device information:
INFO:     > Vendor:   NVIDIA Corporation
INFO:     > Renderer: NVIDIA GeForce GTX 980/PCIe/SSE2
INFO:     > Version:  3.3.0 NVIDIA 535.98
INFO:     > GLSL:     3.30 NVIDIA via Cg compiler

Issue Screenshot

m3dview-1 m3d
raylib-1 m3d

Code Example

test-m3d.zip

@Bigfoot71 Bigfoot71 changed the title [module] Short description of the issue/bug/feature [models] Display problem of vertex colors in the Raylib library for M3D models Jun 13, 2023
@chriscamacho
Copy link
Contributor

have you checked vertex colours are being loaded?

how are you rendering the model, from memory I'm not sure the default shader uses vertex colours, if not you'll need to use a shader that uses these...

@Bigfoot71
Copy link
Contributor Author

@chriscamacho Effectively, the vertex colors are just not loading, so I will have to write a solution myself, thanks for your answer.

@Bigfoot71
Copy link
Contributor Author

I am back to the problem I presented here, and it turns out that you simply needed to add the following to LoadM3D in rmodels.c after something like line 5625:

model.meshes[k].colors = (unsigned char *)RL_CALLOC(model.meshes[k].vertexCount*4, sizeof(unsigned char));

And the rest of the code was already implemented here: L2655

And the model displays its beautiful vertex colors! :D

@raysan5
Copy link
Owner

raysan5 commented Jul 14, 2023

@Bigfoot71 Feel free to send a PR

@raysan5 raysan5 reopened this Jul 14, 2023
@Bigfoot71
Copy link
Contributor Author

Bigfoot71 commented Jul 14, 2023

@raysan5 Well, I think if LoadM3D is designed this way, it seems that there is no direct, simple, and reliable way to know if there are vertex colors in the loaded m3d model and therefore determine whether mesh.colors should be allocated or not. Here's how the m3d model struct used to build the raylib model is designed (L210 & L508):

/* vertex entry */
typedef struct {
    M3D_FLOAT x;                /* 3D coordinates and weight */
    M3D_FLOAT y;
    M3D_FLOAT z;
    M3D_FLOAT w;
    uint32_t color;             /* default vertex color */
    M3D_INDEX skinid;           /* skin index */
#ifdef M3D_VERTEXTYPE
    uint8_t type;
#endif
} m3dv_t;

typedef struct {
    /* [...] */
    M3D_INDEX numvertex;
    m3dv_t *vertex;             /* vertex data */
    /* [...] */
} m3d_t;

In the m3d model, color is not an array itself like in raylib; it is an unsigned int contained within the vertex struct array.

I might be mistaken, and someone more knowledgeable about this file format could correct me, but the only solution I can think of would be to allocate mesh.colors by default and fill it with white if there are no colors, which I don't think is desirable.

The solution I proposed above is correct for me in the sense that I'm sure there will be vertex colors for all my models.

The current method used in LoadM3D is to check if you have allocated mesh.colors and add the colors accordingly (L5655)

If we allocate mesh.colors by default but there is no vertex color in the model the function will return 0x00000000 for all vertex colors, which can be problematic.

@raysan5
Copy link
Owner

raysan5 commented Aug 6, 2023

@bztsrc please, could you take a look to this issue?

@raysan5 raysan5 added the external This issue depends on external lib label Aug 6, 2023
@bztsrc
Copy link

bztsrc commented Aug 10, 2023

please, could you take a look to this issue?

Sure!

it seems that there is no direct, simple, and reliable way to know if there are vertex colors in the loaded m3d model

No, because M3D can store both vertex colors and materials (with textures). You can have a model with colors, that's only used by loaders that do not load materials; and never be shown by loaders that support materials and textures.

In the m3d model, color is not an array itself like in raylib; it is an unsigned int

Of course, because color is a 32-bit packed pixel with separate RGBA channels.

the only solution I can think of would be to allocate mesh.colors by default

You don't need mesh.colors at all. Even if the model uses a color-map, the SDK automatically converts that into 32-bit RGBA packed pixels, so all you can see is a consistent, true-color compatible code.

And this is intended. You shouldn't know if the compression algorithm optimized the colors into a palette or not; you should get the same RGBA pixels for the vertex colors no matter how those were stored in the file.

I am back to the problem I presented here, and it turns out that you simply needed to add the following to LoadM3D in rmodels.c after something like line 5625:

Now this is a different thing. This relates to the VertexBuffer that the GPU uses. Yes, you could always allocate the colors array if you want, that won't hurt.

If we allocate mesh.colors by default but there is no vertex color in the model the function will return 0x00000000 for all vertex colors, which can be problematic.

No. It would fill up mesh.colors with black pixels in worst case, this isn't problematic at all. BTW, even if the allocation fails and mesh.colors indeed becomes 0x0000000, there are checks to avoid NULL dereference, so that wouldn't be a problem either.

To sum it up, I believe adding #3109 (comment) would waste some memory for vertex color-less models, but otherwise I see no harm at all. I think it should be merged.

Cheers,
bzt

@raysan5
Copy link
Owner

raysan5 commented Aug 10, 2023

To sum it up, I believe adding #3109 (comment) would waste some memory for vertex color-less models, but otherwise I see no harm at all. I think it should be merged.

I think this is a key point, most models nowaday do not use vertex colors, allocating vertex colors for every loaded model would be, in most cases, a waste of memory.

@bztsrc
Copy link

bztsrc commented Aug 10, 2023

I think this is a key point, most models nowaday do not use vertex colors, allocating vertex colors for every loaded model would be, in most cases, a waste of memory.

Yes, you're right, it's not common. Here's a proper solution: rmodels.c.zip

In this, when the number of vertices counted for a material, I've added an extra check to see if all vertex colors are fully transparent black or not. The mesh.colors array is only allocated if no materials is defined or there's at least one non fully transparent black color. I've tested this, it works perfectly with normal models (material only), models without materials (which supposed to use colors), and models with both materials and colors (in this case raylib's shader seems to mix the vertex color and the texture's color).
raylibm3d

NOTE: I've also added the materialCount fix from #3177 to this version.

Cheers,
bzt

ps: I'm terribly sorry for creating an attachment instead of a PR, but I have troubles with github ever since it changed to this JS heavy interface. Sorry about this!

@Bigfoot71
Copy link
Contributor Author

Bigfoot71 commented Aug 10, 2023

I believe there was a misunderstanding with my previous message, I must have expressed myself poorly. It's not a big deal though.

So, we are indeed required to reiterate through the faces initially to determine if there are vertex colors?
This could be heavy for the rare cases where we would need it?
Allowing the choice to allocate mesh.colors or not was therefore appropriate in a certain sense?

I had not seen that the loop used to browse the faces was already present, it looks perfect!

raysan5 added a commit that referenced this issue Aug 10, 2023
@raysan5
Copy link
Owner

raysan5 commented Aug 10, 2023

@bztsrc thank you very much for the review! just merged your changes!

I'm terribly sorry for creating an attachment instead of a PR, but I have troubles with github ever since it changed to this JS heavy interface. Sorry about this!

No problem! I agree this new interface is really slow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external This issue depends on external lib
Projects
None yet
Development

No branches or pull requests

4 participants