--PBR/IBL Configuration and customization #2155

jturner65 · 2023-07-11T18:53:16Z

Motivation and Context

This PR facilitates the consumption of image-based lighting in PBR-rendered objects and scenes, as well as introducing many customization capabilities for the PBR/IBL shader. These customizations are enabled through the use of a JSON configuration file, using the extension ".pbr_config.json", a default template of which is included in the PR. Among the customizations exposed in this PR are :

The ability to set a direct lighting level value, so that PBR and Phong results can be more easily balanced without modifying lights.
Specifying BRDF lookup table and Environment Map assets to use for image-based lighting beyond those baked into the resources file.
Enable/disable direct lighting and/or image-based lighting.
Enable/disable tonemapping on the direct lighting and/or image-based lighting results
Enable/disable approximate remapping from sRGB<->linear color spaces for textures that are specified to be sRGB in GLTF standard (basecolor texture, emissive texture, specular color texture), so that all calculations can be performed on linear color data.
Modify (TODO : or disable) the shader-based calculation that enables the consumption of normal textures when precomputed tangents are not provided by computing a TBN frame. (Note : without this calculation, no normal textures will be consumed if no precomputed tangents are present, and the visuals will suffer substantially).
Enable/disable specific KHR extension layers (clear coat, specular layer, anisotropy) to see any potential impact on performance.
Specify the level of direct and indirect specular and diffuse contributions in the final rendered color.
Specify tonemap exposure and sRGB remapping gamma values.

A DrawableConfiguration class is also added to serve as the carrier for the various values required by the drawables, including values that are used by one type (i.e. Phong) but not the other (i.e. PBR). This will also make adding, for example, skindata support to the PBR drawables more easily accomplished in the future.

Tests and Python bindings are also a part of this PR for both the PbrShaderAttributes and the PbrShaderAttributesManager. Documentation modifications/additions still WIP.

NOTE : To support some resource functionality requirements, the Corrade package was updated.

Here's a brief overview of how the new PBR/IBL shader support is accomplished :

The metadata setup

The config file format seen here describes the editable configuration values for the PBR shader, including IBL assets and configuration. This json file has the extension "pbr_config.json". These configs are referenced in a Scene Dataset config in the same way as stage and object attributes are (their tag is "pbr_ibl_configs"), and support all the same functionality except that they only allow .json file extensions. They are read into a PbrShaderAttributes object, managed by a PbrShaderAttributesManager. One manager exists for MetadataMediator, and is shared with all SceneDatasets that are loaded (much like PhysicsAttributesManager). Also like PhysicsAttributes, there is a system default PbrShaderAttributes config that is part of this PR, that will always be loaded and available as a fallback. Each Scene Instance specifies (or inherits) a "default_pbr_shader_config", which will be a handle to a PbrShaderAttributes already loaded, as well as an object, "pbr_shader_region_configs", which holds key-value pairs consisting of string ids (called 'regions') pointing to PbrShaderAttributes handles.

The processing

During the reconfigure process, right before we instantiate a stage, a call is made to ResourceManager::loadAllIBLAssets to query all the loaded PbrShaderAttributes in the current Scene Dataset(s), in order to load their requested IBL assets (BRDF lookup table image and Equirectangular Environment map) if they are not already loaded. Once each requested pair of assets are loaded into textures, these are sent to the PbrIBLHelper to derive the IrradianceMap and Prefiltered Environment Map Cubemap objects that will be used by the IBL functionality of the PBR shader. The PbrIBLHelper is in turn stored in a map in ResourceManager, keyed by the file names of the bLUT and the EnvMap. The biggest change from main for this functionality is that all the asset loading from disk/resource file is now happening in ResourceManager, once per asset, and all derived textures are being cached so if they get requested again they will not be reloaded/reprocessed. Also, multiple PbrIBLHelpers are now possible, describing different lighting configurations based on different environment maps. The appropriate PbrIBLHelper is still sent to the drawables, as before, but now as part of a DrawableConfiguration object.

The DrawableConfiguration class

This is currently just a carrier struct to hold the various assets we would like to send to the Drawables, regardless of their specific type (Phong or PBR, for instance). It holds constructs that are shared between the drawables, as well as constructs that are unique to one or the other (like the PbrIBLHelper and the PbrShaderAttributes that is pertinent for the PBR drawable, or the Skindata that is being used by the Phong drawable). It is the responsiblity of the consuming class to access what it does or does not properly use. Future support of PBR skin rendering will be facilitated by this structure.

The Drawable/Shader cpp and fragment shader code.

This process is much like the newly added material handling process - the config values are read from the attributes, flag values are set accordingly, and a small struct holding a few floating point values is instanced to hold uniform values. These values are then passed to the shader if appropriate based on flags settings during each draw. The shader class will specify which shader compiler directives to instantiate based on the flags settings as before, and also processes the various non-boolean values that are served to the fragment shader as uniforms. For it's part, the PBR fragment shader was largely already prepared for these changes.

This PR is considered a "breaking change" only because the IBL flag of SimulatorConfiguration is being deprecated and ignored. Any image-based lighting control for PBR drawables will come from PbrShaderAttributes control from here on.

How Has This Been Tested

All c++ and python tests pass. Potential problem with javascript test, which may require recapture of test image due to changes in the default Phong material colors.

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 · 2023-07-24T16:35:31Z

src/esp/gfx/DrawableConfiguration.h

+ * This class will hold configuration values and utilities for Drawables and the
+ * shaders that they own.
+ */
+class DrawableConfiguration {


This was made a class and not a struct because I foresee functionality being added/moved from RM in the future. I am not, however, against making it a struct if it is felt that that is more appropriate.

jturner65 · 2023-07-24T16:41:30Z

Due to the size of this PR it may make sense to break it into smaller PRs. Open to suggestions here as well.

eundersander

Good progress! I left some comments.

eundersander · 2023-07-24T17:12:38Z

data/default.pbr_config.json

@@ -0,0 +1,22 @@
+{


I know we've had a problem in the past with data files in this folder in the repo, because folks who install via conda won't get this. E.g. they don't get data/default.physics_config.json, so we made sure that the defaults in that file were also the defaults for the class in code.

What will be the process for a Hab-lab user to utilize these PBR options? It would be nice if we had a tutorial or example project that showed users where to put this file and how to reference it (doesn't have to be part of this PR).

I was unaware of this :

E.g. they don't get data/default.physics_config.json

I will make sure that the defaults will match.

The primary mechanism to start will be to modify json configs or directly modify/create shader configs using a similar mechanism to the current attributes. And yes, I completely agree about a tutorial, that is my primary concern now.

eundersander · 2023-07-24T19:41:26Z

src/esp/assets/ResourceManager.cpp

-  // TODO:
-  // should work with the scene instance config, initialize
-  // different PBR IBLs at different positions in the scene.
+std::shared_ptr<Mn::GL::Texture2D> ResourceManager::loadIBLImageIntoTexture(


Could we move this IBL stuff including loadAllIBLAssets into a separate helper class? My motivations:

we may want to re-use this logic with the upcoming batch renderer, which may not instantiate a ResourceManager.

ResourceManager is already bloated with 3000+ lines, so it's helpful for readability, etc. to isolate new code into separate files.

Agree on both counts. Should I put the class in assets?

I defer to you and others on the location.

I am going to suggest we postpone this refactor (moving the IBL management code to an external class from RM) until I get the AO attributes/managers and HBAO functionality completed. As it is I think those two tasks should take most of August.

eundersander · 2023-07-24T19:51:58Z

src/esp/assets/ResourceManager.cpp

+  // desired configuration of the PBR shader.
+  auto mapOfPbrConfigs = metadataMediator_->getAllPbrShaderConfigs();
+
+  // Only load if rendering is enabled.


This comment doesn't seem to match the if (requiresTextures_) { code. E.g. that flag will be false if you're doing depth-only rendering.

I see. I can modify it to be RGB rendering then.

eundersander · 2023-07-24T19:56:15Z

src/esp/assets/ResourceManager.cpp

+          << helperKey << "`";
+      std::shared_ptr<Mn::GL::Texture2D> blutTexture = nullptr;
+      std::shared_ptr<Mn::GL::Texture2D> envMapTexture = nullptr;
+      if (pbrIBLHelpers_.count(helperKey) == 0) {


I see that loadAllIBLAssets is called from Simulator::createSceneInstance, which is called from Simulator::reconfigure ... Are we generally loading these images whenever we reconfigure to a new scene, or just once at startup?

Just once per image - once they are loaded they will be retained unless RM is remade as well.

eundersander · 2023-07-24T19:58:57Z

src/esp/bindings/AttributesBindings.cpp

@@ -429,11 +431,159 @@ void initAttributesBindings(py::module& m) {

  // TODO : LightLayoutAttributes

+  // ==== PbrShaderAttributes ====
+  py::class_<PbrShaderAttributes, AbstractAttributes, PbrShaderAttributes::ptr>(


I'm curious, for constructing PbrShaderAttributes, what's the purpose of having both python bindings and a codepath for constructing these from JSON? Are we just trying to give Hab-lab users the flexibility of two options for specifying PbrShaderAttributes?

I want to give folks the ability to modify their own shader configurations, but I also want to have the ability to include these configurations in datasets (like we do with object, stage and scene instance attributes).

want to preserve the flags when a material is only being modified so that a new shader is not inadvertently created. Material class ref removed since we're using the material cache instead.

…phong->PBR materials Equalizing brightness between PBR and Phong will now take place via PbrConfig settings.

migrating image load functionality out of this class.

pass a configuration structure to the drawables on creation that will be created beforehand and hold appropriate references to various configuration quantities based on the type of drawable.

PbrShaderAttributes needs to be governed by Scene datasets and MetadataMediator, and IBL status is based on config settings now.

--Remove old lythwood image --Add 4 new 1k hdr envmaps

--also provide more expansive srgb <--> linear control --The default IBL image has also been changed to one that is not so gold/orange.

0mdc

Looks good so far! I did a first pass and left some minor comments. I'll have a look at the shader changes tomorrow.

src/tests/AttributesManagersTest.cpp

0mdc · 2023-07-31T22:29:41Z

data/pbr/env_maps/anniversary_lounge_1k.hdr

Should we store these images in an external repository, along with other assets?

The reason I didn't is that we've always included some default IBL assets as resources, and I wanted to provide a bit of a usable variety as compiled resources so folks didn't have to deal with downloading them.

src/esp/assets/ResourceManager.cpp

src/esp/assets/ResourceManager.h

src/esp/bindings/AttributesBindings.cpp

0mdc · 2023-07-31T22:57:33Z

src/esp/gfx/configure.h.cmake

What is the purpose of this file?

src/esp/sim/SimulatorConfiguration.cpp

src/shaders/gfx/equirectangularToCubeMap.frag

--If a user modifies or adds a new PbrShaderConfig with new IBL assets, while RM exists without re-running sim::reconfigure and tries to instantiate an object using this new PbrShaderConfig, those new assets will not have been loaded yet. This change makes it so that if they haven't been, they will be loaded and cached.

0mdc

LGTM!

aclegg3

LGTM, thanks! Love that IBL is now default, visuals are improving greatly!
A couple of minor comments.
Dont' worry about the JS tests, we can follow up with a solution for that.

aclegg3 · 2023-08-04T14:52:58Z

src/esp/bindings/SimBindings.cpp

@@ -102,7 +102,7 @@ void initSimBindings(py::module& m) {
      .def_readwrite(
          "pbr_image_based_lighting",
          &SimulatorConfiguration::pbrImageBasedLighting,
-          R"(Whether or not to enable image based lighting in the PBR shader.)")
+          R"(DEPRECATED : Use PbrShaderAttributes to specify whether IBL is enabled or disabled.)")


We can slate this for full removal in v0.3.0
Let's make a task to search for and remove any downstream uses of this flag in preperation.

Yes, good idea.

aclegg3 · 2023-08-04T15:09:41Z

src/esp/gfx/PbrIBLHelper.cpp

+  // using the brdflut from here:
+  // https://github.com/SaschaWillems/Vulkan-glTF-PBR/blob/master/screenshots/tex_brdflut.png
+
+  // loadBrdfLookUpTable(blutImageData);


reason to keep this around?

Nope, oversight on my part. Thanks for finding it.

aclegg3 · 2023-08-04T15:16:28Z

src/esp/gfx/PbrShader.cpp

+    setTonemapExposure(4.5f);
  }

+  setGamma({2.2f, 2.2f, 2.2f});


constants set empirically?

Actually that's a reference gamma value specified by IEC in the sRGB standard (kinda out of date - was supposed to model CRTs :D but still in use). It is specified as a uniform more as a kind of heavy handed brightness control, really, when using the sRGB mapping.

0mdc · 2023-08-04T18:34:29Z

@aclegg3

Don't worry about the JS tests, we can follow up with a solution for that.

We could probably disable it. We are moving forward with a new web rendering solution using replay rendering instead of simulator rendering.

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 11, 2023

jturner65 force-pushed the IBL_Improvements branch 5 times, most recently from 64f7a18 to 5cba1d5 Compare July 18, 2023 13:31

jturner65 force-pushed the IBL_Improvements branch from 7ea2f72 to 5189535 Compare July 24, 2023 15:27

jturner65 commented Jul 24, 2023

View reviewed changes

jturner65 requested review from 0mdc, aclegg3 and eundersander July 24, 2023 16:40

jturner65 self-assigned this Jul 24, 2023

jturner65 marked this pull request as ready for review July 24, 2023 16:40

jturner65 changed the title ~~--[WIP] PBR/IBL Configuration and customization~~ --PBR/IBL Configuration and customization Jul 24, 2023

eundersander reviewed Jul 24, 2023

View reviewed changes

jturner65 force-pushed the IBL_Improvements branch 4 times, most recently from 0013543 to bc62d8e Compare July 27, 2023 15:11

jturner65 added 10 commits July 28, 2023 11:14

--initial assignment of gamma and exposure as uniforms

f370c16

--minor uniform renaming

5780eff

--pbr/ibl shader attributes and mgr; default shader config

eec44f4

--add PbrShaderAttributesManager to MM

9f458ba

--handle non-reset material spec; remove class ref to material

bd6520f

want to preserve the flags when a material is only being modified so that a new shader is not inadvertently created. Material class ref removed since we're using the material cache instead.

--Remove out-of-date 'hacky scaling' for default phong materials and …

c871208

…phong->PBR materials Equalizing brightness between PBR and Phong will now take place via PbrConfig settings.

--initial modifications of PBR IBL class

6edca6f

migrating image load functionality out of this class.

--move instantiation of IBL functionality to createSceneInstance

e888fd6

--Drawable Configuration

1312def

pass a configuration structure to the drawables on creation that will be created beforehand and hold appropriate references to various configuration quantities based on the type of drawable.

--clang-tidy

8bf711a

jturner65 added 13 commits July 28, 2023 11:14

--remove unused shader config setting, deprecate IBL directive

692902c

PbrShaderAttributes needs to be governed by Scene datasets and MetadataMediator, and IBL status is based on config settings now.

--make sure IBL is only enabled if a PbrIBLHelper exists.

e06bd35

--make IBL asset load more resilient.

1d293d6

--add PBR Shader Attr test (and fix exposed error)

3a602ca

--add attributes manager test for PbrShaderAttributesManager

cc7e84e

--complete bindings coverage for Pbr shader attributes and managers

96dc65c

--remove unused jpg version of lythwood room

a3f7239

--cleanup pbrShaderAttrMgr

fefa39d

--add env maps and reorganize compiled resources.

e662075

--Remove old lythwood image --Add 4 new 1k hdr envmaps

--region-based PbrShaderAttrib queries

36ca42e

--add config prototypes for additional env maps

2502e58

--shader prep for region IBL maps

b2e8463

--Improve gamma and exposure mapping

9fe790b

jturner65 force-pushed the IBL_Improvements branch from bc62d8e to 9fe790b Compare July 28, 2023 15:29

jturner65 added 2 commits July 28, 2023 17:01

--fix issue with IBL results always mapping to sRGB

8ab4ecf

--also provide more expansive srgb <--> linear control --The default IBL image has also been changed to one that is not so gold/orange.

--update test images due to bugfix with IBL

8d2107e

0mdc reviewed Jul 31, 2023

View reviewed changes

jturner65 added 4 commits August 1, 2023 09:40

--reviewer suggestions (spelling)

afe3f17

--minor adjustments

0edec57

--cleanup

5d6d610

0mdc approved these changes Aug 3, 2023

View reviewed changes

aclegg3 approved these changes Aug 4, 2023

View reviewed changes

--reviewer suggestions

64c7c3e

jturner65 merged commit 4d92aed into main Aug 4, 2023
6 of 10 checks passed

jturner65 deleted the IBL_Improvements branch August 4, 2023 19:59

jturner65 restored the IBL_Improvements branch August 8, 2023 15:28

jturner65 deleted the IBL_Improvements branch August 8, 2023 15:29

jturner65 mentioned this pull request Sep 13, 2023

--[Habitat3.0] Removal of out-of-date/deprecated flags for IBL #2203

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

--PBR/IBL Configuration and customization #2155

--PBR/IBL Configuration and customization #2155

jturner65 commented Jul 11, 2023 •

edited

jturner65 Jul 24, 2023 •

edited

jturner65 commented Jul 24, 2023

eundersander left a comment

eundersander Jul 24, 2023

jturner65 Jul 24, 2023

eundersander Jul 24, 2023

jturner65 Jul 24, 2023

eundersander Jul 26, 2023

jturner65 Jul 31, 2023

eundersander Jul 24, 2023

jturner65 Jul 24, 2023

eundersander Jul 24, 2023

jturner65 Jul 24, 2023

eundersander Jul 24, 2023

jturner65 Jul 24, 2023

0mdc left a comment

0mdc Jul 31, 2023

jturner65 Aug 1, 2023

0mdc Jul 31, 2023

0mdc left a comment

aclegg3 left a comment

aclegg3 Aug 4, 2023

jturner65 Aug 4, 2023

aclegg3 Aug 4, 2023

jturner65 Aug 4, 2023

aclegg3 Aug 4, 2023

jturner65 Aug 4, 2023

0mdc commented Aug 4, 2023

--PBR/IBL Configuration and customization #2155

--PBR/IBL Configuration and customization #2155

Conversation

jturner65 commented Jul 11, 2023 • edited

Motivation and Context

The metadata setup

The processing

The Drawable/Shader cpp and fragment shader code.

How Has This Been Tested

Types of changes

Checklist

jturner65 Jul 24, 2023 • edited

Choose a reason for hiding this comment

jturner65 commented Jul 24, 2023

eundersander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0mdc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0mdc left a comment

Choose a reason for hiding this comment

aclegg3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0mdc commented Aug 4, 2023

jturner65 commented Jul 11, 2023 •

edited

jturner65 Jul 24, 2023 •

edited