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

FB_ngon_encoding extension. #1620

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zellski
Copy link
Contributor

@zellski zellski commented May 29, 2019

We're in a situation where we want to do subdiv rendering on clients, where we want a quad-based mesh, and yet take advantage of all the backend infrastructure we've built around glTF. We came up with this scheme to retain general ngon mesh structure in glTF-valid triangle meshes.

We put this together for internal backend->client delivery & rendering, and there's no current plan to offer explicitly user-friendly download of assets that use it. Then again, they do get sent over the wire; folks will surely intercept & inspect them, and it feels like a goodness for any extended glTF that makes it into the world to be documented somewhere.

I'll leave it to y'all to decide if it makes sense in this repository. If not, I will probably drop it into the FBX2glTF repo, so that tool's output is at least documented.

@pjcozzi
Copy link
Member

pjcozzi commented Aug 18, 2019

@zellski thank you for proposing this.

I want to provide some thoughts on quads/subdivision in general in glTF. glTF was started in 2012 for efficient runtime and transmission. In the early days of glTF if you asked me about adding subdivision, I would have said that (1) that would be slow to compute at runtime, and (2) it would make the spec more complicated to implement and increase the barrier to adoption. I might have been right in the context of 2012 when there was less client-side processing power and it was pre wide-spread glTF adoption.

Today I think the definition of efficient runtime and transmission may be different and subdivision surfaces may fit the spirit perfectly. There is more client-side compute available for subdiv, the storage and bandwidth savings are significant, and the USDZ to glTF conversion pipeline will be cleaner. I also suspect the implementation effort is reasonable and still in the spirit of glTF, especially now that so many glTF runtimes exist and that open-source libraries can make integrating subdiv straightforward.

So whether it is this exact extension or a variation of it, I think this is a great direction for the glTF community to explore.

@zellski
Copy link
Contributor Author

zellski commented Aug 19, 2019

This is one of two plausible ways we brainstormed to provide information sufficient to reconstruct a quad / ngon tessellation from a vanilla-compatible triangulation.

The virtue of this proposal is compactness; it adds no size to the asset, but rather takes advantage of the redundancy of order-independence. That is also its downside: with this extension in place, tools are longer at liberty to reorder triangles and vertices as we please. Pipeline tools will have to make potentially annoying exceptions to optimisation steps when they encounter this extension.

The obvious alternative is to let vertices and triangles remain order-independent as per vanilla glTF. In this case, the information needed to reconstruct the original mesh has to be provided explicitly, likely through the use of an accessor. I imagine there's a few different ways to encode the required information, but one way or another it's likely to consist of references to vertices and/or triangles. Given that fact, I'm not sure we gain any real sturdiness compared to the implicit approach above. A pipeline tool will still have to understand the extension in order not to screw up those references with a hamfisted reordering.

@emackey
Copy link
Member

emackey commented Aug 20, 2019

I wonder if it's worth adding a flag here, to indicate whether the ngons are intended to be used as-is, or whether they represent a subdivision surface that should be further processed by something like the OpenSubDiv library.

@donmccurdy
Copy link
Contributor

donmccurdy commented Aug 21, 2019

That is also its downside: with this extension in place, tools are longer at liberty to reorder triangles and vertices as we please. Pipeline tools will have to make potentially annoying exceptions to optimisation steps when they encounter this extension.

This actually seems OK to me. In general, an optimization tool probably cannot and should not pass along an extension it doesn't recognize. To give a simple example, any extension could reference accessors, and an optimization that alters accessor order would trivially break the extension. The fact that this extension design allows the optimization tool to read a file with FB_ngon_encoding, optimize what it understands, and output a correct file (without FB_ngon_encoding) is a perfectly acceptable example of fallbacks working correctly.

I wonder if it's worth adding a flag here, to indicate whether the ngons are intended to be used as-is, or whether they represent a subdivision surface that should be further processed...

Not sure I understand – can anything be done with quads as-is, except (a) subdivision or (b) loading into a DCC tool? From my perspective, (a) is the only compelling reason to consider this extension, given the stated goals of glTF.

@emackey
Copy link
Member

emackey commented Aug 21, 2019

perfectly acceptable example of fallbacks working correctly.

Agreed. And this is an amazingly elegant solution to the question of quad/ngon storage.

Not sure I understand – can anything be done with quads as-is, except (a) subdivision or (b) loading into a DCC tool? From my perspective, (a) is the only compelling reason to consider this extension, given the stated goals of glTF.

Not all meshes are intended to be subdivided, for example a cube becomes a lumpy ball when subdivision happens. If that's the only reason this extension exists, then subdivision should be in the name of the extension, and clearly stated as the intended use of the quads & ngons.

If this extension is raw quad/ngon storage, and you find a cube of quads stored here, then what did the artist intend the viewer to see? Did the artist want a cube shown, or a ball? Granted, the end user's reasons for sending a non-subdivided cube as quads may be less obvious to the authors of the glTF spec, but we should either enable it or rule it out completely. The README Overview should not say "one common such case" is subdivision, when turning that on or off may have a large impact on the visual result. Subdivision needs to be an intentional choice by the artist, not a client interpretation.

And if we're targeting subdivision exclusively, do we need to include a mechanism for subdivision weights or creases? Is quad/ngon storage enough input to supply to OpenSubDiv? I'm not trying to overwhelm this PR, just trying to think through ways people might use this in the wild.

@bhouston
Copy link
Contributor

bhouston commented Aug 21, 2019

Concave polygons can not be converted into a valid triangle fan. But I guess this extension is for convex only polygons. I must admit that it is a very elegant solution for convex polygons.

I do think that my extension that adds an edge data array covers both concave and convex polygons as well as edge creases:

To make it concrete one can support subdivision and polygons with one array of edge values stored as signed bytes. A negative value means not a true edge (interior poly edge.) A positive value should be considered a crease weight between 0 and 1 of which there would be 128 possible discrete values.

Just define a deterministic face indices to edge indices ordering.

If this isn't supported you can still load and display the data as a normal glTF. If you do support this you can adjust glTF resolution.

This single linear byte array, which is no larger than 1/3 the size of the face indices array uncompressed, will compress very well using zip or brotli.

My proposal is also fully backwards compatible as well if the extra data is not supported.

@bhouston
Copy link
Contributor

bhouston commented Aug 21, 2019

Not all meshes are intended to be subdivided, for example a cube becomes a lumpy ball when subdivision happens. If that's the only reason this extension exists, then subdivision should be in the name of the extension, and clearly stated as the intended use of the quads & ngons.

This is a misunderstanding of subdivision. Correct subdivision involves the use of a value per edge that says whether it is a crease or not (or a value in between if you are doing crease weights). One smooths across edges that are not hard creases. Thus you can subdivision a cube and if you have specified it to have hard edges, it increases the polygon count without actually rounding the corners.

A lot of libraries, such as Three.JS have implemented subdivision incorrectly by assuming all edges are not creases.

Be aware that there are a few different crease representations:

  • In 3DS Max, it uses face smoothing groups and an edge is considered hard if adjacent faces do not share a smoothing group in common.
  • In vertex normal representations, if an interior edge has only one set of vertex normals rather than two sets, then it is smooth. Otherwise it is a hard crease.
  • In tools such as Maya that support variable weight creases (which aligns with OpenSubDiv), there is a scalar per edge that specifies how hard it is.

It is important to note that in complex models there may be different edge crease weights for normals and uvs. In those cases I suggest that one assume they are the same, and only if one specifies explicitly different edge crease weights for the other channels that they are different.

The proposal I am making supports all three because the most expressive/general format is the crease weight system. The other representations can be converted to it.

@donmccurdy
Copy link
Contributor

donmccurdy commented Aug 21, 2019

@bhouston Are you referring to another proposal or PR for subdivision? I'm not sure what you're quoting from.

@bhouston
Copy link
Contributor

@bhouston Are you referring to another proposal or PR for subdivision? I'm not sure what you're quoting from.

Well, it was just an email on the glTF mailing list -- and that was it -- thus it isn't formalized yet, but I think it is reasonable. Basically you need data per edge. Edge indices can be calculated deterministicly from a set of triangles -- though I am unsure if it is a O( t ) or an O( t log t ) operation where t is the number of triangles.

Once you have an edge indexing scheme, you can add attributes per edge as simple flat arrays. These can be a simple boolean interior edge or not -- this would give you a polygonization. And then more specific to sub-div, you can have edge crease weights for the various channels (normals, uvs, etc.)

@meshula
Copy link

meshula commented Aug 21, 2019

My team and I at Apple worked with the OpenSubdiv team to identify the minimum viable set of data to describe subdivision data, you can read our specification here:

https://developer.apple.com/documentation/modelio/mdlsubmeshtopology?language=objc

In addition to edge and vertex crease info, we found it necessary to also encode faces that function as holes. I think the information in our specification for ModelIO is complementary to this proposal.

@zellski
Copy link
Contributor Author

zellski commented Aug 25, 2019

Indeed; I described the limitation in this PR as "It's obvious that this can be done for convex polygons, e.g. quads, but the true constraint is a bit less strict."

I'm not really competent to evaluate @bhouston's solution, but I'm happy to assume at face value that it solves useful cases that this PR doesn't—ours was really hand-crafted for one specific purpose. If so, perhaps we should base a future EXT version on the more general solution, and not promote this vendor one any further.

That said, we're already fairly committed to this one internally, so if users go out of their way to intercept our server-to-client delivery GLBs, those may well require FB_ngon_encoding. If we ever added a 'download' button, we'd probably want to mark those up with an EXT.

@emackey
Copy link
Member

emackey commented Aug 26, 2019

Not all meshes are intended to be subdivided, [...]

This is a misunderstanding of subdivision. Correct subdivision involves the use of a value per edge [...]

Actually I'm trying to point out uses outside of subdivision entirely, for example BIM data, OpenStreetMap etc. There may be reasons (possibly analytical reasons) to preserve quads rather than triangulate everything. But one wouldn't want to ship edge weights or subdivision implications along with such data.

As a much more concrete example, we now have a PR open to import FB_ngon_encoding data into the official Blender importer, in KhronosGroup/glTF-Blender-IO#622. This import does not add the Blender "Subdivision Surface" mesh modifier when the data comes in. But should it? Should it blindly assume that the mere presence of FB_ngon_encoding implies that the subdiv modifier always gets applied? And why is the import software making such decisions on its own, shouldn't it be informed by some setting in the model?

@emackey
Copy link
Member

emackey commented Aug 26, 2019

And to be clear, I'm fine if the answer is "Yes, the mere presence of FB_ngon_encoding means that subdivision itself as a feature should be enabled", but the README needs to say so. Right now the README just says that clients can turn subdivision on or off at their own discretion.

@bhouston
Copy link
Contributor

bhouston commented Aug 26, 2019

(1)

Should it blindly assume that the mere presence of FB_ngon_encoding implies that the subdiv modifier always gets applied.

The mere existence of polygons do not imply subdivision unless you want to do it incorrectly. That said this is a vendor standard so the bar for rigorousness/correctness is lower as it merely had to meet FB's needs, not be how the rest of the high-end 3D industry does things.

(2)
I would also suggest renaming this to FB_convex_ngon_encoding as it doesn't handle convex nor holes.

(3)
The correct solution as Nick (@meshula) points out is to have true polygons and edge crease scalars and even vertex crease scalars.

My suggested backwards compatible alternative of an array of booleans, one per edge, to differentiate interior edges from polygon border edges, handles both concave polygons as well as holes in a backwards compatible fashion.

And then to do correct subdivision, add an identically sized array of scalar to specify edge crease weights to handle the proper subdivision boundaries. To accommodate Nick's suggestion of vertex crease information add another scalar array of the count of vertices -- although in practice I have rarely seen this used, rather I have tended to use a heuristics to derive vertex creases from edge creases.

There may be other ways to do a fully correct solution, my specification may not be optimal, but the currently proposed specification is inadequate for a formal standard which values correctness.

(4)
But this is not a standard proposal, rather just a vendor solution, so go with this extension and get it done.

(5)
If Khronos is adding support to the blender glTF extension for polygons based on this proposal on the assumption that this is the correct way to do it and it should become a defacto standard, I fear we are jumping the gun, and going with an inadequate solution out of expediency.

@emackey
Copy link
Member

emackey commented Aug 26, 2019

Khronos itself isn't adding support, this is an external contribution PR to the open-source importer. It hasn't been merged yet, and I'm concerned that merging it implies more official support than perhaps we would all like at this early stage.

@bhouston
Copy link
Contributor

Khronos itself isn't adding support, this is an external contribution PR to the open-source importer. It hasn't been merged yet, and I'm concerned that merging it implies more official support than perhaps we would all like at this early stage.

I'm not looking to jam up or act as a spoiler for Facebook's efforts. Thus maybe just mark it as Facebook Convex polygons or something so it is there but clear that it isn't proper subdivision.

@zellski
Copy link
Contributor Author

zellski commented Aug 30, 2019

To reiterate with further clarity: we (FB) consider this extension worthwhile mostly as a formalisation of our own internal client/server asset markup. These models are not intended to leak into the world, but nor do we try to hide them. The PR was our attempt to offer websearch-discoverable documentation for if/when people do come across it. I think the esteemed Mr. Mackey is correct that merging this risks undermining a future extension, and quite generally confounding. If not merged here, I will add it to the FBX2glTF repository.

@Peach1
Copy link

Peach1 commented Oct 17, 2019

Is there any progress on accepting this Quad GTLF extension?

  • Adds no extra filesize
  • GLTF viewers display the mesh correctly even without implementing the extension
  • This is already implemented in Blender, which is a significant step in advancing glTF authored assets in general

What's preventing this quad support from becoming an accepted extension exactly?


Quads for a 3D runtime format has a LOT of use cases like runtime adaptive subdivision level of detail, and topology analysis by showing the wireframe of a quad mesh.

Sites like sketchfab show wireframe previews on quad meshes, but when you download as GLTF, there is no way to preserve the topology of the model on reupload.

.obj is still used heavily over GLTF because it's one of the few free 3D formats that support quads.

Modern 3D modeling workflows are extremely quad oriented for topology and subdivision. Projects like OpenSubdiv from Pixar literally cannot use GLTF because this format doesn't support quads yet. For the purpose of wider 3D application adoption, GLTF really needs to support quads.

@zellski
Copy link
Contributor Author

zellski commented Oct 17, 2019

What's preventing this quad support from becoming an accepted extension exactly?

Yeah, at this point I'd propose the PR be merged as-is, a vendor extension. Its limitations are spelled out clearly enough that it shouldn't damage the adoption of a more sophisticated EXT_ version down the line, but that could be a while – and meanwhile, this does provide glTF with quad support.

@Peach1 Peach1 mentioned this pull request Oct 17, 2019
@bhouston
Copy link
Contributor

bhouston commented Oct 17, 2019

I advocate it stay as a vendor extension and we merge it as is.

It is clear it does not meet the needs for DCC-to-DCC loss-less quad/polygon transfer (as it doesn't support multi-topologies) like USD does, nor does it meet the needs for fast/easy GPU subdivision surfaces (as it doesn't store vertex or edge creases, nor guarantee quads, nor provide a limit surface fallback) as Alexey's proposed Quad SubD extension does.

This extension is sort of in an in-between space - quads by themselves.

Here is a presentation that puts this extension - the last slide - into context with Alexey's Quad SubD extension and how traditional raytracers utilize SubD on multi-topology meshes:
https://docs.google.com/presentation/d/1-55EI56KJQvw-oeGcZUpkz6Qd4NhZgXwJqV4u0SnBzU/edit

@zellski
Copy link
Contributor Author

zellski commented Oct 17, 2019

I hadn't understood about corner/crease sharpness before. We could also skip this extension entirely, if we have something else more or less ready to go. I just worry that we're all limited on bandwidth and we don't want this to sit idle for a year.

Copy link
Contributor

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me – if there are no major issues with this as a vendor extension let's go ahead and merge it. It's useful to Facebook as-is, and that's the main requirement for a vendor extension, aside from sufficient spec clarity.

However, and this does not block merging this PR – we don't have answers to @bhouston's comments above yet. I'm not convinced that lossless multi-topology DCC-to-DCC transfer should be a priority for glTF, but I do think GPU SubD would be necessary for an official glTF extension, and we do not have something ready that satisfies those requirements yet. So I'd unfortunately need to vote against merging an implementation of the vendor extension in common tools (like KhronosGroup/glTF-Blender-IO#622) until we have an official extension, which will take more time.

@donmccurdy
Copy link
Contributor

@zellski if you feel this is ready to merge (as a vendor extension for now) could you:

... I'd unfortunately need to vote against merging an implementation of the vendor extension in common tools ...

After some discussion, I feel less strongly about this. Perhaps support could be merged to Blender as-is, disabled by default, and with some warning that it may eventually be replaced by an official extension. Thoughts?

@Leadwerks
Copy link

We're actually using quads a lot because they work better with tessellation. A lot of our mechanical modeling is done with quad meshes.

@bhouston
Copy link
Contributor

bhouston commented Jun 8, 2022 via email

@stephomi
Copy link

stephomi commented Jun 8, 2022

you need to share UVs and normals and positions but differentially across faces

You can merge duplicated vertex at import.
ZBrush does that automatically at import with OBJ.
Blender can do it for glTF as well (checkbox in import menu).
No need for separate face sets then.
You cannot specify per edge crease/edge but at the same time many run-time softwares won't bother with these (typically unused in sculpting workflow, even though subdivision is used a lot).

My concern with the whole thread is the emphasis on (general) subdivision/tesselation topic even though the main extension is just about keeping quad (what about model fidelity, for example displaying the correct wireframe in a web marketplace, that particular point was even raised by @clems71). Or simply keeping the original topology for whatever any purposes that wouldn't be 100% rendering (animation, modeling).

Of course I'm biased (like everyone) because I'd like glTF to be a (slightly) more on the exchange side than a pure transmission format.
I would definitely understand the concern if the proposal was introducing a new types of indices, considering the current proposal is 100% non-invasive..

@Leadwerks
Copy link

The issue is this ngon extension does not support proper subdivision surfaces because in subdivision surfaces you need to share UVs and normals and positions but differentially across faces. This allows for quads but only if both normals, UVs and positions are shared across each edge. In my opinion, this extension is of limited use because of that limitation and incompatibility with general subdivision surfaces.
-- Ben Houston, CTO https://threekit.com 613-762-4113 / http://calendly.com/benhouston3d

We have our own subD system, and none of those issues affect us. I just need to be able to get the original quad primitives out of Blender.

@Leadwerks
Copy link

I would even be happy if the presence of the extension simply indicates "every two triangles form a quad in this indice array". The indice order doesn't even matter, since you can infer that as long as both triangles are using only four indices total.

@Leadwerks
Copy link

Leadwerks commented Jun 13, 2022

For each pair of triangles, there will be four different indices total. Two will be shared, two will be unshared. Find the unshared indice in the first triangle and go backwards one, wind through the first triangle from there, then add the second unshared indice. Dead simple and requires no extra information or special data layouts.

@stephomi
Copy link

stephomi commented Jun 13, 2022

For each pair of triangles, there will be four different indices total. Two will be shared, two will be unshared. Find the unshared indice in the first triangle and go backwards one, wind through the first triangle from there, then add the second unshared indice. Dead simple and requires no extra information or special data layouts.

Hmm did you read the PR proposition, anything wrong with it?
No one complained on this aspect of the extension (no special data layout either, supports ngon and quads, and no ambiguity), the concern were elsewhere.

@fire
Copy link

fire commented Jun 13, 2022

I am in support of this.

From the tooling side, can someone restore the blender addon?

Edited:

For Blender 3.2 (new!).

@Leadwerks
Copy link

Leadwerks commented Jun 13, 2022

Hmm did you read the PR proposition, anything wrong with it?
No one complained on this aspect of the extension (no special data layout either, supports ngon and quads, and no ambiguity), the concern were elsewhere.

Apparently there is, if it hasn't been finished after three years. I'm just pointing out, if you just support quads you don't even need the triangle indices to appear in any particular order.

@fire
Copy link

fire commented Jun 19, 2022

What are the steps to upgrade this to a KHR_ngon_encoding?

Goals

  1. Do not support proper subdivision
  2. Required to share normals, UVs and positions across each edge
  3. Keep ngons across 3D DCC tooling.
  4. Blender addon support

@bhouston
Copy link
Contributor

bhouston commented Oct 11, 2022 via email

@javagl
Copy link
Contributor

javagl commented Oct 11, 2022

@bhouston I might be lacking some context here, but stumbled over this via my notifications. It sounds like the question is strongly related to the question that also arises when trying to convert an OBJ into a glTF. There may be an OBJ file like this

#   3     4
#   |\   /|
#   | \ / | 
#   1--2--5
#
# (NOTE: OBJ indices are 1-based...)

# Vertices
v 0 0 0
v 1 0 0
v 0 1 0
v 2 1 0
v 2 0 0

# Texture coordinates
vt 0 0
vt 1 0
vt 0 1

# Faces: Triangles, where each vertex is
# given via 'vertexIndex/texCoordIndex'
f 1/1 2/2 3/3
f 2/1 4/3 5/2

And the problem is that this can not be represented as a (single-indexed, renderable) glTF asset, because the vertex with index 2 (in the middle) is used in two faces. But in one face, it is associated with texture coordinate 2 (=(1,0)), and in the other one, it is used with texture coordinate 1 (=(0,0)).

So the vertex in the middle has to be duplicated.

In terms of (algorithmic) complexity: The case has to be detected somehow, so one has to iterate over all vertices at least once.

In terms of 'what is actually done there': One question is whther this operation only considers the indices, or whether it should consider the values. Referring to the example above: The vertex once has the texture coordinate with index 2, and once it has texture coordinate with index 1. But one could detect whether these texture coordinates are actually equal, and not do the duplication in this case.

When operating solely on the indices, it's a bit simpler, and fairly straightforward to implement in O(numVertices) time and space. (I did this in a library that is - among other things - used for converting OBJ to glTF, with a function called makePropertiesUnique, that is once called for the texture coordinates and once called for the normals).

@emackey
Copy link
Member

emackey commented Oct 11, 2022

The case has to be detected somehow, so one has to iterate over all vertices at least once.

It sounds like this would break a key feature of glTF, that one can load blocks of vertex attribute binary data directly onto the GPU without using the CPU to iterate over the incoming vertices.

@javagl
Copy link
Contributor

javagl commented Oct 11, 2022

If I understood the core issue of "having to duplicate vertices" correctly (at least, on a conceptual level), then this would indeed break the feature of glTF being "ready to use for the GPU". If there is an index set, then the same index set has to be used for all attributes (i.e. positions, texture coordinates, and normals). If even one vertex has to be duplicated, then the whole chunk of "index data" becomes "invalid", so to speak, and has to be replaced with index data that that covers n+1 vertices (even if that single offending vertex could be detected in O(1) time - so this might be an issue, regardless of the algorithmic complexity).

But again the disclaimer: I might be missing some context here. I did not read all of the comments here (yet), and I might be misinterpreting what the last few comments have been about.

@stephomi
Copy link

ZBrush does that automatically at import with OBJ

Oops: I meant STL, not OBJ (OBJ is explicit about the topology)

It sounds like the question is strongly related to the question that also arises when trying to convert an OBJ into a glTF

Rather that's the opposite problem. OBJ -> glTF is trivial, you simply duplicate necessary UVs/Positions/Normals just as you said.
But for correct topology (subdivision or glTF -> OBJ) you need to detect duplicates by hashing values and merge them.

Potentially doing it with Positions, Normals and UVs (I'd say that hashing UV's is not really necessary but it doesn't matter much).
Hash is still O(1)

Maybe we should add that to the spec as how to handle quads with differing uv and normal topology? Then this fully addresses that concern.

Unfortunately it's not foolproof, the problem is that you can merge values that aren't meant to.
For example, this obj.

# Vertices
v 0 0 0
v 1 0 0
v 1 1 0
v 0 1 0
v 1 0 0
v 2 0 0
v 2 1 0
v 1 1 0

# UVs
vt 0 0
vt 0.5 0
vt 0.5 1
vt 0 1
vt 0.5 0
vt 1 0
vt 1 1
vt 0.5 1

# Faces
f 1/1 2/2 3/3 4/4
f 5/5 6/6 7/7 8/8

If you subdivide it, you should end up with 2 disjoints quads.
But if it's a glTF, there is no way to know the positional edge should't be merged.
And you will end up with 1 rectangle and only 6 vertices.

A robust subdivision extension would probably need to have extra arrays for the remapping.

It sounds like this would break a key feature of glTF, that one can load blocks of vertex attribute binary data directly onto the GPU without using the CPU to iterate over the incoming vertices.

To me sparse array or compression extension seems to break it as well.
But yes, it's more expensive (hash map is still O(1) though).


The easy part is getting the quad from the triangles, getting the topology right with glTF is the headache part.
Personally I'm fine with an extension focused on the tris/quad conversion.
Eventually with a reminder that duplicate positions are best to be merged if one needs to use subdivision.

But even with triangles only, the glTF topology issue already exists from a DCC pov, that's why Blender has a checkbox merge vertices.

Capture d’écran 2022-10-11 à 18 44 18

@donmccurdy
Copy link
Contributor

Rather that's the opposite problem. OBJ -> glTF is trivial, you simply duplicate necessary UVs/Positions/Normals just as you said... Hash is still O(1)

We have implemented this in three.js, and as you said it is not complicated. However, there's a big difference between just uploading the buffer to the GPU, vs. building a hashmap and doing a de-duplication pass over the vertices... O(1) may sound "free" but it is a pretty big cost for large geometry, and incurs garbage collection overhead. When using glTF for transmission to a client runtime, this is a cost we'd much rather not pay. For comparison, vertex compression like meshopt decodes at something like 2 GB/s in fixed memory, in addition to saving a lot of time on network transmission.

I suspect a lot of DCCs will turn this on by default (their goal is often to be lossless, not to export optimized data), so to the extent that we can find a solution here that doesn't dramatically reduce the loading efficiency of glTF models in the ecosystem, that would be my preference.

A separate consideration is that any sidecar arrays used for topology may also need to be reordered for compression purposes... This is compatible with Meshopt compression, but with Draco compression I'm not sure.

@stephomi
Copy link

that doesn't dramatically reduce the loading efficiency of glTF models in the ecosystem

If it's a runtime ecosystem (and not a DCC one), surely most app won't recompute the topology.

But I'm not in favor of adding the whole vertex merge logic into the spec/extension.
I mentioned it because it can make tris/ngon extension viable for subdivision even with uvs in many cases.


I see 3 different things:

  1. tris->ngon (can be used to display wireframe, subdivison (after vertex merging if UVs), etc)
  2. recompute exact topology (for dcc, but it needs extra array)
  3. runtime gpu tesselation (I've never done gpu tesselation, but I suppose the extension would have very specific data)

To me FB_ngon_encoding was only about 1.

@fire
Copy link

fire commented Oct 11, 2022

For me it would be amazing to degrade to 1 but serve the more important 2nd mission of dcc transfer.

@UltraEngine
Copy link

@zellski I'm about to finalize our own quads extension in our engine, but if FBX2GLTF supports this I will adopt your extension. I cannot find any mention of the term "FB_ngon_encoding" in the FBX2GLTF repository though. Is this actually in use?
https://github.com/facebookincubator/FBX2glTF/search?q=FB_ngon_encoding

@zellski
Copy link
Contributor Author

zellski commented Mar 18, 2023

@zellski I'm about to finalize our own quads extension in our engine, but if FBX2GLTF supports this I will adopt your extension. I cannot find any mention of the term "FB_ngon_encoding" in the FBX2GLTF repository though. Is this actually in use?

I'm afraid I don't know; I'm no longer working for Facebook and I haven't worked on the tool for several years. I suspect it's largely abandoned. There's been a fair bit of interest in the extension within the glTF community, but again I'm afraid I don't have a lot of up-to-date authoritative information.

Comment on lines +1 to +3
{
"$schema": "http://json-schema.org/draft-04/schema",
"title": "FB_ngon_encoding glTF extension",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update schema with new glTF schema version, add ID, and adjust title. The file should be renamed to mesh.primitive.FB_ngon_encoding.schema.json to clarify that this extends glTF mesh primitives.

Suggested change
{
"$schema": "http://json-schema.org/draft-04/schema",
"title": "FB_ngon_encoding glTF extension",
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "mesh.primitive.FB_ngon_encoding.schema.json",
"title": "FB_ngon_encoding glTF Mesh Primitive Extension",

Copy link

@fire fire Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaronfranke I recommend taking this under omi or godot namespaces and doing the updates.

There is a vrm blender addon that has ngon support.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet