knightcrawler25 / GLSL-PathTracer

A toy physically based GPU path tracer (C++/OpenGL/GLSL)
MIT License
1.86k stars 175 forks source link

Tone mapping ruins the constant background color #61

Open LiangliangNan opened 2 years ago

LiangliangNan commented 2 years ago

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?

knightcrawler25 commented 2 years ago

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.

LiangliangNan commented 2 years ago

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.

knightcrawler25 commented 2 years ago

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

LiangliangNan commented 2 years ago

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.

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

knightcrawler25 commented 2 years ago

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?

LiangliangNan commented 2 years ago

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

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.

knightcrawler25 commented 2 years ago

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

LiangliangNan commented 2 years ago

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

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.

knightcrawler25 commented 2 years ago

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).

LiangliangNan commented 2 years ago

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

knightcrawler25 commented 2 years ago

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?

LiangliangNan commented 2 years ago

Then it is gone. But why?

knightcrawler25 commented 2 years ago

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

LiangliangNan commented 2 years ago

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

knightcrawler25 commented 2 years ago

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.

LiangliangNan commented 2 years ago

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:

tigrazone commented 2 years ago

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 commented 2 years ago

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

knightcrawler25 commented 2 years ago

@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

tigrazone commented 2 years ago

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

knightcrawler25 commented 2 years ago

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

LiangliangNan commented 2 years ago

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).

knightcrawler25 commented 2 years ago

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.

LiangliangNan commented 2 years ago

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

knightcrawler25 commented 2 years ago

(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

LiangliangNan commented 2 years ago

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

knightcrawler25 commented 2 years ago

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

LiangliangNan commented 2 years ago

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