google-ar / sceneform-android-sdk

Sceneform SDK for Android
https://developers.google.com/sceneform/develop/
Apache License 2.0
1.23k stars 604 forks source link

Gamma Problem - Darker than asset model #96

Closed peronecode closed 6 years ago

peronecode commented 6 years ago

Hello everyone.

In our tests, using OBJ assets, we noticed that the diffuse color texture is darker than it should be. The file is a PNG image with alpha channel, so, gamma is encoded. We doubled the texture's gamma using an image editing software and now the result looks correct. Then, I have two question for you:

1) What is the gamma space that Sceneform expects? (It seems to be trying to linearize the gamma embeded file by applying an inverse curve.)

2) Can we somehow chage how Sceneform inteprets image files?

I really appreciate your time for the answers. Best regards.

AdrianAtGoogle commented 6 years ago

Hi @Toione, thank you for posting this issue. Diffuse textures in Sceneform are interpreted as sRGB - it's possible what you're seeing is a side effect of issue #82, which is fixed in the upcoming 1.3 release. Normal maps and all auxiliary maps (e.g. roughness/metallic maps) are interpreted as linear.

peronecode commented 6 years ago

@AdrianAtGoogle Thanks for the answer, but that isn't what happen with my issue.

Let me explain...

Here is my Sketchfab model for this example.

When I convert the OBJ asset, with the diffuse texture PNG image set with 2.2 Gamma value, to SFB Sceneform binary file, this is what we have: sfb-bad

When I convert the OBJ asset, with the diffuse texture PNG image set with double Gamma value 4.4, to SFB Sceneform binary file, this is the output: sfb-good

So, my doubt persists to know if it's a bug or really is what Sceneform expects. In my mind, this issue is not related to saturated camera as the issue #82.

romainguy commented 6 years ago

Sceneform expects color maps in sRGB, they are converted to linear in the shaders. Could you share your .sfb so we can test it on our end and see what's going on?

peronecode commented 6 years ago

@romainguy Sure.

model.zip

romainguy commented 6 years ago

Thank you, I'll try in our renderer outside of Sceneform first to try and isolate the issue.

romainguy commented 6 years ago

Actually, do you mind giving me a .obj/.fbx/.whateverelse + textures so I can try outside of Sceneform? Thanks!

peronecode commented 6 years ago

@romainguy Here you are. Thanks for support my issue. :+1: model-obj.zip

romainguy commented 6 years ago

Here is what it looks like rendered directly in the Sceneform renderer (file loaded from disk directly, using SRGB8 for the base color map and RGB8 for metallic/roughness). It looks like an issue in the asset workflow toolchain with Sceneform.

@AdrianAtGoogle, could we have a material that's not setup correctly or we misinterpret the color map as data (linear)?

screen shot 2018-06-12 at 11 33 02 am
peronecode commented 6 years ago

[Off Topic] @romainguy Can I have this SFB viewer outside of the Sceneform plugin? I have another issue to take some shots of the SFB model inside the Sceneform.

romainguy commented 6 years ago

It's unfortunately not an SFB viewer, just .obj + base color texture + metallic/roughness texture. Happy to share it with you if that's useful though.

peronecode commented 6 years ago

Oh, that's OK, a viewer for this files types I already have.

There isn't a SFB viewer? I only have a Gradle plugin for IntelliJ to see how my SFB asset, but, on my Linux IntelliJ doesn't work. :cry:

So, thanks for helping :smile::+1:

romainguy commented 6 years ago

The IntelliJ plugin is the only SFB viewer we have. What doesn't work for you on Linux? IntelliJ itself or just the viewer?

peronecode commented 6 years ago

Just the viewer. Import assets feature works fine.

romainguy commented 6 years ago

Would you mind filing a separate bug for this issue? It should work on Linux.

peronecode commented 6 years ago

For sure, in few minutes I'll open an issue for Sceneform Plugin on Linux. Sorry for mix the subjects. :smile:

AdrianAtGoogle commented 6 years ago

Thank you for the clarification (and source assets!), @Toione. I was way off base pointing to #82 and have more information to share:

With a custom material sourcing the metallic-roughness texture, your asset looks like this in the 1.3 plugin: image

Beyond differences in the IBL probe, this seems to match your sketchfab link.

The darkening of the diffuse texture doesn't seem to be gamma related - in the 1.0 branch we're tracking your baseColor texture as sRGB and displaying it as such, however we're also multiplying the baseColor by the diffuse color from the .mtl file, Kd 0.58, 0.58, 0.58, and using the specular color Ks 0 0 0 to estimate values for metallic and roughness.

As an experiment, I built an sfa from your source asset and then edited the baseColorTint to be 1,1,1,1 and changed the metallic/roughness parameters to 0.0 and 0.5 (rough averages of the channels in the metallic-roughness texture). The result looks much closer to what you'd expect:

image

I hope this helps; When 1.3 is released I'll post the custom material to this issue

peronecode commented 6 years ago

@AdrianAtGoogle I be so grateful for your return about the this issue, it's so helpful, but, I need to delivery some assets for the company I work for. I need the next release with this issue fixed because our client works in the big retail business.

We just waiting for this release because this issue #96 and the issue #27 will be fixed.

Now the boring question, is there any release date for this next release?

I appreciate so much for your attention and help on my questions. Regards, Perone.

romainguy commented 6 years ago

To fix this issue you can change the baseColorTint in your .sfa file or change the diffuse color in the source asset to be white before importing.

peronecode commented 6 years ago

Ok, I get this, but when the issue will be fixed, I'll need to re-generate all the delivered SFB assets for my client and we're talking about hundreds of thousands delivered assets. :sweat_smile: Anyway, thanks for the answers.

malik-at-work commented 6 years ago

We are aiming for release date in about a week. This is not set in stone, many things could possibly come up to change release timing.

Terran-Marine commented 6 years ago

@malik-at-work I'm looking forward for the next version. Has it solved the problem of 'uv error'? By the way,I have a lot of models needed to be converted into 'sfb', do you know if there is any tools or scripts to help me to batch conversion? thanks 😄

peronecode commented 6 years ago

@ToadPrincess You can use my co-worker repository for this issue, but you need to create a batch. Repository

malik-at-work commented 6 years ago

@ToadPrincess if you are referring to https://github.com/google-ar/sceneform-android-sdk/issues/27 yes that will be fixed.

claywilkinson commented 6 years ago

I believe all 1.4 contains the fixes. Please re-open if there is still a problem.