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

Use amount and perceptual roughness constants from Filament. #56822

Closed
wants to merge 1 commit into from

Conversation

fire
Copy link
Member

@fire fire commented Jan 15, 2022

Reviewing the constants from Filament and changed to the roughness amount of 1.0 and the roughness limit to be 0.002025. https://github.com/google/filament/blob/main/shaders/src/shading_lit.fs

https://github.com/google/filament/blob/9914eb84a4d613d609ae77a1d031fadf973db02c/shaders/src/common_material.fs#L6

Need some help doing screenshot comparison @Calinou .

Updated:

  1. Update sample to latest Godot master Calinou/test_specular_aliasing#1
  2. PR Use amount and perceptual roughness constants from Filament. #56822

Edited:

Uses the 0.002025 constant now.

@Calinou
Copy link
Member

Calinou commented Jan 15, 2022

Need some help doing screenshot comparison @Calinou .

I made a slider comparison with Calinou/test_specular_aliasing@61836cb: https://imgsli.com/OTEwNzM/1/2
(Note: I pushed a new commit since that uses lossless texture compression to work around the VRAM mipmap issue.)

This PR seems to get ever so slightly closer to ground truth, so I think the new values are a good idea.

@fire fire force-pushed the roughness-filtering branch from c552717 to 2f68c72 Compare January 15, 2022 22:14
@fire fire requested a review from a team as a code owner January 15, 2022 22:14
@fire
Copy link
Member Author

fire commented Jan 15, 2022

#define MIN_ROUGHNESS 0.002025 on rendering/anti_aliasing/screen_space_roughness_limiter/limit

Removes a lot of the aliasing artifacts but I can't tell if it's correct.

@fire fire force-pushed the roughness-filtering branch from 2f68c72 to 9962a2e Compare January 15, 2022 22:32
@fire
Copy link
Member Author

fire commented Jan 15, 2022

@Calinou can you post an imgsli of the new 0.002025 constant?

@fire fire force-pushed the roughness-filtering branch from 9962a2e to 2d635b7 Compare January 15, 2022 22:43
@fire
Copy link
Member Author

fire commented Jan 15, 2022

@Calinou
Copy link
Member

Calinou commented Jan 15, 2022

@Calinou can you post an imgsli of the new 0.002025 constant?

Done: https://imgsli.com/OTEwODc/1/2

It seems the new roughness limiter values are barely different from having the roughness limiter disabled in the first place.

@fire
Copy link
Member Author

fire commented Jan 15, 2022

For someone who was wondering what barely different from having the roughness limiter disabled in the first place means here's a video.

2022-01-15.15-01-05.mp4

@fire fire force-pushed the roughness-filtering branch from 2d635b7 to e77c6c0 Compare January 16, 2022 00:18
@fire fire marked this pull request as draft January 16, 2022 02:56
@fire fire force-pushed the roughness-filtering branch 6 times, most recently from 21589b6 to ea477be Compare January 16, 2022 04:51
@Calinou
Copy link
Member

Calinou commented Jan 16, 2022

With the new changes, I can still barely see a difference between the current roughness limiter and the new one: https://imgsli.com/OTEyOTE

The images are different, as demonstrated by dssim:

difference png-0

@fire
Copy link
Member Author

fire commented Jan 16, 2022

I was told that:

The function mainly affects specular aliasing on geometry edges

So I was recommended to test something metallic but mostly geometry instead of shader detail, like not a geometric primitive plane.

This matches the intuition that the current system uses surface normals.

Edited:

Something like a chair or table, or if you just want something obvious a bunch of metal bars and spikes.

@fire fire force-pushed the roughness-filtering branch from ea477be to 4ec19f1 Compare February 9, 2022 16:52
@fire
Copy link
Member Author

fire commented Feb 9, 2022

The paper authors use bistro with all objects turned metallic.

From clayjohn.

@fire fire force-pushed the roughness-filtering branch from 4ec19f1 to b3d5641 Compare May 19, 2022 16:14
@fire fire marked this pull request as ready for review May 19, 2022 16:14
@SaracenOne SaracenOne force-pushed the roughness-filtering branch from b3d5641 to 866094d Compare June 7, 2022 23:31
@fire fire requested a review from clayjohn August 24, 2022 15:39
@fire
Copy link
Member Author

fire commented Aug 24, 2022

After a conversation with @clayjohn, he said that we should test to demonstrate improvement.

After the great work of wickedinsignal, we now have a great test map at #63374.

@fire
Copy link
Member Author

fire commented Sep 1, 2022

I wasn't able to find time to test this again. Probably needs to be salvaged.

Use the roughness  amount and roughness constants from Filament.
@fire fire force-pushed the roughness-filtering branch from 866094d to 96f5000 Compare January 6, 2023 16:29
@fire
Copy link
Member Author

fire commented Jan 7, 2023

Godot's settings now almost match the pdf's with a difference of like 0.03

@fire fire closed this Jan 7, 2023
@fire fire deleted the roughness-filtering branch January 7, 2023 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants