Skip to content
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

Spatial material roughness/env map issues #21680

Closed
fracteed opened this issue Sep 2, 2018 · 33 comments · Fixed by #29182 or #33668
Closed

Spatial material roughness/env map issues #21680

fracteed opened this issue Sep 2, 2018 · 33 comments · Fixed by #29182 or #33668

Comments

@fracteed
Copy link

fracteed commented Sep 2, 2018

Godot version:
3.1 alpha 1

OS/device including version:
Win10 960m

Issue description:
This is a continuation of issue #18880 which is a long thread that discussed roughness values in Substance as compared to Godot. As a result, @reduz added this commit : 6223342

Here is a comparison to the previous 3.0.6 result and the same scenes in Substance and Blender/eevee.

roughness_test_comparison

The metallics do look a little better, but there is still almost no distinction between roughness levels in the metallics at levels 0.1/0.2 and 0.3/0.4 as compared to the smooth gradation in Substance and Blender. The blurring in general does not look very smooth as compared to Substance...not sure if this is the amount of the blurring or the technique used?

However, the highlight reflections for the dielectric materials (in orange) in Godot do now look better then eevee, in comparison to Substance . Also, the blue rings on top/bottom look very close now compared to the Substance render.

More strangely though is the general shading of the orange dielectrics in 3.1 alpha now. Not sure if it is because of the recent env map changes, but the material is almost looking like a flat luminant material now, as the shading has almost disappeared. Look at the darker non facing parts between the orange rings in the previous renders as compared to now.

Note that there are no lights or SSAO in this scene, just the hdri providing all lighting info.

Here is a zipped file that contains the Godot, Blender and Substance Painter project files:

roughness_test_3_1alpha1.zip

EDIT: the more I looked at this image, it became clear that the correct solution is about halfway between what 3.0.6 and 3.1 alpha are doing. I mocked this up in an image editor so that the 3.1 render was overlaid over the 3.0.6 render at exactly 50% opacity. It looks very close to the Substance render for the dielectric! Reduz, if you can get that cubemap angle at only 50% of what you changed it from, this could be the solution?

roughness_test_comparison2

@fracteed
Copy link
Author

fracteed commented Sep 2, 2018

Just did a quick render comparison between 2.0.6 (on left) and 3.1 alpha 1 (on right) and they look identical. Nothing has broken on the shading side, as this scene uses only giprobe and luminant textures to light it (no hdri). I would guess that the strange result above is due to the recent env map commit.

godot_compare_test

@akien-mga akien-mga added this to the 3.1 milestone Sep 2, 2018
@reduz reduz self-assigned this Sep 8, 2018
@akien-mga akien-mga changed the title Spatial material roughness/env map issues in 3.1 alpha 1 Spatial material roughness/env map issues Jan 13, 2019
@akien-mga
Copy link
Member

Is this still reproducible in the current master branch?

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 23, 2019
@fracteed
Copy link
Author

Yes, as far as I know nothing has changed here.

@mysticfall
Copy link
Contributor

Yes, I still have to manually edit roughness maps that I export from Substance Painter, cranking the brightness up, because it looks way too glossy in Godot even with the roughness set to 1.0.

@mysticfall
Copy link
Contributor

By the way, I have a similar kind of a problem with the transmission texture also. Just like it is the case with the roughness value, assigning a moderately bright grayscale image to the transmission slot makes the material looks extremely bright.

If it was possible to adjust such a value from 0 to 1.0 before, assinging a texture feels like it's stuck at 0.95-1.0 (or 0.0-0.05 for the roughness).

I'm not sure if I should report a separate issue for this, but I'm beginning to wonder if there could be the same type of a bug across material parameters regarding a texture map input.

@akien-mga
Copy link
Member

Is this better after #27898?

@volzhs
Copy link
Contributor

volzhs commented May 14, 2019

GLES3
Screenshot_20190514_161722

GLES2
Screenshot_20190514_162418

This is what I got without changing anything. just for curiosity.

@Calinou
Copy link
Member

Calinou commented May 14, 2019

I ran the demo locally and can confirm it looks the same as on @volzhs' screenshots.

@volzhs
Copy link
Contributor

volzhs commented May 14, 2019

hm.... my personal project seems broken...
all material is too bright now :(

@volzhs
Copy link
Contributor

volzhs commented May 14, 2019

tps1
this looks similar to mine with current master.

@volzhs
Copy link
Contributor

volzhs commented May 14, 2019

I found that looks fine without Omni/Spot/Directional light.
But with those light sources, it makes too bright.

@clayjohn
Copy link
Member

@volzhs The GLES2 and GLES3 backends are not expected to look the same, when switching between them you need to change the light intensities to account for the difference between the way each renders objects. Godot doesn't use physically based units for lighting, so you always have to fudge the numbers a little bit. It is expected that rough materials will appear slightly brighter than they did previously

Here is a picture of various roughness/metallic settings on a sphere. You can see that the rough spheres (especially the rough metal) appears way darker than the rest.
http://docs.godotengine.org/en/latest/_images/PBR.png
This was because the previous behaviour was losing light on rough materials (especially metallics) the solution was to make the brdf energy conserving. But if it's no longer losing light, then it is going to appear a little brighter.

@Calinou
Copy link
Member

Calinou commented May 14, 2019

I guess we'll need to add a project setting to emulate the old behavior then. It should probably be disabled by default, but it'd be good if the project version conversion code could enable it automatically.

Still, on @volzhs' TPS demo screenshot, the robot really looks like it's made of clay, which I find suspect (it's not just too bright, it barely displays the albedo information).

@Megalomaniak
Copy link

Megalomaniak commented May 14, 2019

As surface illumination gets brighter there is a loss in saturation(washout if you will). Might be the case though that the washout/effect is too strong perhaps? Might be up to tone-mapping, too. Will likely require further investigation to see what the actual underlying issue is, if there really is one.

@clayjohn
Copy link
Member

clayjohn commented May 14, 2019

Tried reverting #27898, It fixes the issues. Strange that it didnt have this effect on my test scene as I wrote it. I will work out a fix ASAP.

edit: Issues a fix in #28891 With this fix you should not see any change in this issue.

@clayjohn
Copy link
Member

clayjohn commented May 26, 2019

I've been playing around with this issue a little bit lately. I'm starting to see some good results. Increasing number of samples and decreasing the size of the radiance map gives really good results and is faster.

env-map-fix

The above image uses 1024 samples (current is 512 for desktop, but because of a bug desktop was using 64) and a radiance size of 128 (default is 512, but with proper samples the visual difference between 128 and 512 is negligible).

edit: A proper solution is going to take a bit of discussion. We can solve this issue easily with three changes 1) use high_quality_ggx setting instead of high_quality_ggx.mobile setting for desktops

bool ggx_hq = GLOBAL_GET("rendering/quality/reflections/high_quality_ggx.mobile");

(right now everyone is stuck using a low sample count for computing radiance)
2) increase SAMPLE_COUNT (for desktop) to 1024 (at least)
#define SAMPLE_COUNT 512u

and 2) decrease radiance_size (default is 512 now, new default should be 128 or 64)

The issue is that if we increase SAMPLE_COUNT, or even fix the settings bug, users are going to see a huge increase in load times without an appreciable difference in quality because the radiance size in their projects will be stuck at 512 when it should be much lower.

So we need a way to fix the ggx quality bug, increase the SAMPLE_COUNT, and decrease radiance size without impacting users too much. Im not sure how that can be done.

Also, if we apply the above fixes, we will fix #27422 as well.

@fracteed
Copy link
Author

@clayjohn thanks a lot for looking into this issue! Good to hear that you are one the case :)
The metallics on levels 0.3/0.4 definitely looking better and closer to the substance reference. Any idea why metallic 0.3/04 are looking almost identical, whereas when you look at the substance reference there is a noticeable increase in roughness over all 5 levels.

Levels at 0 roughness look incorrect on on both metallic and dielectric as you can barely even see the hdri reflection. Any idea why?

The roughness reflections also are looking better on the dielectrics, but in general they are still looking strange as compared to godot 3.0.6. If you look at the reference comparison image, there is no falloff on the 90 degree faces, and it almost looks like it has a fake sss/backlit look. I actually thought that the 3.0.6 version looked much better. Not sure what reduz changed exactly..was it the way the cubemaps are blurred?

@clayjohn
Copy link
Member

clayjohn commented May 26, 2019

any idea why metallic 0.3/04 are looking almost identical, whereas when you look at the substance reference there is a noticeable increase in roughness over all 5 levels.

Not sure exactly, the results now are more similar to the ones in eevee. It is likely due to using a different brdf. Or maybe substance takes the power of roughness.

Levels at 0 roughness look incorrect on on both metallic and dielectric as you can barely even see the hdri reflection. Any idea why?

My camera was in the wrong position! I actually fixed that after. Moving the camera around really affects the way that they appear.

The roughness reflections also are looking better on the dielectrics, but in general they are still looking strange as compared to godot 3.0.6

I think it is also a result of the brdf that we use when rending and not in the production of the radiance map.

edit: Ah! I forgot to mention. In the meantime you can drastically increase the quality of your radiance maps by enabling "high_quality_ggx.mobile" in your project settings and decreasing the size of your radiance maps to 128 or less.

@fracteed
Copy link
Author

fracteed commented May 26, 2019

Oh yeah, I just used an orthographic front view for all of the screengrabs, to make sure that they looked as similar as possible between applications.

So I am guessing that radiance map size is Quality/reflections/Atlas size? What about the atlas subdiv?

I think it is also a result of the brdf that we use when rending and not in the production of the radiance map.

The big change in dielectric appearance that occured between 3.0.6 and 3.1 via 6223342 looks like it only changed the cubemap filter. I don't think that the brdf changed in this timeframe?

@clayjohn
Copy link
Member

So I am guessing that radiance map size is Quality/reflections/Atlas size? What about the atlas subdiv?

Its actually a property in the sky itself http://docs.godotengine.org/en/latest/classes/class_sky.html#class-sky-property-radiance-size

The big change in dielectric appearance that occured between 3.0.6 and 3.1 via 6223342 looks like it only changed the cubemap filter. I don't think that the brdf changed in this timeframe?

I don't know. I'm not familiar with how things were in 3.0 as I wasn't heavily involved back then. I'll be looking into improving the radiance map stuff further though.

@fracteed
Copy link
Author

Cheers! Yeah, there was definitely something funky that happened with that particular commit. Maybe it was the normalize removed from vec3 L = normalize(2.0 * dot(V, H) * H - V) ? The math is beyond me, but there was a visible regression with this commit.

As noted in my photoshopped comparison image in the first post, the correct looking result (aside from the actual quality of the roughness) seemed to be somewhere between what 3.0.6 and 3.1 were doing when it comes to the cubemap filtering code.

@fracteed
Copy link
Author

@clayjohn I have been playing around with the roughness demo in the current master. The changes you have made do make the metallics look better, but there are still issues with the dielectric materials.

I made an attempt to compare the old cube filter code and the current one by reverting 6223342 and changing the 2 x multiplier in line 247 so that it reads: vec3 L = normalize(4.0 * dot(V, H) * H - V);

This gives a much better appearance to the dielectrics and is very close to the substance reference, but the metallics don't look as good as the current master. As can be seen in this image.

roughness_test_comparison

I am way out of my league here, so maybe you know how the cubefilter could be changed to get the best of both of these worlds ?

@clayjohn
Copy link
Member

clayjohn commented May 30, 2019

@fracteed I wish I could say it was that easy. The L = normalize(2.0 * dot(V, H) * H - V) is standard code, it's not really a tweakable parameter. It has a very clear meaning in the context of PBR rendering.

While that change does make it look nicer it is likely doing it for the wrong reasons. There is something else that is being done wrong but I can't find what it is. The dielectrics do look waaaay better though. Im just wondering if there is something going on with the actual rendering of the materials using the map. Or maybe it has something to do with the way the texture array works. You'll notice if you change your project setting to use a single mip-mapped texture things go bonkers. Or in GLES3 certain resolutions make the environment map go crazy.

Anyway, the point of all that is that there is a lot of work to be done on the environment maps.

edit: If you want more information about how it works take a look here https://learnopengl.com/PBR/IBL/Specular-IBL we use the exact same code...

@fracteed
Copy link
Author

fracteed commented May 30, 2019

Ha, yes it was just a cheap hack on my part :) Thanks for the link, as I had no idea what the code was based on. That is a very nice website, so at least try to wrap my head around the theory at least.

You are right that the problem probably lies elsewhere, so I hope that you can keep digging into the problem! If I had a choice I do prefer the previous look before that reduz commit and would revert it. Most scenes tend to have more dielectric surfaces than pure metallic ones, so I would prefer having the dielectrics to look more accurate if it was one way or the other.

I am still developing my game in 3.0.6 but have noticed when I test it on 3.1 or later that my outdoor levels (which use a HDRI) look somewhat worse. The surfaces have less definition and look more stylized as can be seen in the roughness demo comparison. Not sure if all of this will be rewritten for Vulkan anyway, or if it will inherit the same issues...time will tell.

Btw, I tried upping the radiance from the default value on this demo scene to 512 or 1024 and get an instant crash (this is only my old win7 laptop with a pitiful hd4000), but I thought I would at least mention it.

edit: and I just tried the same scene using gles2 and noticed that the metallics look decent but the dielectrics look very odd. Not sure if this is to be expected as I never usually use gles2?

@clayjohn
Copy link
Member

I get the crash too on higher resolution maps with my low-end device too. Intel devices tend to quit early if they suspect the GPU of hanging. Mine usually crashed on 1024, and always crashed on 2048 previously and now always crashes on 1024. To be honest though, you don't need above 128 for most scenes, I would only go up to 512 if I had a lot of large objects with high metallic and no roughness.

For Vulkan this will all be rewritten. So reduz doesn't have a huge interest in fixing stuff up. For GLES3 I am just trying to focus on bugs and low-hanging fruit. My focus is mostly on GLES2 now (although GLES2 will need to be substantially rewritten for 4.0 too).

@fracteed
Copy link
Author

Agreed that 128 radiance is fine, and I don't usually change the default. Sounds like crashes from high values are not a new phenomena. Does make me wonder if there is any point to having those presets that go above 512.

I plan on shifting my games over to Vulkan at the end of the year, or whenever there is an alpha of 4.0. Am happy to mostly stick with 3.0.6 for the moment, so I am not overly concerned about the cubemap/roughness issues. It may not be noticed by many devs anyway, unless they are using properly authored PBR assets with outdoor scenes lit by a HDRI. The majority seem to be doing more stylised low poly games.

@fracteed
Copy link
Author

@akien-mga can you please re-open this issue. Although the change by @clayjohn did improve the metallics appearance, there is still a noticeable regression from 3.0.6 to 3.1 as noted in the original post that has still not been addressed.

No doubt this issue will have to be revisited when Vulkan builds appear in a few months, so it would be better to leave this open as a point of reference.

@clayjohn
Copy link
Member

clayjohn commented Nov 15, 2019

@fracteed Im making some good progress :)

Looks like specular highlights are still too bright on non-metallics though. Just as pointed out in #33616

Screenshot (4)

What I did to get this result is simply add a proper irradiance map for diffuse. Previously irradiance was being ignored. I need to figure out why specular IBL is appearing so strong. Once that gets tuned down things should be good to go. :)

@fracteed
Copy link
Author

Thanks @clayjohn I really appreciate you looking into this matter! Yes, this is looking closer. I look forward to your progress :)

@clayjohn
Copy link
Member

Updated Progress.

Screenshot (10)

@fracteed
Copy link
Author

Oh wow, great work...that is a big improvement! This is looking very close to the substance reference on the dielectrics now. The main thing that stands out now is the white halo around the grazing edges of the top/bottom rings.

Are you going to apply the radiance map changes you made to the metallics as well, to get them a bit smoother looking?

@clayjohn
Copy link
Member

The main thing that stands out now is the white halo around the grazing edges of the top/bottom rings.

Ive been working on this all day. I still cant figure out the cause. One thing that I noticed is that it only appears this strongly when using an orthogonal camera. With the normal perspective camera the effect is a lot more subtle and appropriate.

Are you going to apply the radiance map changes you made to the metallics as well, to get them a bit smoother looking?

The irradiance map only applies to dielectrics. I did some other work to smooth out the metallics. :)

At this point I think I will make a WIP PR so you can test out the current changes. I still need to copy all the work over to GLES2 as well.

Current iteration (Orthogonal):

Screenshot (16)

Current iteration (Perspective):
Screenshot (20)

@fracteed
Copy link
Author

You have done an amazing job here @clayjohn! I was probably just nitpicking about the grazing edges :) This hdri is quite extreme anyway, which is why I chose it for this test scene. Overall it is looking fantastic, and I am impressed with how much better the metallics are looking. It is actually looking much closer to the substance look than eevee, so I look forward to trying it out.

On a related note, when the time comes, it would be great for you to make these kind of changes to the vulkan branch. The IBL is quite broken in the vulkan branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment