SceneView / sceneform-android

Sceneform Maintained is an ARCore Android SDK with Google Filament as 3D engine. This is the continuation of the archived Sceneform
https://sceneview.github.io/sceneform-android/
Apache License 2.0
633 stars 146 forks source link

Incorrect colors in ViewRenderable #60

Closed grassydragon closed 3 years ago

grassydragon commented 3 years ago

Hello, @ThomasGorisse! When using the colors with the full pixel intensity, for example, the red #ff0000, green #00ff00 and blue #0000ff colors, they are displayed incorrectly on a ViewRenderable. This can be tested by changing the label background color here: https://github.com/ThomasGorisse/sceneform-android-sdk/blob/master/samples/gltf/src/main/res/drawable/rounded_bg.xml#L18 I think that this is caused by the inverseTonemapSRGB function here: https://github.com/ThomasGorisse/sceneform-android-sdk/blob/master/core/assets-sources/materials/sceneform_view_material.mat#L41 Should I ask about that in the Filament repository since I get the same results in the hello-triangle example?

ThomasGorisse commented 3 years ago

Hi @grassydragon,

You are totally right about it. Actually I put it in the TODO list since I was not completely sure about the ViewRenderable inverseTonemapSRGB original usage reason. I already removed it elsewhere (in the Renderable class from what I can remember) without issues (also from what I can remember). I actually try not to spend too much time on the Sceneform little bug fixes and focus more on the new framework publication date.

But it would be very helpful for anyone using Sceneform if you could ask the Filament team about the reason why inverseTonemapSRGB could have been used. Link the question to this issue and make a quick PR.

Thanks a lot for your great report !

grassydragon commented 3 years ago

As I understand the inverseTonemapSRGB does two things. It converts the color from the sRGB space to the linear space and applies the inverse transformation to cancel the tone-mapping effect. The problem can be solved by converting the color manually as described in the FIlament materials guide: https://google.github.io/filament/Materials.html#handlingcolors/linearcolors. I will ask about that because I am not sure about the performance impact.