-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tone mapping ruins the constant background color #61
Comments
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: Right now, the renderer doesn't track alpha/transparency and it would require some modification to get it to work. |
Thanks for the quick reply. |
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. |
Thanks for the efforts. Very much appreciated! Below I summary the issues I have identified in the test and in reading the code.
These things so far, hope it helps and looking forward to any update. Happy New Year! |
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? |
Oh, forgot to mention that.
To understand what goes wrong, I took snapshots for the four FBOs (every time when new contents have been written into). |
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 |
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. |
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). |
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? |
Then it is gone. But why? |
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. GLSL-PathTracer/src/core/TiledRenderer.cpp Lines 520 to 560 in a6526c8
|
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. |
One more issue.
When is no EnvMap loaded program fails also.
|
I check tests from https://github.com/KhronosGroup/glTF-Sample-Models glTF-Sample-Models\2.0\SpecularTest\ not passed. Specular parameter not changed glTF-Sample-Models\2.0\SpecGlossVsMetalRough\ not passed. Right bottle not texture mapped like left bottle glTF-Sample-Models\2.0\AttenuationTest |
@tigrazone : GLTF material support is not complete yet. Only metallic roughness workflow is used and transmissionFactor from the extensions: GLSL-PathTracer/src/loaders/GLTFLoader.cpp Lines 245 to 279 in ddbba9a
For the last screenshot, you'll have to increase number of bounces to get the blue color |
Agreed with blue in scenes - increase number of bounces helps to render blue colored boxes. |
@LiangliangNan : Could you also try the debug branch? It has just enough code to render cornell_box.scene and would be easier to check. |
Now there are fewer "black" things, but still similar. See the recording below: Untitled.mp4Now it seems the top and bottom planes are semi-transparent (but still partially). |
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. |
(Edit: Ignore this, its just an effect of clamping negative values) Normals don't look right. Was the screenshot from 9a09313? This is what I see from that commit: |
Yes. it is from 9a09313. |
Do you see the same image with the last two commits where mesh transformations have been removed? |
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:
With tone mapping:
I think whether it is background can be recorded as the w component in pathTraceTexture. Maybe there are other better ideas?
The text was updated successfully, but these errors were encountered: