horde3d / Horde3D

Horde3D is a small 3D rendering and animation engine. It is written in an effort to create an engine being as lightweight and conceptually clean as possible.
http://horde3d.org/
1.55k stars 308 forks source link

Support meshes with line list primitive type #137

Closed jcmonnin closed 5 years ago

jcmonnin commented 5 years ago

This patch probably needs to be reviewed by @Irdis as it uses the primitive type to issue Quads for tessellation instead of separate flag. I thought it would be a good idea to avoid redundancy by removing flag, but I'm happy to redactor if this is not a good idea.

algts commented 5 years ago

There seems to be a bug in parsing (double check for patches and it sets lines):

if (_stricmp(itr->second.c_str(), "TriangleList") == 0) {
            meshTpl->primType = MeshPrimType::TriangleList;
        } else if (_stricmp(itr->second.c_str(), "Patches") == 0) {
            meshTpl->primType = MeshPrimType::LineList;
        } else if ( _stricmp( itr->second.c_str(), "Patches" ) == 0 ) {
            meshTpl->primType = MeshPrimType::LineList;
        } else {
            result = false;

Other than that, I don't see any problems, it can be safely integrated, if the sample works correctly. I just wanted to ask - if you are using lines for drawing, maybe it is better to create an extension specifically for drawing debug primitives, like lines and cubes, instead of adding functions to standard mesh?

jcmonnin commented 5 years ago

Thanks for catching the bug in the xml parsing. I will fix it.

In my fork, I was using an integer to specify the primitive type, but I thought it was better to modify it to a descriptive string. Obviously I didn't test well enough.

The samples run fine (including the tesselation example that I had to modify slightly). Also note that it modifies the public API call h3dAddMeshNode

I need lines in a normal scene (it's not really a debug view, but some technical drawing for engineers). You are right that an extension could do that too, but doing it in the way of this patch allows me to use geo resource to draw line (automatic support do define them in files) and also allows me to dynamically create and modify the vertices with the normal API like h3dMapResStream. This would all be possible with an extension, but with much more redundant code. This is why this implementation was my preferred choice.

I didn't expose the TriStrip and TriFan (and LineString or LineLoop) primitive types because I though they didn't make much sense on the mesh which is indexed drawing by design.

horde3d commented 5 years ago

So I'm waiting for a new PR to fix the outstanding parsing issue.