Implement FB_ngon_encoding extension

[scurest]

Implements the FB_ngon_encoding extension in the importer.

[emackey]

Wow! @scurest Are you planning to do the exporter too, or is that much harder?

/cc @zellski

[scurest]

No, just the importer. I don't know much about the exporter side so I'm not sure how hard it would be.

[emackey]

I don't think this PR needs to be blocked by export implementation, but, I do think we should sort out what's happening with the FB_ngon_encoding extension itself before merging this. @donmccurdy and I are restarting the conversation there, hopefully we can get that to converge soon.

Depending how that goes, could this importer PR apply a subdivision modifier to any mesh that came in with this extension on it?

[scurest]

I added support in the exporter after all. Quads are now encoded in output files. n-gons for n>4 are still triangulated.

Suzanne exported and then imported back in vs. a newly created
Suzanne. The quads round-tripped successfully.

It currently puts the extension on every primitive. I can put it behind an off-by-default export option instead if you like.

Would be really great to have someone familiar with extract_primitives check it and tell me if I did it right. I could also use help changing the splitting code so two tris from one quad don't get split across primitives (basically what I need is a split shouldn't occur between two tris with the same first index). Splitting code has been removed, so this is no longer an issue.

[scurest]

@emackey The discussion on the FB_ngon_encoding PR seems to have faded out. No one seemed very hot on having this though :( IMO this is a really nice extension. Pretty much everyone who took a look at it said it was a really elegant solution for what it does. At least in the importer, it seems like it could co-exist perfectly fine with other extensions for encoding arbitrarily complex n-gons or subdivision data as an option for a simple and lightweight way to encode quads. If you want to do something else instead though, that's fine of course. Let me know.

[emackey]

@scurest Thanks for adding export! This does look like a very nice implementation. The GitHub issue is stalled, but subdiv surfaces are still a hot topic for glTF. It may be some weeks yet before the dust settles, and it seems likely that some new extension name will be introduced before this is done.

[donmccurdy]

Per KhronosGroup/glTF#1620, the FB_ngon_encoding extension is likely complete. In its current form it will not become an official KHR_ or EXT_ extension, but we are still investigate alternatives that might — with the primary goal of supporting SubD.

That being the case... do we want to merge this PR to Blender? After some discussion, I'm undecided. Pros: it's likely helpful as-is to Blender users. Cons: The FB extension may be superceded by an official extension at some point, and we'd probably want to drop support for the FB extension then. FB themselves may drop support for the extension eventually, as happened with 3D Posts. And we don't want to create a de-facto standard before we have the chance to ratify an official extension that would better support SubD. These are the risks inherent to supporting a vendor extension in widely-used tools.

We could mitigate that somewhat by (1) putting this extension behind an export option, disabled by default, and/or (2) adding a warning in the tooltip and documentation that the option may be removed in the future. Because the extension allows graceful fallback when it's unsupported, the risks here seem lower than they'd be with many other vendor extensions. Or we could wait for an official version. Thoughts?

[Peach1]

• This extension should just be used for encoding quads, it does not need to imply subdivision, no need for conflict with subdivision specific extensions later on
• This extension allows for an ideal graceful transition to any future extension - it has no extra runtime or filesize cost, and displays correctly regardless of viewer support
• An exporter can simply switch to a better quad extension if one ever officially arrises, because again this extension has an unobtrusive design

---

The FB extension may be superceded by an official extension at some point

Any more advanced extension that supersedes this extension, is likely to add additional filesize compared to this extension. For users who want just quads with no extra filesize, this extension will be quite useful.

Pro:
Accepting this quad extension now, gives time for more quad-glTF files in the ecosystem.

The future subdivision extension will need quad based source geometry anyway, so if there are more quad-glTFs to test, the future subdivision extension will have a better ecosystem of quad-glTFs that can easily convert to using the future official subdivision extension

---

Supporting quads now -> quad-glTFs become more common place -> theoretical subdivision extension comes -> existing quad-glTFs can be easily upgraded to use the new subdivision extension because they have quad topology

Not supporting quads now -> zero quad-glTFs in ecosystem -> theoretical subdvision extension comes -> no glTFs can be easily upgraded directly to use the new extension because there are zero quad-glTFs in the ecosystem -> theoretical extension now needs time to be integrated into applications like Blender and you would need to retopologize or re-export all existing glTF models to use quads before you can even use the new subdivision extension

---

I highly recommend the Supporting quads now option.

do we want to merge this PR to Blender
Please Yes. Quads are an essential part of subdivision, so having more quad-glTFs in the ecosystem from now on, will only help future subdivision extensions have more quad-glTF models to test with anyway. Supporting quads now is going to increase glTF adoption and make the transition to an official subdivision glTF system smoother

[JayFoxRox]

Why was this branch closed? Is implementation of this extension still planned despite being stuck upstream?

[donmccurdy]

@JayFoxRox Unless an official extension is approved, or seems close to being approved, I don't think we are going to merge support for the extension here. Shipping half-finished extensions in Blender would create a mess that other glTF tools have to deal with. I would suggest adding feedback on the glTF repository (e.g. KhronosGroup/glTF#1687) if quad support is useful to you, and sharing what you can about your use cases. Thanks!

[devernay]

Assimp already supports exporting glTF using this extension (which is fully backward compatible, so there's no risk of breaking anything): assimp/assimp#3695

[fire]

@scurest I am having trouble updating your blender extension to the latest Blender. :(

[scurest]

@fire Yeah, I know. Since this wasn't merged, I freely used the assumption that all polys are tris to optimize stuff. Are you doing the exporter or the importer?

[fire]

I got stuck on both the importer and the exporter.

1. The importer lost the ability to get the loop count and loop total.
2. I was using https://github.com/futurerealitylab/immersive-presentation/blob/362e04b04f8261e7217995d4ee453f6b85368f54/js/render/gltf/examples/PollyCastle/castle_polly_nomad.glb as the sample.
3. This would be a good test of the extension system to make ngon support mesh primitive extensions.
4. Having mesh primitive extensions would help future subdivision support.

[scurest]

You won't be able to do it with the extension system, it requires deep integration with the mesh code.

For the exporter, I believe one poly becomes adjacent loop triangles with the same polygon_index, so you can use that to find out what are supposed to be quads. The importer will be simpler if you just rewrite all the first part of the mesh code from scratch (up to here). Skip the merge verts stuff too. You'll need to track the loop_counts, but I think you can ignore loop_starts and do them at the end with np.cumsum(loop_counts).

[fire]

@scurest I don't want to have a 6 month waiting cycle for Blender stable releases, can you devise a way to do ngons and subdiv gltf2 extensions?

I'll see if I can do the importer first, since I have the castle_polly test model.

[scurest]

No, not interested in working on this.