KhronosGroup / glTF-Sample-Assets

To store all models and other assets related to glTF
255 stars 30 forks source link

Playset scene containing lighting #116

Closed rsahlin closed 1 month ago

rsahlin commented 2 months ago

A 3D scene created by one of our 3D modellers, it contains a setup with a couple of IKEA items. The scene has lights and camera to create the desired output. We would prefer to be able to use environment based lights to emulate diffuse screens in a photoshoot, since this is not present in glTF we have added one directional light as a substitute.

echadwick-artist commented 2 months ago

Thanks for prepping this asset!

  1. The screenshots in the readme seem to be reversed, not matching their respective captions?
  2. The readme seems to be indicating the incorrect extension name for emissive, should be KHR_materials_emissive_strength.
  3. The scene seems to render too brightly in various common glTF renderers (Sample Viewer, Babylonja, Filament, Threejs). Is this intentional? Perhaps something about this in the readme might be helpful.
  4. I wonder if the copyright field in the glTF should match the Text in the metadata.json?
elalish commented 2 months ago

Thanks, this will be a very useful asset for demonstrating lighting units and exposure and such. Would you mind linking your preferred environment HDR here (even if it's not part of the PR)? Assuming it's under the same CC license? I can use that to set this asset up with a good comparison scene in https://github.com/KhronosGroup/glTF-Render-Fidelity once I finally get that up and running.

lexaknyazev commented 2 months ago

@rsahlin Please rename (via the "Edit" button in the top right corner) the PR title to make it more descriptive, e.g., by using the asset name.

rsahlin commented 2 months ago

Thanks for reviewing @echadwick-artist - Amazed of the minute details you are able to pick out!!! :-) Fixed 1, 2 and 4.

As per question 3: This is a physically correct model with one light using actual values from our products, plus one directional light. It is a manifestation of the areas that are not covered in the glTF specification: How 'connect' calculated values to display output - and lack of shutter, aperture and ISO in the camera :-)

rsahlin commented 2 months ago

Thanks @elalish

Would you mind linking your preferred environment HDR here Well - for us the purpose of this model is to show what it must look like when using the features that are part of the standard + Khronos extensions. For this model it means that we want it to be rendered using black background, ie no ibl, environment map or irradiance map.

rsahlin commented 2 months ago

@lexaknyazev - Done - yeah, that was a poor name :-)

elalish commented 2 months ago

I was referring to your comment:

We would prefer to be able to use environment based lights to emulate diffuse screens in a photoshoot

An IBL is a necessary part of PBR rendering, as all realistic scenes have some light coming from all directions. It's not included in glTF for the reasons you gave: glTF is a model, not a scene format, and there would be no way to combine different IBLs from models meant to be shown together in one scene. Additionally, the IBL has a significant bandwidth cost that you don't want to pay for over and over when it is reused for multiple assets.

I'll just use one of my own if I have to, but I'd be happy to use yours to look deeper into the lighting unit issues we've been discussing.

rsahlin commented 2 months ago

This asset is clearly a scene - and a great example of why we need an extension to provide environment information in a specified manner - we simply cannot continue referring to features and data that is not part of the standard.

Each glTF asset is actually a scene - with the possibility to specify multiple scenes if wanted. Combining environments is only a problem if they lack dimension. The bandwidth can easily be kept to a minimum using SH. All and more is solved here:

https://github.com/KhronosGroup/glTF/pull/1956

Once an environment extension is defined, default behavior can be specified in 3D commerce. It's not the place for 3D formats. glTF is simply a dataformat, an enabler - going into runtime behavior is a path we should avoid.

I'll just use one of my own if I have to, but I'd be happy to use yours to look deeper into the lighting unit issues we've been discussing.

Please don't use that for this model - the intention is to use standardized features of glTF. The issues we have been discussing are clearly evident in the model, as is.

echadwick-artist commented 1 month ago

It seems that "Directional" is mis-spelled?

Can you indicate which realtime renderer you used? Are there specific settings applied for camera exposure, tone mapping, etc.? Is the renderer available for other people to test against?

It might help to add a screenshot from the glTF Sample Viewer, to help explain the intentions of the asset lighting vs. the current state of glTF rendering in commonly available renderers.

rsahlin commented 1 month ago

Thanks @echadwick-artist! Updated the readme to better explain need for light scale factor. I am reluctant to add screenshot from sample-viewer since this would just look white....

echadwick-artist commented 1 month ago

Thanks for the edits, I think these help explain the intentions better.

Doesn’t make sense to describe this as a Showcase asset? Showcases are meant to demonstrate positive examples of what the tech can do. But this scene is specifically designed to expose a problem with how we treat lighting units.

rsahlin commented 1 month ago

Good thinking @echadwick-artist :-) I have changed the tag to test, updated folder and name.