google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.95k stars 820 forks source link

Re-introduce environment-intensity; it's too hard to change brightness without it #889

Closed hybridherbst closed 2 years ago

hybridherbst commented 4 years ago

Description

Models that previously looked good now look very dark and washed out. image Note that the environment image for the top image is already set to environment-intensity="4". From my testing, might it be that you replaced "environment-intensity" with "exposure"? Are these the same thing? They should be different, from the name of it.

(I would expect environment-intensity to control background brightness and the resulting IBL brightness; thus environment intensity shouldn't affect Unlit models while exposure should affect everything, as its a camera parameter).

Live Demo

https://glitch.com/edit/#!/bramble-geranium I noticed the issue on https://prefrontalcortex.de/projects/schreibkugel/ (scroll down).

Browser Affected

OS

Versions

latest

cdata commented 4 years ago

@soraryu thanks for filing this issue. Can you confirm which versions of <model-viewer> you noticed the regression across?

cc @elalish

hybridherbst commented 4 years ago

Just tested: works on 0.6.2 (nice and shiny) and breaks beginning with 0.7 (dark).

cdata commented 4 years ago

Ah yes, sorry, we removed environment-intensity in v0.7.0. We left comments about this in the release notes, including recommendations for migration: https://github.com/GoogleWebComponents/model-viewer/releases/tag/v0.7.0

Since exposure works similarly to the same term in photography, it is not equivalent to setting environment-intensity. Changing environment-intensity is roughly analogous to increasing the global brightness of the light, while changing exposure is roughly analogous to increasing the amount of time a camera shutter is open when exposing film to light.

Making exposure higher will make the scene brighter, but the rendered result will be different from how it looked when making environment-intensity higher.

elalish commented 4 years ago

Yes, and the first rendering is dark because it uses an LDR (jpg) environment. You can compensate with higher exposure, or by using an HDR environment image (like our default).

hybridherbst commented 4 years ago

Ah, thank you, I didn't see that or didn't realise what I was reading :). Just to confirm - this means that the example I outlined in my description (of changing background intensity vs. changing exposure in regards to Unlit models) is correct, and it's now more difficult to adjust background vs. model for Unlit models? (this is not necessarily bad, just want confirmation)

cdata commented 4 years ago

@soraryu I don't know if I understand exactly what you are asking, but I do think it is different now to do what you are trying to do, and subjectively more difficult for some cases.

You can set different environment-image and background-image assets, which should allow you to achieve the effect of changing the light brightness as it hits the model without changing the brightness of the skybox.

Changing the brightness of the environment-image (say, in Photoshop) is roughly the same as changing the environment-intensity.

hybridherbst commented 4 years ago

Yes, that makes it clear, we're talking about the same thing. I just had the case in mind where basically you're reusing an environment probe for multiple models across a website, and you want to adjust model brightness without affecting background brightness.

cdata commented 4 years ago

@soraryu Sounds good. Let me know if you still think there is an actionable issue here!

hybridherbst commented 4 years ago

I think you can close this issue; however I'd like to rephrase the last thing as a question to get your input: "How would I reuse an environment probe for multiple models across a website where I want to adjust model brightness without affecting environment picture brightness?"

cdata commented 4 years ago

@soraryu I think you raise a good point, which is that we probably end up being somewhat inefficient here. To accomplish what you are asking, I would do something like this:

<model-viewer src="foo.glb"
  background-image="skybox.hdr"
  environment-image="environment-1.hdr">
</model-viewer>
<model-viewer src="bar.glb"
  background-image="skybox.hdr"
  environment-image="environment-2.hdr">
</model-viewer>

In this scenario, we will be able to cache skybox.hdr for use across both models. It will keep a consistent brightness in the skybox, and you will have two different levels of brightness for the models. But, you have to generate the brightness levels you want in advance and represent them as different environment maps, which means we have to load and process N environment maps where N is the number of brightness levels you need (undermining cache).

This kind of seems like an argument in favor of allowing for environment-intensity (or else, something like it). It would allow us to cut down on network costs, script execution costs and memory overhead significantly in a scenario like this.

cc @elalish

hybridherbst commented 4 years ago

Yeah that's exactly why I thought this case here is an regression - because it makes that scenario way harder than before. Imagine a shop where shop owners only have a single slider exposed which is the brightness of IBL onto their 3d model, everything else is basically defined by the corporate identity of the shop.

elalish commented 4 years ago

Thanks for the info! We haven't seen a lot of use of the skybox in the wild (we usually see uses more like the images you shared here, with a solid color background); is there a chance you could share a link to the scenario you're talking about? But you're right, this is a problem we can solve.

elalish commented 2 years ago

I think we don't want two concepts of brightness in the scene. As it stands, our renderer functions like a real camera; once we add a way to change the relative brightness of the background vs the foreground, we break that physicality.