-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: main
Are you sure you want to change the base?
Conversation
@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. |
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. |
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. |
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
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. |
Agreed. And this is an amazingly elegant solution to the question of quad/ngon storage.
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. |
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:
My proposal is also fully backwards compatible as well if the extra data is not supported. |
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:
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. |
@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.) |
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. |
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. |
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 |
And to be clear, I'm fine if the answer is "Yes, the mere presence of |
(1)
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) (3) 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) (5) |
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. |
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. |
Is there any progress on accepting this Quad GTLF extension?
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. |
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. |
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: |
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. |
There was a problem hiding this 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.
@zellski if you feel this is ready to merge (as a vendor extension for now) could you:
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? |
We're actually using quads a lot because they work better with tessellation. A lot of our mechanical modeling is done with quad meshes. |
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
On Wed, Jun 8, 2022 at 10:12 AM Leadwerks Software ***@***.***> wrote:
We're actually using quads a lot because they work better with
tessellation. A lot of our mechanical modeling is done with quad meshes.
—
Reply to this email directly, view it on GitHub
<#1620 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEPV7IJP4AHEGUN5CGDUF3VOCS3LANCNFSM4HQPBS6Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Ben Houston, CTO https://threekit.com
613-762-4113 / http://calendly.com/benhouston3d
|
You can merge duplicated vertex at import. 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. |
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. |
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. |
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? |
I am in support of this. From the tooling side, can someone restore the blender addon? Edited: For Blender 3.2 (new!). |
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. |
What are the steps to upgrade this to a KHR_ngon_encoding? Goals
|
That is incredibly helpful. So basically use the same vertex position but
have it twice but with different uvs and just merge them at load into a
properly index structure. Yeah that makes sense.
Okay. 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.
What is the O notation of such a merge? Is it feasible as a runtime load
operation? I suspect you want to hash the vertices into a hash table or
something like that
On Wed, Jun 8, 2022 at 6:50 PM Stéphane GINIER ***@***.***> wrote:
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 <https://github.com/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..
—
Reply to this email directly, view it on GitHub
<#1620 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEPV7IOCDULVZKAN7VA7DLVOEPTLANCNFSM4HQPBS6Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Ben Houston, CTO https://threekit.com
613-762-4113 / http://calendly.com/benhouston3d
|
@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
And the problem is that this can not be represented as a (single-indexed, renderable) glTF asset, because the vertex with index 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 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 |
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. |
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. |
Oops: I meant STL, not OBJ (OBJ is explicit about the topology)
Rather that's the opposite problem. OBJ -> glTF is trivial, you simply duplicate necessary UVs/Positions/Normals just as you said. 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).
Unfortunately it's not foolproof, the problem is that you can merge values that aren't meant to.
If you subdivide it, you should end up with 2 disjoints quads. A robust subdivision extension would probably need to have extra arrays for the remapping.
To me sparse array or compression extension seems to break it as well. The easy part is getting the quad from the triangles, getting the topology right with glTF is the headache part. But even with triangles only, the glTF topology issue already exists from a DCC pov, that's why Blender has a checkbox |
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. |
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 I see 3 different things:
To me |
For me it would be amazing to degrade to 1 but serve the more important 2nd mission of dcc transfer. |
@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. |
{ | ||
"$schema": "http://json-schema.org/draft-04/schema", | ||
"title": "FB_ngon_encoding glTF extension", |
There was a problem hiding this comment.
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.
{ | |
"$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", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.