wkjarosz / hdrview

A simple research-oriented image viewer with an emphasis on examining and comparing high-dynamic range (HDR) images, and including minimalistic editing and tonemapping capabilities.
Other
291 stars 11 forks source link

HDRView UI coming out bright gray on Windows #92

Closed ommaury closed 1 year ago

ommaury commented 1 year ago

Hi Wojciech,

In my windows environment, the UI comes out a bright gray, as if some gamma correction had been incorrectly applied to the UI. I've confirmed that this is specific to my environment, since others are seeing the regular darker gray. Do you have any hints as to what could be causing this? Some nanogui configuration maybe?

Thanks, Olivier

image

wkjarosz commented 1 year ago

Thanks for reporting this. Indeed, I have seen some other windows users with this issue, but I don't know the cause. I also suspect it's a nanogui thing. It's unlikely I'll be able to debug this myself, since I don't have a windows machine, but it's good to keep this issue open in case anyone has further insights. You might consider submitting a bug report to nanogui too.

pgrit commented 1 year ago

This is caused by the use of float16 frame buffers. The pixel values inside those are interpreted as linear RGB, but the shaders / nanogui are writing sRGB color values.

A workaround (for LDR monitors, of course) is to set float_buffer=false in the Screen class constructor.

wkjarosz commented 1 year ago

@pgrit thanks for the info. Can you clarify: are you suggesting this is something that needs to be changed ideally in nanogui, but that a workaround in hdrview could fix it for LDR monitors (and not HDR monitors)?

pgrit commented 1 year ago

Exactly. The workaround in hdrview would be to change the float_buffer parameter in this line to false https://github.com/wkjarosz/hdrview/blob/47da311b30f63724ddb4e39a97a4cff045df78d8/src/hdrviewscreen.cpp#L39 That creates an 8 bit LDR sRGB frame buffer, which works as desired. So it also fixes the problem on HDR monitors, but it no longer outputs an HDR image.

Another, more elaborate workaround would be to convert all color values passed to nanogui from sRGB to linear, for example the 40 here https://github.com/wkjarosz/hdrview/blob/47da311b30f63724ddb4e39a97a4cff045df78d8/src/hdrviewscreen.cpp#L49 And then also change the hdrimageview shader to output linear RGB https://github.com/wkjarosz/hdrview/blob/47da311b30f63724ddb4e39a97a4cff045df78d8/resources/hdrimageview_frag.gl#L50 That would add quite a few extra ifdefs, though. And will be hard to test if it works on all possible combinations of API, GPU vendor, and OS...

The correct fix should be in nanogui, which should ensure that framebuffers always operate in the same color space independent of the bit depth, API, and platform. If that's even possible...

wjakob commented 1 year ago

The last time I looked into this, HDR/EDR support on Windows was very experimental and not widely supported. I suggest doing the same thing as tev and disabling the feature based on the result of the nanobind test_10bit_edr_support() function (https://github.com/Tom94/tev/blob/master/src/main.cpp#L478).

wkjarosz commented 1 year ago

Thanks @wjakob for the tip. I've implemented it.

When someone (@ommaury?, @pgrit?) has a chance, could you please check whether the current develop branch fixes this issue on windows? You could just download the build artifact from this github action run.

pgrit commented 1 year ago

Thanks @wjakob for the tip. I've implemented it.

When someone (@ommaury?, @pgrit?) has a chance, could you please check whether the current develop branch fixes this issue on windows? You could just download the build artifact from this github action run.

Works on my system.

ommaury commented 1 year ago

Works for me as well :) Thanks @pgrit, @wkjarosz, @wjakob!