From 36120106cdfe4610bd09620a5ae6006423e7f928 Mon Sep 17 00:00:00 2001
From: Romain Guy <romainguy@curious-creature.com>
Date: Fri, 23 Feb 2024 14:33:26 -0800
Subject: [PATCH] Add new PBR Neutral tone mapper (#7597)

This tone mapper was designed to preserve the color apperance
of materials. It provides good saturation and contrast while
controlling the highlights.
---
 NEW_RELEASE_NOTES.md                          |  1 +
 .../src/main/cpp/ToneMapper.cpp               |  5 ++
 .../google/android/filament/ToneMapper.java   | 15 +++-
 filament/include/filament/ToneMapper.h        | 20 ++++--
 filament/src/ToneMapper.cpp                   | 68 +++++++++++++------
 libs/viewer/include/viewer/Settings.h         |  3 +-
 libs/viewer/src/Settings.cpp                  | 11 +--
 libs/viewer/src/ViewerGui.cpp                 |  5 +-
 8 files changed, 95 insertions(+), 33 deletions(-)

diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md
index b9947d8a8e..7e1350a537 100644
--- a/NEW_RELEASE_NOTES.md
+++ b/NEW_RELEASE_NOTES.md
@@ -10,3 +10,4 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md).
 
 - android: NDK 26.1.10909125 is used by default
 - android: Minimum API level on Android is now API 21 instead of API 19. This allows the use of OpenGL ES 3.1
+- rendering: New PBR Neutral tone mapper, designed to preserve materials color appearance
diff --git a/android/filament-android/src/main/cpp/ToneMapper.cpp b/android/filament-android/src/main/cpp/ToneMapper.cpp
index 21c7e7ab24..860b95508b 100644
--- a/android/filament-android/src/main/cpp/ToneMapper.cpp
+++ b/android/filament-android/src/main/cpp/ToneMapper.cpp
@@ -47,6 +47,11 @@ Java_com_google_android_filament_ToneMapper_nCreateFilmicToneMapper(JNIEnv*, jcl
     return (jlong) new FilmicToneMapper();
 }
 
+extern "C" JNIEXPORT jlong JNICALL
+Java_com_google_android_filament_ToneMapper_nCreatePBRNeutralToneMapper(JNIEnv*, jclass) {
+    return (jlong) new PBRNeutralToneMapper();
+}
+
 extern "C" JNIEXPORT jlong JNICALL
 Java_com_google_android_filament_ToneMapper_nCreateAgxToneMapper(JNIEnv*, jclass, jint look) {
     return (jlong) new AgxToneMapper(AgxToneMapper::AgxLook(look));
diff --git a/android/filament-android/src/main/java/com/google/android/filament/ToneMapper.java b/android/filament-android/src/main/java/com/google/android/filament/ToneMapper.java
index 58ffc501ec..c3d3490441 100644
--- a/android/filament-android/src/main/java/com/google/android/filament/ToneMapper.java
+++ b/android/filament-android/src/main/java/com/google/android/filament/ToneMapper.java
@@ -16,12 +16,14 @@ package com.google.android.filament;
  * <li>Configurable tone mapping operators</li>
  * <ul>
  *   <li>GenericToneMapper</li>
+ *   <li>AgXToneMapper</li>
  * </ul>
  * <li>Fixed-aesthetic tone mapping operators</li>
  * <ul>
  *   <li>ACESToneMapper</li>
  *   <li>ACESLegacyToneMapper</li>
  *   <li>FilmicToneMapper</li>
+ *   <li>PBRNeutralToneMapper</li>
  * </ul>
  * <li>Debug/validation tone mapping operators</li>
  * <ul>
@@ -100,11 +102,21 @@ public class ToneMapper {
         }
     }
 
+    /**
+     * Khronos PBR Neutral tone mapping operator. This tone mapper was designed
+     * to preserve the appearance of materials across lighting conditions while
+     * avoiding artifacts in the highlights in high dynamic range conditions.
+     */
+    public static class PBRNeutralToneMapper extends ToneMapper {
+        public PBRNeutralToneMapper() {
+            super(nCreatePBRNeutralToneMapper());
+        }
+    }
+
     /**
      * AgX tone mapping operator.
      */
     public static class Agx extends ToneMapper {
-
         public enum AgxLook {
             /**
              * Base contrast with no look applied
@@ -233,6 +245,7 @@ public class ToneMapper {
     private static native long nCreateACESToneMapper();
     private static native long nCreateACESLegacyToneMapper();
     private static native long nCreateFilmicToneMapper();
+    private static native long nCreatePBRNeutralToneMapper();
     private static native long nCreateAgxToneMapper(int look);
     private static native long nCreateGenericToneMapper(
             float contrast, float midGrayIn, float midGrayOut, float hdrMax);
diff --git a/filament/include/filament/ToneMapper.h b/filament/include/filament/ToneMapper.h
index 74e2661438..e89e702c99 100644
--- a/filament/include/filament/ToneMapper.h
+++ b/filament/include/filament/ToneMapper.h
@@ -21,7 +21,7 @@
 
 #include <math/mathfwd.h>
 
-#include <stdint.h>
+#include <cstdint>
 
 namespace filament {
 
@@ -39,10 +39,12 @@ namespace filament {
  *
  * - Configurable tone mapping operators
  *   - GenericToneMapper
+ *   - AgXToneMapper
  * - Fixed-aesthetic tone mapping operators
  *   - ACESToneMapper
  *   - ACESLegacyToneMapper
  *   - FilmicToneMapper
+ *   - PBRNeutralToneMapper
  * - Debug/validation tone mapping operators
  *   - LinearToneMapper
  *   - DisplayRangeToneMapper
@@ -117,11 +119,22 @@ struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper {
     math::float3 operator()(math::float3 x) const noexcept override;
 };
 
+/**
+ * Khronos PBR Neutral tone mapping operator. This tone mapper was designed
+ * to preserve the appearance of materials across lighting conditions while
+ * avoiding artifacts in the highlights in high dynamic range conditions.
+ */
+struct UTILS_PUBLIC PBRNeutralToneMapper final : public ToneMapper {
+    PBRNeutralToneMapper() noexcept;
+    ~PBRNeutralToneMapper() noexcept final;
+
+    math::float3 operator()(math::float3 x) const noexcept override;
+};
+
 /**
  * AgX tone mapping operator.
  */
 struct UTILS_PUBLIC AgxToneMapper final : public ToneMapper {
-
     enum class AgxLook : uint8_t {
         NONE = 0,   //!< Base contrast with no look applied
         PUNCHY,     //!< A punchy and more chroma laden look for sRGB displays
@@ -185,9 +198,6 @@ struct UTILS_PUBLIC GenericToneMapper final : public ToneMapper {
     /** Returns the contrast of the curve as a strictly positive value. */
     float getContrast() const noexcept;
 
-    /** Returns how fast scene referred values map to output white as a value between 0.0 and 1.0. */
-    float getShoulder() const noexcept;
-
     /** Returns the middle gray point for input values as a value between 0.0 and 1.0. */
     float getMidGrayIn() const noexcept;
 
diff --git a/filament/src/ToneMapper.cpp b/filament/src/ToneMapper.cpp
index b24f253e0a..2afa58f4be 100644
--- a/filament/src/ToneMapper.cpp
+++ b/filament/src/ToneMapper.cpp
@@ -230,6 +230,32 @@ float3 FilmicToneMapper::operator()(math::float3 x) const noexcept {
     return (x * (a * x + b)) / (x * (c * x + d) + e);
 }
 
+//------------------------------------------------------------------------------
+// PBR Neutral tone mapper
+//------------------------------------------------------------------------------
+
+DEFAULT_CONSTRUCTORS(PBRNeutralToneMapper)
+
+float3 PBRNeutralToneMapper::operator()(math::float3 color) const noexcept {
+    // PBR Tone Mapping, https://modelviewer.dev/examples/tone-mapping.html
+    constexpr float startCompression = 0.8f - 0.04f;
+    constexpr float desaturation = 0.15f;
+
+    float x = min(color.r, min(color.g, color.b));
+    float offset = x < 0.08f ? x - 6.25f * x * x : 0.04f;
+    color -= offset;
+
+    float peak = max(color.r, max(color.g, color.b));
+    if (peak < startCompression) return color;
+
+    float d = 1.0f - startCompression;
+    float newPeak = 1.0f - d * d / (peak + d - startCompression);
+    color *= newPeak / peak;
+
+    float g = 1.0f - 1.0f / (desaturation * (peak - newPeak) + 1.0f);
+    return mix(color, float3(1.0f), g);
+}
+
 //------------------------------------------------------------------------------
 // AgX tone mapper
 //------------------------------------------------------------------------------
@@ -262,14 +288,14 @@ float3 agxDefaultContrastApprox(float3 x) {
     float3 x2 = x * x;
     float3 x4 = x2 * x2;
     float3 x6 = x4 * x2;
-    return  - 17.86     * x6 * x
-            + 78.01     * x6
-            - 126.7     * x4 * x
-            + 92.06     * x4
-            - 28.72     * x2 * x
-            + 4.361     * x2
-            - 0.1718    * x
-            + 0.002857;
+    return  - 17.86f    * x6 * x
+            + 78.01f    * x6
+            - 126.7f    * x4 * x
+            + 92.06f    * x4
+            - 28.72f    * x2 * x
+            + 4.361f    * x2
+            - 0.1718f   * x
+            + 0.002857f;
 }
 
 // Adapted from https://iolite-engine.com/blog_posts/minimal_agx_implementation
@@ -278,23 +304,23 @@ float3 agxLook(float3 val, AgxToneMapper::AgxLook look) {
         return val;
     }
 
-    const float3 lw = float3(0.2126, 0.7152, 0.0722);
+    const float3 lw = float3(0.2126f, 0.7152f, 0.0722f);
     float luma = dot(val, lw);
 
     // Default
-    float3 offset = float3(0.0);
-    float3 slope = float3(1.0);
-    float3 power = float3(1.0);
-    float sat = 1.0;
+    float3 offset = float3(0.0f);
+    float3 slope = float3(1.0f);
+    float3 power = float3(1.0f);
+    float sat = 1.0f;
 
     if (look == AgxToneMapper::AgxLook::GOLDEN) {
-        slope = float3(1.0, 0.9, 0.5);
-        power = float3(0.8);
+        slope = float3(1.0f, 0.9f, 0.5f);
+        power = float3(0.8f);
         sat = 1.3;
     }
     if (look == AgxToneMapper::AgxLook::PUNCHY) {
-        slope = float3(1.0);
-        power = float3(1.35, 1.35, 1.35);
+        slope = float3(1.0f);
+        power = float3(1.35f, 1.35f, 1.35f);
         sat = 1.4;
     }
 
@@ -305,16 +331,16 @@ float3 agxLook(float3 val, AgxToneMapper::AgxLook look) {
 
 float3 AgxToneMapper::operator()(float3 v) const noexcept {
     // Ensure no negative values
-    v = max(float3(0.0), v);
+    v = max(float3(0.0f), v);
 
     v = AgXInsetMatrix * v;
 
     // Log2 encoding
-    v = max(v, 1E-10); // avoid 0 or negative numbers for log2
+    v = max(v, 1E-10f); // avoid 0 or negative numbers for log2
     v = log2(v);
     v = (v - AgxMinEv) / (AgxMaxEv - AgxMinEv);
 
-    v = clamp(v, 0, 1);
+    v = clamp(v, 0.0f, 1.0f);
 
     // Apply sigmoid
     v = agxDefaultContrastApprox(v);
@@ -325,7 +351,7 @@ float3 AgxToneMapper::operator()(float3 v) const noexcept {
     v = AgXOutsetMatrix * v;
 
     // Linearize
-    v = pow(max(float3(0.0), v), 2.2);
+    v = pow(max(float3(0.0f), v), 2.2f);
 
     return v;
 }
diff --git a/libs/viewer/include/viewer/Settings.h b/libs/viewer/include/viewer/Settings.h
index 6682260c3e..2094296bd7 100644
--- a/libs/viewer/include/viewer/Settings.h
+++ b/libs/viewer/include/viewer/Settings.h
@@ -59,7 +59,8 @@ enum class ToneMapping : uint8_t {
     FILMIC        = 3,
     AGX           = 4,
     GENERIC       = 5,
-    DISPLAY_RANGE = 6,
+    PBR_NEUTRAL   = 6,
+    DISPLAY_RANGE = 7,
 };
 
 using AmbientOcclusionOptions = filament::View::AmbientOcclusionOptions;
diff --git a/libs/viewer/src/Settings.cpp b/libs/viewer/src/Settings.cpp
index 453f9beccc..639d07a2c9 100644
--- a/libs/viewer/src/Settings.cpp
+++ b/libs/viewer/src/Settings.cpp
@@ -88,6 +88,7 @@ static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, ToneMapp
     else if (0 == compare(tokens[i], jsonChunk, "FILMIC")) { *out = ToneMapping::FILMIC; }
     else if (0 == compare(tokens[i], jsonChunk, "AGX")) { *out = ToneMapping::AGX; }
     else if (0 == compare(tokens[i], jsonChunk, "GENERIC")) { *out = ToneMapping::GENERIC; }
+    else if (0 == compare(tokens[i], jsonChunk, "PBR_NEUTRAL")) { *out = ToneMapping::PBR_NEUTRAL; }
     else if (0 == compare(tokens[i], jsonChunk, "DISPLAY_RANGE")) { *out = ToneMapping::DISPLAY_RANGE; }
     else {
         slog.w << "Invalid ToneMapping: '" << STR(tokens[i], jsonChunk) << "'" << io::endl;
@@ -679,11 +680,12 @@ constexpr ToneMapper* createToneMapper(const ColorGradingSettings& settings) noe
         case ToneMapping::FILMIC: return new FilmicToneMapper;
         case ToneMapping::AGX: return new AgxToneMapper(settings.agxToneMapper.look);
         case ToneMapping::GENERIC: return new GenericToneMapper(
-                    settings.genericToneMapper.contrast,
-                    settings.genericToneMapper.midGrayIn,
-                    settings.genericToneMapper.midGrayOut,
-                    settings.genericToneMapper.hdrMax
+                settings.genericToneMapper.contrast,
+                settings.genericToneMapper.midGrayIn,
+                settings.genericToneMapper.midGrayOut,
+                settings.genericToneMapper.hdrMax
         );
+        case ToneMapping::PBR_NEUTRAL: return new PBRNeutralToneMapper;
         case ToneMapping::DISPLAY_RANGE: return new DisplayRangeToneMapper;
     }
 }
@@ -734,6 +736,7 @@ static std::ostream& operator<<(std::ostream& out, ToneMapping in) {
         case ToneMapping::FILMIC: return out << "\"FILMIC\"";
         case ToneMapping::AGX: return out << "\"AGX\"";
         case ToneMapping::GENERIC: return out << "\"GENERIC\"";
+        case ToneMapping::PBR_NEUTRAL: return out << "\"PBR_NEUTRAL\"";
         case ToneMapping::DISPLAY_RANGE: return out << "\"DISPLAY_RANGE\"";
     }
     return out << "\"INVALID\"";
diff --git a/libs/viewer/src/ViewerGui.cpp b/libs/viewer/src/ViewerGui.cpp
index 9d352e3600..216288d3f8 100644
--- a/libs/viewer/src/ViewerGui.cpp
+++ b/libs/viewer/src/ViewerGui.cpp
@@ -145,6 +145,9 @@ static void computeToneMapPlot(ColorGradingSettings& settings, float* plot) {
             );
             hdrMax = settings.genericToneMapper.hdrMax;
             break;
+        case ToneMapping::PBR_NEUTRAL:
+            mapper = new PBRNeutralToneMapper;
+            break;
         case ToneMapping::DISPLAY_RANGE:
             mapper = new DisplayRangeToneMapper;
             break;
@@ -196,7 +199,7 @@ static void colorGradingUI(Settings& settings, float* rangePlot, float* curvePlo
 
         int toneMapping = (int) colorGrading.toneMapping;
         ImGui::Combo("Tone-mapping", &toneMapping,
-                "Linear\0ACES (legacy)\0ACES\0Filmic\0AgX\0Generic\0Display Range\0\0");
+                "Linear\0ACES (legacy)\0ACES\0Filmic\0AgX\0Generic\0PBR Neutral\0Display Range\0\0");
         colorGrading.toneMapping = (decltype(colorGrading.toneMapping)) toneMapping;
         if (colorGrading.toneMapping == ToneMapping::GENERIC) {
             if (ImGui::CollapsingHeader("Tonemap parameters")) {