RenderKit / ospray

An Open, Scalable, Portable, Ray Tracing Based Rendering Engine for High-Fidelity Visualization
http://ospray.org
Apache License 2.0
1.01k stars 182 forks source link

add convert function srgba to linear in example #433

Closed TWITTM closed 3 years ago

TWITTM commented 4 years ago

Ospray expects color to be in linear space, the ColorPicker passes the colors as srga. So this just converts the color spaces. I also changed the documentation so that it clarify that colors are expected in linear space.

See behavior before change : ColorPicker uses (50%,50%,50%) => gimp(on the right side) ( 74%,74%,74%) osp_ray_background

johguenther commented 3 years ago

Right, color parameters of type float are in linear space. We may extend that in future to accept also uint32 colors (which then could be in sRGBA or in linear RGBA space). The question is, what color space should the color picker widget represent. We can probably agree that the color displayed in the color picker for the background color should then also match the color of the background. However, this can also be achieved by displaying the color widget in sRGB space (instead of converting the color when passing to OSPRay).

TWITTM commented 3 years ago

The main motivation, of this pull request is that beginners are often not aware about color spaces. So hiding the conversation in the color-picker, might not solve this concern. However if you plan to introduce another method to accept sRGA values, you can close this request. You might think about changing the documentation to clearify that parameters are expected in linear space. Which is a huge trap, and not easy to solve, when someone already stepped in.

johguenther commented 3 years ago

Until we have a clear idea how to support setting sRGB color on OSPRay side (which may involve merging FB color channel and texture format enums into OSPDataType) I like to focus changes to the app side (i.e. ospExamples). There is already some discussion of sRGB and linear color space in the documentation (in the textures and the material section) and because color parameters are in float (vec3f) I'd hoped it is clear that this is in linear space.

Nevertheless, I agree that if the color checker displays "byte" values (integers in 0-255) the expectation by users is that those values should be consistent to e.g. Gimp's color checker, which is similar to sRGB textures values. Which means color from the color checker should be converted to linear. See also https://github.com/ocornut/imgui/issues/578.

TWITTM commented 3 years ago

"float (vec3f) I'd hoped it is clear that this is in linear space."

As many formats define the option for per vertex colors, which are also often defined as floats, but in many cases has to be interpreted as sRGB (for example GLTF2 ).

I already had a hard time to figure out in which file-fomat which color format is defined. But even this knowledge didn't help, as the most common viewers do interpret this formats wrong. And so the user expectation did not fit.

In my opinion you, changing the documentation would be a first step.

johguenther commented 3 years ago

I changed documentation based on your suggestion and added an sRGB conversion in ospExamples to arrive at

ospExample_sRGB

Maybe better handling can be achieved once https://github.com/ocornut/imgui/pull/2943 is merged.

johguenther commented 3 years ago

see 4d32a46f1fbb00fb2043610b1c545562d0e0ed61

TWITTM commented 3 years ago

@johguenther Perfect, thanx.