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

Remove <source> configuration API #5

Closed
smalls opened this issue Oct 18, 2018 · 9 comments
Closed

Remove <source> configuration API #5

smalls opened this issue Oct 18, 2018 · 9 comments
Assignees

Comments

@smalls
Copy link
Contributor

smalls commented Oct 18, 2018

It's pedantically correct but potential confusing - including requiring users to know/specify mime-types).

Instead, use a simpler, attribute-based approach. The main glTF resource can remain source, and we can have other optional attributes for source-usdz and potentially source-ml (or source-fbx).

@mikkoh
Copy link
Contributor

mikkoh commented Mar 13, 2019

Sorry to bring this up way late in the game.

I would argue that you could infer the model type by the file extension if type is not provided. type should be optional.

<model-viewer> has some parallels to <video>. For example it displays content from multiple sources.

eg for video

<video>
    <source src="some.webm" type="video/webm" />
    <source src="some.mp4" type="video/mp4" />
    This browser does not support the HTML5 video element.
</video>

<model-viewer> could look like this:

<model-viewer>
    <source src="some.usdz" />
    <source src="some.glb" />
    All kinds of bad things happened and we can't do a thing
</model-viewer>

Also the naming of src and ios-src precludes some level of importance src is more important than the secondary ios-src. Also if USDZ is supported in the future by THREE natively do we still want to call it ios-src.

Doing this would allow for the follow:

  1. Handle an ever increasing number of potential sources that could be rendered
  2. Is inline with how <video> and <picture> work
  3. There's no need to remember arbitrary attribute names eg src, ios-src, hologram-src??

@cdata
Copy link
Contributor

cdata commented Mar 13, 2019

@mikkoh thanks for that feedback!

Recently, our rationale has been that USDZ will be very difficult to support on the web (compared to glTF, which is a relatively constrained and well specified data structure).

That said, I think we might want to reconsider this approach for a few non-USDZ cases. For example, a user may have one or more of the following:

  1. A *.gltf canonical model
  2. A *.glb version (for e.g., request count optimization, Magic Leap support)
  3. A *.drc DRACO encoded version for further network optimization

And expect <model-viewer> to use the most appropriate source for any given feature.

Granted, this scenario is somewhat contrived. But, I think you are correct @mikkoh that we will ultimately contend with multiple meaningful notions of "source" (just as <video> and <picture> do), even if it has nothing to do with USDZ.

@donmccurdy
Copy link
Contributor

donmccurdy commented Mar 13, 2019

A couple notes:

  1. .glb is the most efficient glTF packing option in nearly all cases - I don't think you'd ever need both a .gltf and .glb to be specified.
  2. .drc is a geometry-only Draco file, unrelated to glTF, and it's not widely used. glTF files using Draco compression still use the normal .glb or .gltf extensions. If a client doesn't support Draco, we'd need some other indicator.

In general I don't think <model-viewer/> can or should try to support very many formats – this gives users the impression that the format doesn't matter, and that they should bring whatever they have and we'll deal with it. Unfortunately many common formats (COLLADA, FBX, OBJ) are very inefficient for use on the web, and/or unreliable to parse, and IMO we're setting our users up for a bad experience by heading in the direction of an ever increasing number of sources.

Nevertheless, the suggestion of src instead of *-src seems fine to me.

@smalls
Copy link
Contributor Author

smalls commented Mar 13, 2019

This makes sense to me, even if the only formats that are ever included are glTF and USDZ. I've filed #429 to track moving back to this, along with a short proposal that I believe will bring us (as suggested) more in line with <video>.

Thanks @mikkoh!

@cdata
Copy link
Contributor

cdata commented Mar 13, 2019

In general I don't think can or should try to support very many formats

I agree with this. To the extent that formats are not in some way related to glTF I don't think we should support them outside of fallback scenarios like USDZ on iOS.

.glb is the most efficient glTF packing option in nearly all cases - I don't think you'd ever need both a .gltf and .glb to be specified.

My rationale behind the *.gltf canonical model is that it may be useful to advertise an unbundled / uncompressed version for metadata purposes. But, I admit that I do not have a concrete use case in mind.

.drc is a geometry-only Draco file, unrelated to glTF, and it's not widely used. glTF files using Draco compression still use the normal .glb or .gltf extensions.

My mistake. The canonical Three.js example led me astray. Is it just mime-type that sets a DRACO-compressed glTF apart then? Or is it the presence of an extension? Some other signal?

@donmccurdy
Copy link
Contributor

A glTF file using the Draco extension (KHR_draco_mesh_compression) will have the extension listed in the asset's extensionsUsed array – the mimetype is the same regardless of which extensions are present. So, a client would need to read at least the .gltf portion of an unpacked asset, or the first 'chunk' of a .glb asset, to know what extensions are included. It's possible to create a file with optional Draco compressed data:

  • model.gltf
  • common.bin
  • compressed.bin
  • uncompressed.bin

... in this case both a client that supports Draco and a client that does not can still download only the data they actually need. However, I'm not sure it's reasonable to ask users to prepare their models this way, and it's still less efficient than having two separate .glb files.


Pulling on a different thread here: (1) it would be highly advantageous for Magic Leap to support Draco compression, and we should encourage them to do so, and (2) it's likely possible to load a Draco-compressed glTF file in <model-viewer/>, decompress it in memory, and then pass the result into Magic Leap as a Blob URL if necessary. I'd prefer (1) over (2). 😇


There are other extensions threejs may support that other renderers may not: KHR_texture_transform and KHR_materials_pbrSpecularGlossiness for example. Taking a bit of inspiration from sizes and srcset, consider:

<model-viewer>
    <source src="A.usdz" />
    <source src="B.glb"
            size="4mb"
            extensions="KHR_draco_mesh_compression, KHR_texture_transform"/>
    <source src="C.glb"
            size="30mb"
            extensions=""/>
    Oh noes.
</model-viewer>

@donmccurdy
Copy link
Contributor

Also note that some extensions, like KHR_materials_unlit, you might prefer only on a less capable device. Taking this example:

^The model shown here (no extensions) renders great on my laptop, but has a low FPS on my iPhone SE. By adding the KHR_materials_unlit extension I can improve the framerate significantly on mobile, but with the cost of some visual quality. In other cases, KHR_materials_unlit might be used for stylistic rather than performance reasons, and it would be preferred on any device that supports it.

^I may be overcomplicating the actual needs for <source/> configuration, if so please ignore me. 😇

@cdata
Copy link
Contributor

cdata commented Mar 14, 2019

@donmccurdy I think you are striking at a very real problem we will be approaching, which is that there are limits to how adaptive <model-viewer> can be on behalf of users unless they give us explicit signals, hints or configuration that we can act on in different scenarios. Thanks for all of the insight.

@cdata
Copy link
Contributor

cdata commented Mar 15, 2019

This discussion has been great, BTW, just want to suggest to folks that we move further discussion to the newly opened issue #429

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

No branches or pull requests

4 participants