GLSL-PathTracer icon indicating copy to clipboard operation
GLSL-PathTracer copied to clipboard

Tone mapping ruins the constant background color

Open LiangliangNan opened this issue 2 years ago • 28 comments

Hi, thanks for the great project!

In the shader code in tonemap.glsl, tonemap is applied to all pixels, which is not correct if a constant background color is desired. See the effects below:

Without tone mapping: Screen Shot 2021-12-29 at 12 20 30

With tone mapping: Screen Shot 2021-12-29 at 12 21 39

I think whether it is background can be recorded as the w component in pathTraceTexture. Maybe there are other better ideas?

LiangliangNan avatar Dec 29 '21 11:12 LiangliangNan

Hi,

The background color actually acts as a constant light source (which I was using as a furnace test) so perhaps it should be named differently to avoid confusion.

I think the standard way of doing this (that I know of) is to record transparency in a channel, as you mentioned, and then composite the rendered image over a white background or any other image.

Something like this:

untitled

glass

Right now, the renderer doesn't track alpha/transparency and it would require some modification to get it to work.

knightcrawler25 avatar Dec 29 '21 13:12 knightcrawler25

Thanks for the quick reply. Now I see. But it is indeed confusing and would be good to make it behave the same as other software.

LiangliangNan avatar Dec 29 '21 15:12 LiangliangNan

I added support for background color and transparency to the dev branch. There might be some bugs and transparency doesn't work with the denoiser as of now, but you should be able to set a background color or save out an image and composite it over another image in GIMP or photoshop.

White Background: img_113

Transparency in viewer: Transparent

Composited: test

knightcrawler25 avatar Dec 29 '21 19:12 knightcrawler25

Thanks for the efforts. Very much appreciated! The white color works now.

Below I summary the issues I have identified in the test and in reading the code.

  1. Low-reso rendering when the mouse moves. I found the "cornell_box.scene" is not properly rendered. It seems the low-resolution pass is not correctly rendered, or it may be due to the composition or that the previous content was not cleared. Here is a short recording: https://user-images.githubusercontent.com/15526536/147749593-40d0f1f4-cdf3-427b-a555-5aee230be5ed.mp4

  2. Switching scenes. When switching to a new scene, the original contents seem not cleared. This is really weird because the renderer has been rebuilt. Here is a snapshot (which can be reproduced by firstly zooming out the cornell_box scene and then switching to cornell_box2) Screen Shot 2021-12-30 at 13 00 30

  3. Other issues in the code.

  • in uniforms.glsl: uniform useEnvMap is never used and can be removed
  • in anyhit.glsl: LIGHTS -> OPT_LIGHTS
  • in pathtrace.glsl: OPENGL_NORMALMAP -> OTP_OPENGL_NORMALMAP
  • in Scene.cpp: missing "delete texture;" if texture creation failed, in function "int Scene::AddTexture(const std::string& filename)";
  • in Scene.cpp: "resizedTex" in function "void Scene::ProcessScene()" is allocated using "new[]" and thus should be freed using "delete []".

These things so far, hope it helps and looking forward to any update. Happy New Year!

LiangliangNan avatar Dec 30 '21 12:12 LiangliangNan

Thanks a lot for pointing these out. I have pushed a commit to the repo that has the changes for 3.

For 1 and 2 however, I'm unable to reproduce them on windows. Are you running this on Linux?

knightcrawler25 avatar Dec 30 '21 12:12 knightcrawler25

Oh, forgot to mention that. I am using an apple laptop:

  • maxOS 11.6
  • CLang 13.0.0
  • GPU: Intel(R) Iris(TM) Plus
  • OpenGL version: 4.1 INTEL-16.5.2
  • GLSL version: 4.10

To understand what goes wrong, I took snapshots for the four FBOs (every time when new contents have been written into). https://www.dropbox.com/s/td9bfe3ijm7ou39/fbo_snapshots.zip?dl=0 I was not able to get a clue, maybe easier for you.

LiangliangNan avatar Dec 30 '21 15:12 LiangliangNan

I've actually never tested the code on a mac as I don't have access to one. Unfortunately, I don't think it would be possible for me to debug this as I don't see the same issues on windows or linux. I've also tried running the code on some older hardware as well and can't replicate it.

The first issue looks rather odd, sort of like uninitialized data is being read by the shader. Do you see this issue across all scenes? Perhaps you can strip out much of the logic in pathtrace.glsl and just check if ClosestHit() is returning a proper hit

knightcrawler25 avatar Dec 30 '21 18:12 knightcrawler25

Now the 2nd issue has been fixed, with your latest commit and the following change:

  • in TiledRenderer::Render(), after drawing into the low reso FBO, the scene->dirty should be changed to false, i.e.,
    scene->instancesModified = false; becomes scene->instancesModified = false; scene->dirty = false;

The first issue happens only to the "cornell_box.scene", which is really weird. I couldn't find any potential cause in the shader code.

LiangliangNan avatar Jan 01 '22 13:01 LiangliangNan

Thanks, I added that change in.

It's odd that you see an issue in that scene but not in cornell_box2.scene. The second scene uses the same objects (Difference being cbox_smallbox.obj and slight difference in light emission, material colors and path depth).

knightcrawler25 avatar Jan 01 '22 13:01 knightcrawler25

Indeed!

Below are 3 consecutive snapshots of the trace FBO (the lower left part of the "cornell_box. scene"). And we can see that many hits are missed:

fbo_path_trace_012 fbo_path_trace_028 fbo_path_trace_044

LiangliangNan avatar Jan 01 '22 14:01 LiangliangNan

Is cornell_box.scene the first scene that gets loaded when you run the code? What if you switch to a different scene and then switch back to cornell_box.scene, does the issue still remain?

knightcrawler25 avatar Jan 01 '22 14:01 knightcrawler25

Then it is gone. But why?

LiangliangNan avatar Jan 01 '22 14:01 LiangliangNan

I'm guessing it could be one of these uniforms that is not being sent to the shader the first time around and when you reload the scene things might be getting initialized properly.

https://github.com/knightcrawler25/GLSL-PathTracer/blob/a6526c8bf7964b64724b0ddea9d31a5bb1b1bffc/src/core/TiledRenderer.cpp#L520-L560

knightcrawler25 avatar Jan 01 '22 14:01 knightcrawler25

I've checked those uniforms and the values are the same.

Look at the result. There are uniform regions with straight boundaries. I guess the issue might be due to the transformation of each mesh. Screen Shot 2022-01-01 at 16 29 36

LiangliangNan avatar Jan 01 '22 15:01 LiangliangNan

Not really sure where the issue is but could you try the latest code in the dev branch and see if that fixes anything? I've refactored some of the code.

knightcrawler25 avatar Jan 05 '22 20:01 knightcrawler25

Thanks for the effort. Just tried and it is still the same: Screen Shot 2022-01-05 at 23 41 50

Two files that do not compile:

  • Loader.cpp, line 483:
    printf("Unable to load gltf %s\n", filename); should be printf("Unable to load gltf %s\n", filename.c_str());
  • EnvironmentMap.cpp: missing "#include "

LiangliangNan avatar Jan 05 '22 22:01 LiangliangNan

One more issue. When I wrote in command line PathTracer -s c:\temp\glTF-Sample-Models\2.0\WaterBottle\glTF-Binary\WaterBottle.glb program fails. in Main.cpp from line 524 must be rewriten

if (!sceneFile.empty())
    {
        scene = new Scene();

        GetEnvMaps();
        LoadScene(sceneFile);
    }
    else
    {
        GetSceneFiles();
        GetEnvMaps();
        LoadScene(sceneFiles[sampleSceneIdx]);
    }

When is no EnvMap loaded program fails also. TiledRenderer.cpp lines 251-252 and 274-275 must be rewriten like this

       glUniform2f(glGetUniformLocation(shaderObject, "envMapRes"), scene->envMap == nullptr ? 0.0: (float)scene->envMap->width, scene->envMap == nullptr ? 0.0 :(float)scene->envMap->height);
        glUniform1f(glGetUniformLocation(shaderObject, "envMapTotalSum"), scene->envMap == nullptr ? 0.0 : scene->envMap->totalSum);

tigrazone avatar Jan 06 '22 07:01 tigrazone

I check tests from https://github.com/KhronosGroup/glTF-Sample-Models

glTF-Sample-Models\2.0\SpecularTest\ not passed. Specular parameter not changed image

glTF-Sample-Models\2.0\SpecGlossVsMetalRough\ not passed. Right bottle not texture mapped like left bottle image right result by https://github.com/SaschaWillems/Vulkan-glTF-PBR image

glTF-Sample-Models\2.0\AttenuationTest image where is blue? image

glTF-Sample-Models\2.0\TransmissionRoughnessTest image

where is blue? image

tigrazone avatar Jan 06 '22 07:01 tigrazone

@tigrazone : GLTF material support is not complete yet. Only metallic roughness workflow is used and transmissionFactor from the extensions: https://github.com/knightcrawler25/GLSL-PathTracer/blob/ddbba9a1f2bf1c9dec0f1c2634a1af3949e5f25a/src/loaders/GLTFLoader.cpp#L245-L279

For the last screenshot, you'll have to increase number of bounces to get the blue color

knightcrawler25 avatar Jan 06 '22 08:01 knightcrawler25

Agreed with blue in scenes - increase number of bounces helps to render blue colored boxes. But other is not solved yet

tigrazone avatar Jan 06 '22 08:01 tigrazone

@LiangliangNan : Could you also try the debug branch? It has just enough code to render cornell_box.scene and would be easier to check.

knightcrawler25 avatar Jan 06 '22 14:01 knightcrawler25

Now there are fewer "black" things, but still similar. See the recording below:

https://user-images.githubusercontent.com/15526536/148407642-7d879c37-f0b9-4d2e-baa0-1d764adb1bd0.mp4

Now it seems the top and bottom planes are semi-transparent (but still partially).

LiangliangNan avatar Jan 06 '22 15:01 LiangliangNan

That's good news lol. I added a couple of commits each progressively removing a feature until its down to just bvh traversal and triangle intersection. Let me know if any of the commits ends up working, that way I'll know where to look.

knightcrawler25 avatar Jan 06 '22 17:01 knightcrawler25

Thanks for the efforts 👍 "Normals" is correct and "Reflect" has the problem. See below:

Disable NEE Disable NEE

Remove materialTex Remove materialTex

Reflect Reflect

Normals Normals

LiangliangNan avatar Jan 06 '22 18:01 LiangliangNan

(Edit: Ignore this, its just an effect of clamping negative values)

Normals don't look right. Was the screenshot from 9a09313dc3c8deba67cf94fe6e0a7a540b726f07?

This is what I see from that commit:

image

knightcrawler25 avatar Jan 06 '22 19:01 knightcrawler25

Yes. it is from 9a09313. The figure captions correspond to your comments

LiangliangNan avatar Jan 06 '22 19:01 LiangliangNan

Do you see the same image with the last two commits where mesh transformations have been removed?

knightcrawler25 avatar Jan 06 '22 20:01 knightcrawler25

Here are the results from the last three commits:

hit dist: hit dist

remove transformation from mesh remove transformation from mesh

Remove transformation from BVH Remove transformation from BVH

LiangliangNan avatar Jan 07 '22 19:01 LiangliangNan