Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Added production asset: Carbon Frame Bike #357

Closed

Conversation

hybridherbst
Copy link

@hybridherbst hybridherbst commented Jul 9, 2022

Hi, I was able to get good licensing terms (CC-BY-SA) for an asset from one of our productions, so I'm happy being able to contribute it to the glTF Sample Models.

Link to PR readme: https://github.com/prefrontalcortex/glTF-Sample-Models/tree/carbon-frame-bike/2.0/CarbonFrameBike

I think it's interesting as sample model because it uses a number of glTF features heavily instead of just being a "feature test" for one in particular:

  • multiple skinned meshes
  • many animated objects
  • texture transforms
  • grows quite outside of rest bounds during animation (a challenge for many viewers)
  • challenging to compress due to a mix of small and large geometry features
  • animated shadow planes in the model (a challenge for some viewers)
  • there's a matching USD + USDZ model in the USD Working Group's new usd-wg/assets repository

An interactive version (including AR view) can be found here: https://prefrontalcortex.de/labs/model-viewer/upload/CarbonFrameBike/

20220617-carbonframebike.mp4

(At a later point I'd like to add meshopt compressed files since that can also compress the animation, but currently there seems to be a bug in gltfpack preventing that.)

@emackey
Copy link
Member

emackey commented Aug 26, 2022

Hi @hybridherbst, cool bike! I love the exploded view animation.

I think the glTF-Compressed folder doesn't match any existing structure. The existing structure of this repository isn't great, but I'm trying to prevent it splintering into even more pieces. The closest we have is the StainedGlassLamp which places one model in glTF-KTX-BasisU (that name was also the subject of debate here).

Can you either remove glTF-Compressed or replace it with a single best-case model following the glTF-KTX-BasisU pattern?

Also, depending on your toolchain there may not be an easy fix, but the validator warns that you have skinned meshes with transforms applied to them. Skinning is a complex topic and we've had an effort here to present "clean" skinning samples in this repo that don't show meaningless transforms applied in the wrong places. Even so, we have lots of examples of non-root skins in this repo, but putting transforms on the skinned node itself is really a further step in the wrong direction. If it's easy to clean these out, that would be appreciated. Thanks!

@hybridherbst
Copy link
Author

Hi @emackey, there's many other models that have glTF-Draco folders, but the interesting point here is that this is a model where I've

  • seen compression fail in various tools (due to thin skinned meshes, due to many differently sized textures, or just in general)
  • different compression algorithms give different qualitative and size-wise results in an interesting way
  • it's a good model to demonstrate how much impact good compression (of both meshes, textures and ultimately animations) can have.

Of course if you think that's better I can put these variations elsewhere, but I think it's interesting here for a wider audience.

Regarding the skinned meshes, personally I think it's strange that the validator warns for it. Its clear from the spec that their transform is to be ignored, there's no reason for it to be "zeroed" technically, and in reality the position of the transform does have an impact on how various tools calculate bounding boxes and other things (even when that is not in accordance with what the spec wants).

And another reason to keep them would be that this would be the point of sample models in my opinion - spec behaviour is clear, real-world files don't null those transforms, so having only models that null them in this repo is counterproductive for testing exact implementations :)

What I suggest instead is that I add a paragraph regarding the use of skinned meshes and why its done like this - would that work for you?

@emackey
Copy link
Member

emackey commented Aug 29, 2022

Good point, we should have models that test behavior for transformed skinned meshes. I'm fine with leaving those in.

I guess the one piece of structure that's really changed here is until now, we have only one example model per folder, and the model's name matches the sample's parent folder name. So for example, CarbonFrameBike.glb matches the folder name CarbonFrameBike, but does not match something like CarbonFrameBike.etc1s+draco.glb. I'm not sure what the best solution is here. Side-by-side comparisons are easy to do by file size alone, but etc1s is more about conserving GPU memory than file size, so it's harder to understand the benefit when placing different files next to each other. I think this repo should focus on showing a well-formed glTF sample per model, rather than compare and contrast all the compression permutations. Also I don't think we need the USDZ references in the readme, as this repo is focused only on the glTF versions of models. This repo is already well over 1GB in size, so the more focus we can bring, the better.

@hybridherbst
Copy link
Author

hybridherbst commented Nov 5, 2022

@emackey I went ahead and moved all compressed files into their respective subfolders and they all have the same name now. Personally I find that harder to work with / easier to be confused, but I think it is very valuable to have these compressed versions as part of the samples repo since it's hard to find actual comparisons as to why/why not someone should be using which compression.

@DRx3D
Copy link
Contributor

DRx3D commented May 11, 2023

@hybridherbst : We really like this model and was about to merge it when we noticed that there are marks on the bike that might be logos. The bike frame has 'RobertS' and the wheels have 'NEEDLE' and 'PREFRONTAL CORTEX'. If these are trademarks, usage marks, or otherwise considered "owned" property, then the license and/or model needs adjusting. Khronos has no objections to various marks being present. We want to make sure that those marks are suitable protected and ownership is established.

The options are:

  1. Remove the marks
  2. Include a separate statement indicating that those marks are owned by and permission is granted to use those marks in this 3D model without modification.

As the model is currently licensed, someone could take the and change the marks to something else. I suspect that is not desirable.

@DRx3D
Copy link
Contributor

DRx3D commented Dec 15, 2023

Closing issue so repo can be archived. An issue was create in Sample Assets referring to this PR.

@DRx3D DRx3D closed this Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants