Implement clearcoat per the Filament and the `KHR_materials_clearcoat` specifications. #13031

pcwalton · 2024-04-19T17:21:13Z

Clearcoat is a separate material layer that represents a thin translucent layer of a material. Examples include (from the Filament spec) car paint, soda cans, and lacquered wood. This commit implements support for clearcoat following the Filament and Khronos specifications, marking the beginnings of support for multiple PBR layers in Bevy.

The KHR_materials_clearcoat specification describes the clearcoat support in glTF. In Blender, applying a clearcoat to the Principled BSDF node causes the clearcoat settings to be exported via this extension. As of this commit, Bevy parses and reads the extension data when present in glTF. Note that the gltf crate has no support for KHR_materials_clearcoat; this patch therefore implements the JSON semantics manually.

Clearcoat is integrated with StandardMaterial, but the code is behind a series of #ifdefs that only activate when clearcoat is present. Additionally, the pbr_feature_layer_material_textures Cargo feature must be active in order to enable support for clearcoat factor maps, clearcoat roughness maps, and clearcoat normal maps. This approach mirrors the same pattern used by the existing transmission feature and exists to avoid running out of texture bindings on platforms like WebGL and WebGPU. Note that constant clearcoat factors and roughness values are supported in the browser; only the relatively-less-common maps are disabled on those platforms.

This patch refactors the lighting code in StandardMaterial significantly in order to better support multiple layers in a natural way. That code was due for a refactor in any case, so this is a nice improvement.

A new demo, clearcoat, has been added. It's based on the corresponding three.js demo, but all the assets (aside from the skybox and environment map) are my original work.

Changelog

Added

StandardMaterial now supports a clearcoat layer, which represents a thin translucent layer over an underlying material.
The glTF loader now supports the KHR_materials_clearcoat extension, representing materials with clearcoat layers.

Migration Guide

The lighting functions in the pbr_lighting WGSL module now have clearcoat parameters, if STANDARD_MATERIAL_CLEARCOAT is defined.
The R reflection vector parameter has been removed from some lighting functions, as it was unused.

github-actions · 2024-04-19T17:31:01Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

github-actions · 2024-04-19T20:18:52Z

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

specifications. Clearcoat is a separate material layer that represents a thin translucent layer of a material. Examples include (from the [Filament spec]) car paint, soda cans, and lacquered wood. This commit implements support for clearcoat following the Filament and Khronos specifications, marking the beginnings of support for multiple PBR layers in Bevy. The [`KHR_materials_clearcoat`] specification describes the clearcoat support in glTF. In Blender, applying a clearcoat to the Principled BSDF node causes the clearcoat settings to be exported via this extension. As of this commit, Bevy parses and reads the extension data when present in glTF. Note that the `gltf` crate has no support for `KHR_materials_clearcoat`; this patch therefore implements the JSON semantics manually. Clearcoat is integrated with `StandardMaterial`, but the code is behind a series of `#ifdef`s that only activate when clearcoat is present. Additionally, the `pbr_feature_layer_material_textures` Cargo feature must be active in order to enable support for clearcoat factor maps, clearcoat roughness maps, and clearcoat normal maps. This approach mirrors the same pattern used by the existing transmission feature and exists to avoid running out of texture bindings on platforms like WebGL and WebGPU. Note that constant clearcoat factors and roughness values *are* supported in the browser; only the relatively-less-common maps are disabled on those platforms. This patch refactors the lighting code in `StandardMaterial` significantly in order to better support multiple layers in a natural way. That code was due for a refactor in any case, so this is a nice improvement. A new demo, `clearcoat`, has been added. It's based on [the corresponding three.js demo], but all the assets are my original work. [Filament spec]: https://google.github.io/filament/Filament.html#materialsystem/clearcoatmodel [`KHR_materials_clearcoat`]: https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_materials_clearcoat/README.md [the corresponding three.js demo]: https://threejs.org/examples/webgl_materials_physical_clearcoat.html

robtfm

just a first pass. the changes to the core lighting functions are pretty extensive, @mockersf could we please run a demo comparison?

i think we need to make proper efforts to stop the lighting code becoming more write-only than it already is. not saying this makes it much worse than it already was, but it's honestly very hard to follow.

it would also be nice to improve the example a bit (the three.js one is nicer, and i don't think their implementation is better) but that can certainly be done later.

Cargo.toml

robtfm · 2024-04-21T14:10:13Z

crates/bevy_gltf/src/loader.rs

+
+impl ClearcoatExtension {
+    #[allow(unused_variables)]
+    fn parse(


could this be upstreamed?

There is a PR: gltf-rs/gltf#362

And it's been sitting since 2022. :( gltf doesn't seem to be well maintained. I pinged the maintainer, but I don't think we can realistically block this PR on that...

crates/bevy_pbr/src/render/pbr_fragment.wgsl

crates/bevy_pbr/src/render/pbr_functions.wgsl

robtfm · 2024-04-21T14:50:19Z

crates/bevy_pbr/src/pbr_material.rs

+        #[cfg(feature = "pbr_multi_layer_material_textures")]
+        {
+            if self.clearcoat_texture.is_some() {
+                flags |= StandardMaterialFlags::CLEARCOAT_TEXTURE;
+            }


nitpick - these flags should probably be added in the prepass as well in case anybody wants to use them

I don't know what this means. The prepass doesn't care about any of these flags, and we don't do this anywhere else (grepping for e.g. BASE_COLOR brings up nothing related to prepasses, except for alpha discard, which isn't relevant here).

Yeah I wouldn't add them. Sometime soon we should revisit how pipeline keys are managed however.

pcwalton · 2024-04-22T03:19:06Z

@robtfm How would you like me to improve the example? I wanted to include a transparent one because @JMS55 asked me how clearcoat looked with transparency. Should I add more of a noise pattern to the car paint? Redo the golf ball texture?

pcwalton · 2024-04-22T03:21:31Z

@robtfm I tried to improve the lighting code as I went and make it as easy to follow as possible. Sorry to hear that I didn't succeed :( Do you have any suggestions?

robtfm · 2024-04-22T14:30:24Z

@robtfm How would you like me to improve the example? I wanted to include a transparent one because @JMS55 asked me how clearcoat looked with transparency. Should I add more of a noise pattern to the car paint? Redo the golf ball texture?

i think the main issue i have is the specular aliasing from the normal maps. it seems less "bitty" in the 3js version. perhaps just downres the normal map, or just use a smaller one maybe so the single pixel highlights are less frequent?
the texture on the carpaint is nicer in 3js as well (and the spinning spheres) but i'm not sure that's worth adding a new texture for.

@robtfm I tried to improve the lighting code as I went and make it as easy to follow as possible. Sorry to hear that I didn't succeed :( Do you have any suggestions?

yes sorry, i didn't mean to sound so critical. i think if you hadn't made such an effort it would be much worse, whereas now it's not much worse, and with more functionality. the 2 specific comments about a param struct and factoring the meshlet sample were the main things i thought would help. i guess if the lighting functions api is clean then at least the trickier stuff is more contained.

pcwalton · 2024-04-22T23:10:14Z

@robtfm I believe every comment has been addressed. pbr_lighting.wgsl has been largely rewritten. I made the spheres rotate, added some blur and decreased the levels on the scratch normal map, added some subtle blue noise as a normal map to the car paint, and switched to ACES filmic tonemapping, as this is what makes the normal map on the car paint in three.js appear purple.

mockersf · 2024-04-22T23:13:04Z

just a first pass. the changes to the core lighting functions are pretty extensive, @mockersf could we please run a demo comparison?

Mostly good, just a change in the transmission example:
https://pixel-eagle.vleue.com/project/B25A040A-A980-4602-B90C-D480AB84076D/run/852/compare/840?screenshot=3D%20Rendering/transmission.png

pcwalton · 2024-04-22T23:16:45Z

How do I get rid of the GitHub typos check on Fo?

robtfm

looks good!

we should probably add something to the migration guide mentioning the changes required for custom prepasses/shaders that use apply_normal_mapping.

superdump · 2024-05-01T04:00:39Z

I’ve read the discussion, has any GPU-profiling/benchmarking been done across vendors to see what impact the shader refactoring has on performance without clear coat?

pcwalton · 2024-05-01T22:17:31Z

On NVIDIA, warp occupancy and register count are identical both before and after this PR.

Elabajaba · 2024-05-01T22:28:24Z

I did a quick test on my amd 6800xt, register pressure was identical and performance was the same according to radeon gpu profiler.

NthTensor · 2024-05-02T12:07:49Z

No obvious performance differences for me testing vs main. What's blocking this moving forward? The more general changes seem like a marked improvement in standardization and code quality.

pcwalton · 2024-05-02T19:12:36Z

@NthTensor It only has one approval. Having a second approval would be very helpful.

pcwalton · 2024-05-03T07:08:17Z

Closing as I am burned out and have no motivation to get this landed anymore.

alice-i-cecile · 2024-05-04T14:19:58Z

@pcwalton once merge conflicts are resolved this should be good to merge. Do reach out if you'd like a hand with that.

pcwalton · 2024-05-05T22:43:13Z

Updated.

alice-i-cecile · 2024-05-05T22:56:59Z

I'm comfortable with the thoroughness and expertise of the reviews here, and this isn't a controversial addition. Merging.

Fixes a regression introduced by bevyengine#13031.

Fixes a regression introduced by #13031.

…ng_input.F_ab (#13379) # Objective - The clearcoat PR #13031 had a small typo which broke transmission - Fixes #13284 ## Solution - Set transmissive_lighting_input.F_ab to the correct value ![transmission_fix](https://github.com/bevyengine/bevy/assets/688816/92158117-de3a-4fa5-8af8-dcbd1d5eee04)

pcwalton requested review from JMS55, robtfm and superdump April 19, 2024 17:21

pcwalton added C-Enhancement A new feature A-Rendering Drawing game state to the screen labels Apr 19, 2024

pcwalton added this to the 0.14 milestone Apr 19, 2024

pcwalton force-pushed the clearcoat branch from 10a3bfe to 9301903 Compare April 19, 2024 20:10

pcwalton force-pushed the clearcoat branch from 9301903 to 8a72736 Compare April 19, 2024 20:27

robtfm reviewed Apr 21, 2024

View reviewed changes

pcwalton added 6 commits April 21, 2024 20:59

Merge remote-tracking branch 'origin/main' into clearcoat

8d3c1dd

Address some review comments

7b9a968

Warning and Clippy police

2c29539

Taplo police

b6d5943

Fix transmission

b789bec

Remove ifdefs

c18daae

pcwalton added 5 commits April 22, 2024 13:30

Refactor the PBR lighting shader to use a shared struct

4f97524

Make layers individual structs

892d164

Refactor lighting some more

1a9d76f

Add a new normal map and change the tonemapping to match three.js better

f870325

Merge remote-tracking branch 'origin/main' into clearcoat

6077751

Make pbr_prepass.wgsl use the new sample_texture function

e2ab245

pcwalton requested review from robtfm and JMS55 April 30, 2024 03:59

robtfm approved these changes Apr 30, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into clearcoat

6e96b2c

Merge remote-tracking branch 'origin/main' into clearcoat

63bea02

JMS55 approved these changes May 3, 2024

View reviewed changes

pcwalton closed this May 3, 2024

pcwalton reopened this May 4, 2024

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through labels May 4, 2024

Merge remote-tracking branch 'origin/main' into clearcoat

819158f

Merge remote-tracking branch 'origin/main' into clearcoat

ddcd0a4

alice-i-cecile added this pull request to the merge queue May 5, 2024

Merged via the queue into bevyengine:main with commit 77ed72b May 5, 2024
31 checks passed

pcwalton added a commit to pcwalton/bevy that referenced this pull request May 6, 2024

Modulate the emissive texture by the emissive color again.

88a22c7

Fixes a regression introduced by bevyengine#13031.

pcwalton mentioned this pull request May 6, 2024

Modulate the emissive texture by the emissive color again. #13251

Merged

github-merge-queue bot pushed a commit that referenced this pull request May 6, 2024

Modulate the emissive texture by the emissive color again. (#13251)

59b52fc

Fixes a regression introduced by #13031.

mockersf mentioned this pull request May 8, 2024

transmission renders black on some GPU #13284

Closed

geckoxx mentioned this pull request May 15, 2024

Fix transmission by setting the correct value for transmissive_lighting_input.F_ab #13379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement clearcoat per the Filament and the `KHR_materials_clearcoat` specifications. #13031

Implement clearcoat per the Filament and the `KHR_materials_clearcoat` specifications. #13031

pcwalton commented Apr 19, 2024 •

edited

Loading

github-actions bot commented Apr 19, 2024

github-actions bot commented Apr 19, 2024

robtfm left a comment

robtfm Apr 21, 2024

pcwalton Apr 22, 2024 •

edited

Loading

robtfm Apr 21, 2024

pcwalton Apr 22, 2024

JMS55 Apr 25, 2024

pcwalton commented Apr 22, 2024

pcwalton commented Apr 22, 2024

robtfm commented Apr 22, 2024

pcwalton commented Apr 22, 2024

mockersf commented Apr 22, 2024 •

edited

Loading

pcwalton commented Apr 22, 2024

robtfm left a comment

superdump commented May 1, 2024

pcwalton commented May 1, 2024

Elabajaba commented May 1, 2024

NthTensor commented May 2, 2024

pcwalton commented May 2, 2024

pcwalton commented May 3, 2024

alice-i-cecile commented May 4, 2024

pcwalton commented May 5, 2024

alice-i-cecile commented May 5, 2024

Implement clearcoat per the Filament and the KHR_materials_clearcoat specifications. #13031

Implement clearcoat per the Filament and the KHR_materials_clearcoat specifications. #13031

Conversation

pcwalton commented Apr 19, 2024 • edited Loading

Changelog

Added

Migration Guide

github-actions bot commented Apr 19, 2024

github-actions bot commented Apr 19, 2024

robtfm left a comment

Choose a reason for hiding this comment

robtfm Apr 21, 2024

Choose a reason for hiding this comment

pcwalton Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

robtfm Apr 21, 2024

Choose a reason for hiding this comment

pcwalton Apr 22, 2024

Choose a reason for hiding this comment

JMS55 Apr 25, 2024

Choose a reason for hiding this comment

pcwalton commented Apr 22, 2024

pcwalton commented Apr 22, 2024

robtfm commented Apr 22, 2024

pcwalton commented Apr 22, 2024

mockersf commented Apr 22, 2024 • edited Loading

pcwalton commented Apr 22, 2024

robtfm left a comment

Choose a reason for hiding this comment

superdump commented May 1, 2024

pcwalton commented May 1, 2024

Elabajaba commented May 1, 2024

NthTensor commented May 2, 2024

pcwalton commented May 2, 2024

pcwalton commented May 3, 2024

alice-i-cecile commented May 4, 2024

pcwalton commented May 5, 2024

alice-i-cecile commented May 5, 2024

Implement clearcoat per the Filament and the `KHR_materials_clearcoat` specifications. #13031

Implement clearcoat per the Filament and the `KHR_materials_clearcoat` specifications. #13031

pcwalton commented Apr 19, 2024 •

edited

Loading

pcwalton Apr 22, 2024 •

edited

Loading

mockersf commented Apr 22, 2024 •

edited

Loading