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

--[PBR part 3 of 3]Map KHR Extensions to PBRDrawables and Implement in Shader #2091

Merged
merged 75 commits into from
Jun 16, 2023

Conversation

jturner65
Copy link
Contributor

@jturner65 jturner65 commented May 1, 2023

Motivation and Context

This PR will introduce some limited support in our existing rewrites our PBR shader and extends it to support Khronos-specified materials layers (ior, specular textures, clearcoat, anisotropy) by

  • Mapping the ClearCoat and KHR extension fields that exist in a particular material layer to the PBRDrawable cache
  • Using those cached values to be written into the shader
  • Using those fields in the shader for the fragment color calculation.

Tangent-space TBN frame derivation is also supported when precomputed tangents are not provided, which enables the use of normal textures.

Extensions supported so far :

  1. KHR_materials_ior : instead of using a hardcoded DielectricSpecular of 0.04 (which assumes a material IOR of 1.5), we are using material-provided IOR (if exists) to derive the DielectricSpecular. This mainly served as proof of concept and pipeline test.
    https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_materials_ior

  2. KHR_materials_specular : The values in this extension consist of float and texture strength and color and texture specular colors. These are used to modify the fresnel specular normal incidence and glancing incidence terms.
    https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_materials_specular

  3. KHR_materials_clearcoat : The values in this extension consist of a float and texture strength, a 3-vector and texture color and a texture-based normal. Currently the normal support is incomplete (it uses base normal), but otherwise clearcoat is supported.
    https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_materials_clearcoat

  4. Generating tangents for normal textures, clearcoat normal textures and anisotropy when precomputed tangents are not provided. Using a partial derivative calculation in the fragment shader on both position and texture coordinates, the ability to generate tangent-space TBN frame was added. This follows the concept given here https://jcgt.org/published/0009/03/04/paper.pdf Section 3.3.

  5. KHR_materials_anisotropy : The values in this extension consist of a strength value, a direction in tangent-space, and potentially a texture. These are used to give a directional bias to the specular calculation.
    https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_materials_anisotropy

Comparison Before and After videos of PBR changes using a Floorplanner scene

A few things to note :

  • There is no Image Based Lighting in either example. These are intended only to showcase the differences in the direct lighting PBR calculations.
  • Both videos have the same lighting configuration.

Below is the current state of the PBR shader:

oldPBR102817140.mp4

And here is an example of the same scene with this PR's updated PBR shader computations

newPBR102817140.mp4

A few improvements to notice :

  1. The floor in the first example is washed out and hard to see. Compare this to the floor in the updated shader.
  2. Note the shadows in interior of the the washer/dryer between the old and new shader. In the new shader they follow a more realistic interior geometry, which becomes even more apparent with IBL (see below). This improved "shadow"/normal behavior is due to the addition of generated tangents.
  3. The picture frame by the dinner table has appropriate specularity in the new shader. This was lacking due to a small calculation bug in the original PBR shader.
  4. There are a cluster of 3 white capsule-like statues at approximately 1/3 of the way through each video. In the current PBR shader they are flat, whereas in the new PBR shader they have a nice shine and a dark edge to them - that's the clearcoat layer in action.
  5. In the old shader many objects are flat and dull, including the plants, furnishings in the kitchen, the ceramic cactus in the bedroom and the thermos-looking statues by the television. Compare these with the new shader. Many of these owe their shine to a clearcoat layer, and some have specular textures.

New PBR with IBL

Here's that same scene lit with both the current default lights on main and the current IBL setup on main, weighted equally 50/50.

newPBR_IBL102817140.mp4

How Has This Been Tested

Existing c++ and python tests pass, with new test images where appropriate.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jturner65 jturner65 requested a review from 0mdc May 1, 2023 19:53
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 1, 2023
@jturner65 jturner65 requested a review from aclegg3 May 1, 2023 19:54
@jturner65
Copy link
Contributor Author

jturner65 commented May 4, 2023

I fully expect Simtest to fail, since I am changing the PBR shader and the core lighting. The images for a few tests will probably have to be retaken. The test images have been updated for the new lighting configs.

Tests may need to be added to test extension performance. (I.E clearcoat, specular texture, etc)

@jturner65 jturner65 force-pushed the mapKHRToDrawables branch 2 times, most recently from 82dcc7e to 6fbffd6 Compare May 5, 2023 13:32
src/esp/gfx/PbrDrawable.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrDrawable.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrDrawable.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.h Outdated Show resolved Hide resolved
src/esp/assets/ResourceManager.cpp Outdated Show resolved Hide resolved
src/esp/gfx/PbrDrawable.cpp Outdated Show resolved Hide resolved
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch 2 times, most recently from be3f571 to 90ca688 Compare May 9, 2023 17:14
@jturner65 jturner65 changed the title --[WIP]Map KHR Extensions to PBRDrawables and Shader --[WIP]Map KHR Extensions to PBRDrawables and Implement Shader May 10, 2023
@jturner65 jturner65 changed the title --[WIP]Map KHR Extensions to PBRDrawables and Implement Shader --[WIP]Map KHR Extensions to PBRDrawables and Implement in Shader May 10, 2023
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch 9 times, most recently from 548e5ef to ca31098 Compare May 17, 2023 12:41
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from 454e5ff to ba4746a Compare May 18, 2023 15:51
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch 5 times, most recently from 1e57af3 to 805e938 Compare May 24, 2023 17:54
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from 790cc13 to 654f9ea Compare May 29, 2023 13:45
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from 654f9ea to 9d5620c Compare May 29, 2023 18:07
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from 8ec0dcf to 93bd93d Compare June 13, 2023 21:18
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from dbc960e to a928fb3 Compare June 14, 2023 21:38
Copy link
Collaborator

@mosra mosra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't go through the actual shader algorithms, but the GLSL code itself looks alright.

src/esp/gfx/PbrDrawable.cpp Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrTextureUnit.h Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.cpp Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.cpp Outdated Show resolved Hide resolved
src/esp/gfx/PbrShader.cpp Outdated Show resolved Hide resolved
src/esp/gfx/Drawable.h Outdated Show resolved Hide resolved
src/esp/assets/ResourceManager.cpp Outdated Show resolved Hide resolved
@jturner65 jturner65 force-pushed the mapKHRToDrawables branch from 362ec2e to cdfd241 Compare June 15, 2023 21:21
Consistent flag comparison using subset.
Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, great step toward better rendering. 👍

This is to provide the user a single field by which they can tweak how bright or dark their scene is without having to reauthor their lighting configs.
Configuration-controllable color remapping.
@jturner65 jturner65 merged commit 960e05a into main Jun 16, 2023
@jturner65 jturner65 deleted the mapKHRToDrawables branch June 16, 2023 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants