CesiumGS / cesium-native

Apache License 2.0
440 stars 215 forks source link

Reconsider division of glTF and 3D Tiles functionality into libraries #760

Open kring opened 1 year ago

kring commented 1 year ago

cesium-native jumps through some hoops in order to minimize the dependencies of its glTF and 3D Tiles support. Specifically, it divides up the functionality into a whole bunch of different libraries. Let's look at glTF. The glTF support in cesium-native is divided into four different libraries:

  1. CesiumGltf: This is meant to have nearly zero dependencies on anything, so it can be used independently in almost any project. Perhaps even as an alternative to tinygltf. In practice, it depends on CesiumUtility and GSL, and the dependencies of CesiumUtility have been growing over time (as they often do) and now include glm, uriparser, spdlog, and zlibstatic. 😬
  2. CesiumGltfReader: Adds the ability read glTF from JSON and GLB. Also knows how to "process" glTFs in various ways at load time, including image (JPEG, PNG, WebP) decoding, Draco decoding, KTX transcoding, Base64 (data URI) decoding, and meshopt decoding. For this reason it depends on a bunch of third-party libraries that implement these things. From cesium-native, it also depends on CesiumGltf (of course), CesiumJsonReader, and CesiumAsync.
  3. CesiumGltfWriter: The writing side of CesiumGltfReader. It doesn't support encoding all the things that the reader can decode, so it has fewer dependencies. Just CesiumGltf, CesiumJsonWriter, and modp_64 for data URI base64 encoding.
  4. CesiumGltfContent: This is a new one that is not in main yet, and I don't love the name. But this is the first library that depends on cesium-native proper, most notably CesiumGeometry and CesiumGeospatial. If we want to compute a bounding region for glTF, or compute texture coordinates for raster overlay draping, we need those libraries. Basically the idea is that this library contains the higher-level glTF manipulation functionality; anything outside representing the glTF in memory (CesiumGltf) or reading and writing it (CesiumGltfReader / CesiumGltfWriter).

This basic division into four libraries exists for 3D Tiles, too.

So this is a lot of complexity. Is there a better alternative? Does anyone actually care that CesiumGltf is usable without most of the rest of cesium-native? Is it at all useful to have a library like CesiumGltfReader that minimizes cesium-native dependencies, while basically being no-holds-barred on third-party dependencies?

Some possible alternatives:

Two libraries

  1. CesiumGltfObjectModel (better name idea welcome): Contains everything in CesiumGltf plus the parts of CesiumGltfReader and CesiumGltfWriter that do not require third-party dependencies. So this library knows how to read a GLB, but it doesn't know how to do Draco decoding. Depends on almost nothing in cesium-native (in fact we should try to eliminate the CesiumUtility dependency). Doesn't do anything asynchronous because it doesn't depend on CesiumAsync.
  2. CesiumGltf: Contains everything else. Knows how to interpret / decode / manipulate glTFs. Can and will depend on just about anything in cesium-native.

One library

Just put all the glTF functionality into a single library that depends on whatever third-party and cesium-native libraries are needed. Easy. But then using any part of our glTF support will require using most of cesium-native.

I think the critical question here is whether a low-dependency CesiumGltf is valuable to anyone. If not, combining everything into one library feels like a pretty reasonable approach.

@javagl I'm interested in your opinion on this, including possibly from a Khronos perspective.

timoore commented 1 year ago

Cesium-native is a good runtime glTF loader, or it would be if #610 were ever merged :smile: If you want to replace e.g. Assimp with a glTF-focused loader, then you really need the 3rd-party libraries like draco to be useful; at that point, it doesn't matter if some extra cesium-native code is coming along for the ride.

I would vote for putting all the glTF reading / writing code in a single library. I presume that the 3D Tiles functionality would be kept separate? How about putting all that in one library too?

csciguy8 commented 1 year ago

I also vote for the One Library, from just a maintenance perspective.

Let's not put effort into an organizational structure that has no immediate need.

javagl commented 1 year ago

Disclaimers:

  1. I'm trying to keep my role as an 'Independent Khronos Member' and my work for Cesium separate. In Khronos calls, I never speak ~"on behalf of Cesium". And here, I will not speak ~"on behalf of Khronos". (I occasionally mentioned e.g. that something like cesium-native/CesiumGltf exists, but without "endorsing" it in any way).

  2. I've not been actively involved in cesium-native for a while now. So I might not be up to date in every way (but I tried to keep track of some things...)

  3. I'll try to keep comments about ~"the usual trade-offs" short... 🤞


There could be alternative forms of the 'Two Libraries' approach. I agree that one could merge the CesiumGltf/Reader/Writer library into one. But I don't necessarily agree with the details:

CesiumGltfObjectModel [...] knows how to read a GLB, but it doesn't know how to do Draco decoding

When people want to load a glTF/GLB (file ;-) ), then they'd usually like to just do a target_link_libraries(example Cesium::CesiumGltf) in their CMake file and be done. They don't want to see an "ERROR: There's Draco, and I don't know what to wo with that" when they try to load that file. They don't want to manually twiddle with additional dependencies. And they certainly don't want to re-implement something like decodeDraco.cpp.

On the other hand, dependency handling is a source of headaches. Git submodules, version incompatibilities, compiler differences, compile times... And for glTF, there will be Draco, Meshopt, WebP, KTX, and potentially more in the future.

So I think that a clean separation of the "core glTF code" and the "convenient glTF library" depends on a clean and good mechanism for handling extensions .

This might involve something like a "plugin concept" for extension support. Roughly: Someone can do target_link_libraries(example Cesium::CesiumGltf) and then load a glTF file without extensions.

For everything beyond that, there could be something like target_link_libraries(example Cesium::CesiumGltfKtx) (Depends on KTX-Software) and on the code side:

#include <CesiumGltfReader/GltfReader.h>
#include <CesiumGltfKtx/GltfKtx.h>
...
CesiumGltfReader::GltfReader gltfReader;
gltfReader.registerExtension(...CesiumGltfKtx...); // Now it can read glTF with KTX
....

Whether or not this is achievable with reasonable effort? I don't know.... (But it could be 'nice'/'good' in many ways...)


in fact we should try to eliminate the CesiumUtility dependency

+1 for that. (Skipping some details about libraries that contain ~"stuff of which I don't know where else to put it" - it is difficult to sort that out).

From a quick code search, CesiumGltf depends on three things from CesiumUtility:

#include <CesiumUtility/ExtensibleObject.h>
#include <CesiumUtility/JsonValue.h>
#include <CesiumUtility/SpanHelper.h>

The SpanHelper is 1 or 2 functions with 1 line of code, and could trivially be inlined. The ExtensibleObject is... due to the overlap with 3D-Tiles. The JsonValue appears only (?) in the classes that deal with glTF metadata extensions.

The latter is another example why it could be important to handle extensions with a "plugin-like" concept. This is strongly related to this image of some Cesium3DTiles and CesiumGltf classes that I posted earlier somewhere:

cesium_metadata_classes

I'm convinced that there should be a Cesium3DMetadata library. Then, Cesium3DTiles could depend on this. And some CesiumGltfMetadata ("extension-plugin") library could also depend on this.


When it comes to the "role" of the CesiumGltf library, then this may also be relevant: https://github.com/CesiumGS/cesium-native/issues/221

(So... there might be a CesiumImage library that offers KTX support, and the CesiumGltfKtx extension plugin library then depends on the CesiumImage library, but other dependency structures might be possible, depending on "what belongs together"...)


CesiumGltf: Contains everything else.

Given the thoughts from above, and the point about

CesiumGltfContent: [...] to compute a bounding region for glTF, or compute texture coordinates for raster overlay draping,

Then I'd say that these are indeed things that are not related to glTF itself, but to the way how glTF is used within Cesium. So iff there was a Cesium glTF library (that supports Draco, KTX, Metadata and all the other "glTF-inherent stuff", maybe via plugins), then there could still be some CesiumGeospatialGltf library that contains these things that are not inherent to glTF itself, but specific for the context of geospatial applications.

kring commented 1 year ago

@timoore,

If you want to replace e.g. Assimp with a glTF-focused loader, then you really need the 3rd-party libraries like draco to be useful; at that point, it doesn't matter if some extra cesium-native code is coming along for the ride.

This is a really good point, Tim. Certainly right now, the cesium-native dependencies are much lighter than the various third-party libraries involved in "complete" glTF loading. And almost everyone that wants to load a glTF will also want to do all those various decodings that are necessary to understand what was loaded (in all but trivial cases), anyway. There might be exceptions, but they're rare, so why should we optimize for it?

@javagl,

I'm trying to keep my role as an 'Independent Khronos Member' and my work for Cesium separate.

I definitely understand and appreciate that. Still, from your Khronos discussions, you likely have some insight into how much interest there in something like CesiumGltf, and what people's expectations are for it. So, to the extent you can share, I'm really interested in that.

They don't want to see an "ERROR: There's Draco, and I don't know what to wo with that" when they try to load that file. They don't want to manually twiddle with additional dependencies. And they certainly don't want to re-implement something like decodeDraco.cpp.

They wouldn't see an error. But they would see a representation of the extension as it truly exists in the glTF file, without any built-in way to, say, decode the Draco buffer into vertices. If they need that, they would need to take another dependency.

But I think you're essentially making the same point Tim did. Pretty much no one wants to load a glTF without also wanting to decode Draco (for example).

So I think that a clean separation of the "core glTF code" and the "convenient glTF library" depends on a clean and good mechanism for handling extensions .

This is very doable, and I find it quite appealing. But I think what it boils down to is having even more glTF related libraries (such as your CesiumGltfKtx example). And then users that want a fully-functionaly glTF loader that supports all the things would need to either:

1) add all the dependencies and write the bit of code to register all the extensions, or 2) instead take a dependency on a higher-level library that pulls everything together.

And I think the point that both you and Tim made (and I largely agree) is that pretty much everyone will choose (2) when given the option. And in that case, why do the extra work of maintaining all these fine-grained libraries anyway?

Regardless, a pluggable glTF load/save pipeline is a really good idea, if for no other reason than so that it can be extended from outside cesium-native. The details are tricky. But yes, I think we should do this.

I'm convinced that there should be a Cesium3DMetadata library. Then, Cesium3DTiles could depend on this. And some CesiumGltfMetadata ("extension-plugin") library could also depend on this.

Yes. This would be much easier if the specs themselves referred to common JSON Schema.

Then I'd say that these are indeed things that are not related to glTF itself, but to the way how glTF is used within Cesium. So iff there was a Cesium glTF library (that supports Draco, KTX, Metadata and all the other "glTF-inherent stuff", maybe via plugins), then there could still be some CesiumGeospatialGltf library that contains these things that are not inherent to glTF itself, but specific for the context of geospatial applications.

Yeah true, that's a good point. The idea is that the Content library contains various ways to manipulate / modify / enhance a glTF. Not all of them are geospatial in nature. We could go finer-grained, of course. That's always my temptation and default move, honestly. But sometimes I'm reminded that that slippery slope eventually leads to a library per class. And what's the point of that, versus having all the functionality in a single library and the flexibility to #include the bits you need, and letting the linker sort it out? Like all slippery slope arguments, that obviously doesn't mean we shouldn't ever factor things out into separate libraries. We just need to be thoughtful about why we're doing it, rather than just doing it because these things are independent and so we can.

javagl commented 1 year ago

... how much interest there in something like CesiumGltf, and what people's expectations are for it.

I don't have specific insights into the demand for native/desktop glTF loader libraries. There has been some discussion on a high level, ~"which libraries do exist" or ~"how (and under which conditions) can certain forms of development of these libraries be supported", but no specific actions derived from that yet.

In terms of the expectations, I could try to summarize one takeaway message: glTF 2.0 is pretty much fixed and final. There will almost certainly not be a glTF 3.0 in the foreseeable future, and even a 2.1 seems to be unlikely. Basically all development of new features happens via extensions. This is why "handling extensions" is important on all levels (specifications, libraries, applications). Specifically: It is very likely that people will expect (all) ratified extensions to be supported. (That's where new development and maintenance effort for loader libraries will come from)


But I think what it boils down to is having even more glTF related libraries (such as your CesiumGltfKtx example) [...] [...] pretty much everyone will choose (2) when given the option. And in that case, why do the extra work of maintaining all these fine-grained libraries anyway?

This is one of the "usual trade-offs" points. And it's hard to strike the right balance here.

Two extremes:

Having a CesiumGltfTextureBasisu project that defines a KTX-reader plugin for the Cesium glTF loader would make a ton of sense, in terms of cleanly modeling dependencies, defining interfaces, managing build times, maintenance, etc.

Having a CesiumGltfMaterialsEmissiveStrength sub-project with directory and headers and CMake files that defines a "plugin" so that the Cesium glTF loader can read a single number would look bloated.

I think that rigurously saying "one project per extension, period!" would be nice and clean from a conceptual perspective (and avoid the burden to have to decide on a case-by-case basis). Given the potential overhead, one could argue for a middle ground, and, say, put all the ("simple") PBR extensions into a CesiumGltfRatifiedPbrExtensions project or so. But this should be thought trough in terms of evolvability and scalability.

In all cases, the question is still whether users will have to manually add/register these "extension projects", or whether there should be the complete (but bulky) off-the-shelf solution that contains everything by default.


This would be much easier if the specs themselves referred to common JSON Schema.

The JSON files are basically copy-and-pasted, to "make both specs 'standalone'".


We just need to be thoughtful about why we're doing it, rather than just doing it because these things are independent and so we can.

I couldn't say anything here that you're not aware of, and I certainly don't have experience with maintaining complex and evolving C++-projects over an extended period of time. But it's usually far easier to combine two things, than to nicely split a large library (that 'accidentally' covered 'unrelated' domains). And to start with the more 'fine-grained' approach tends to have positive effects in terms of the clarity of the API and the dependency handling.

dbs4261 commented 3 months ago

Hi, just want to chime in late and ask about splitting the libraries in a different way. Both Cesium3DTiles and CesiumGltf deal with metadata, semantics, properties and whatnot. Why not segment this off into a separate library that both can consume? There is a lot of really helpful utilities around accessors that currently can only be used for GLTFs and not on Tilesets or Subtrees. I think the most motivating example are the duplicate implementations of Buffer and BufferCesium.

javagl commented 3 months ago

@dbs4261 Aggreed - the duplication was also shown in the screenshot in my comment above, and I said

I'm convinced that there should be a Cesium3DMetadata library. Then, Cesium3DTiles could depend on this. And some CesiumGltfMetadata ("extension-plugin") library could also depend on this.

In some way, that's independent of the glTF library question (or at least, it's one chunk of work that could be carved out and addressed more or less independently).

kring commented 3 months ago

Yeah I agree too in principle @dbs4261. Getting there is a little tricky...