From 9b4a418bcb23324e413fde7c6daa16f7a6028781 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Thu, 5 Oct 2023 11:39:57 -0700 Subject: [PATCH 1/5] Add AgX tonemapper --- NEW_RELEASE_NOTES.md | 1 + filament/include/filament/ToneMapper.h | 7 +++ filament/src/ToneMapper.cpp | 67 ++++++++++++++++++++++++++ libs/viewer/include/viewer/Settings.h | 5 +- libs/viewer/src/Settings.cpp | 3 ++ libs/viewer/src/ViewerGui.cpp | 5 +- 6 files changed, 85 insertions(+), 3 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 5b74000c762..10bbb922065 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -10,3 +10,4 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - engine: Added parameter for configuring JobSystem thread count - engine: In Java, introduce Engine.Builder +- engine: New tone mapper: `AgXTonemapper`. diff --git a/filament/include/filament/ToneMapper.h b/filament/include/filament/ToneMapper.h index 8d19c59756b..4ec532f5887 100644 --- a/filament/include/filament/ToneMapper.h +++ b/filament/include/filament/ToneMapper.h @@ -115,6 +115,13 @@ struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper { math::float3 operator()(math::float3 x) const noexcept override; }; +struct UTILS_PUBLIC AgxToneMapper final : public ToneMapper { + AgxToneMapper() noexcept; + ~AgxToneMapper() noexcept final; + + math::float3 operator()(math::float3 x) const noexcept override; +}; + /** * Generic tone mapping operator that gives control over the tone mapping * curve. This operator can be used to control the aesthetics of the final diff --git a/filament/src/ToneMapper.cpp b/filament/src/ToneMapper.cpp index d3dcdd698af..040650561cc 100644 --- a/filament/src/ToneMapper.cpp +++ b/filament/src/ToneMapper.cpp @@ -230,6 +230,73 @@ float3 FilmicToneMapper::operator()(math::float3 x) const noexcept { return (x * (a * x + b)) / (x * (c * x + d) + e); } +//------------------------------------------------------------------------------ +// AgX tone mapper +//------------------------------------------------------------------------------ + +DEFAULT_CONSTRUCTORS(AgxToneMapper) + +// Computed using the default configuration from: +// https://github.com/sobotka/SB2383-Configuration-Generation +// primaries_rotate = [4.5, -0.5, -2.0] +// primaries_scale = [0.15, 0.1, 0.1] +constexpr mat3f AgXInsetMatrix { + 0.9135008, 0.08087188, 0.03593972, + 0.04474362, 0.81547429, 0.0342846, + 0.04175558, 0.10365383, 0.92977567 +}; + +constexpr mat3f AgxOutsetMatrix { inverse(AgXInsetMatrix) }; + +// LOG2_MIN = -10.0 +// LOG2_MAX = +6.5 +// MIDDLE_GRAY = 0.18 +const float AgxMinEv = -12.47393f; // log2(pow(2, LOG2_MIN) * MIDDLE_GRAY) +const float AgxMaxEv = 4.026069f; // log2(pow(2, LOG2_MAX) * MIDDLE_GRAY) + +// Adapted from https://iolite-engine.com/blog_posts/minimal_agx_implementation +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; +} + +float3 AgxToneMapper::operator()(float3 v) const noexcept { + // TODO: It's unclear if we need to temporarily transform to 709 primaries again. + // The original AgX inset matrix assumes 709 primaries, but Blender's implementation of AgX + // keeps the color in Rec.2020. + // v = Rec2020_to_sRGB * v; + + v = AgXInsetMatrix * v; + + // Log2 encoding + v = max(v, 1E-10); // avoid 0 or negative numbers for log2 + v = log2(v); + v = (v - AgxMinEv) / (AgxMaxEv - AgxMinEv); + + v = clamp(v, 0, 1); + + // Apply sigmoid + v = agxDefaultContrastApprox(v); + + // Linearize + v = pow(v, 2.2); + + v = AgxOutsetMatrix * v; + + // v = sRGB_to_Rec2020 * v; + + return v; +} + //------------------------------------------------------------------------------ // Display range tone mapper //------------------------------------------------------------------------------ diff --git a/libs/viewer/include/viewer/Settings.h b/libs/viewer/include/viewer/Settings.h index eeb62e2a784..8ebc38763f0 100644 --- a/libs/viewer/include/viewer/Settings.h +++ b/libs/viewer/include/viewer/Settings.h @@ -57,8 +57,9 @@ enum class ToneMapping : uint8_t { ACES_LEGACY = 1, ACES = 2, FILMIC = 3, - GENERIC = 4, - DISPLAY_RANGE = 5, + AGX = 4, + GENERIC = 5, + DISPLAY_RANGE = 6, }; using AmbientOcclusionOptions = filament::View::AmbientOcclusionOptions; diff --git a/libs/viewer/src/Settings.cpp b/libs/viewer/src/Settings.cpp index f416fdd43a9..3ebb8d2d848 100644 --- a/libs/viewer/src/Settings.cpp +++ b/libs/viewer/src/Settings.cpp @@ -86,6 +86,7 @@ static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, ToneMapp else if (0 == compare(tokens[i], jsonChunk, "ACES_LEGACY")) { *out = ToneMapping::ACES_LEGACY; } else if (0 == compare(tokens[i], jsonChunk, "ACES")) { *out = ToneMapping::ACES; } 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, "DISPLAY_RANGE")) { *out = ToneMapping::DISPLAY_RANGE; } else { @@ -619,6 +620,7 @@ constexpr ToneMapper* createToneMapper(const ColorGradingSettings& settings) noe case ToneMapping::ACES_LEGACY: return new ACESLegacyToneMapper; case ToneMapping::ACES: return new ACESToneMapper; case ToneMapping::FILMIC: return new FilmicToneMapper; + case ToneMapping::AGX: return new AgxToneMapper; case ToneMapping::GENERIC: return new GenericToneMapper( settings.genericToneMapper.contrast, settings.genericToneMapper.midGrayIn, @@ -673,6 +675,7 @@ static std::ostream& operator<<(std::ostream& out, ToneMapping in) { case ToneMapping::ACES_LEGACY: return out << "\"ACES_LEGACY\""; case ToneMapping::ACES: return out << "\"ACES\""; case ToneMapping::FILMIC: return out << "\"FILMIC\""; + case ToneMapping::AGX: return out << "\"AGX\""; case ToneMapping::GENERIC: return out << "\"GENERIC\""; case ToneMapping::DISPLAY_RANGE: return out << "\"DISPLAY_RANGE\""; } diff --git a/libs/viewer/src/ViewerGui.cpp b/libs/viewer/src/ViewerGui.cpp index 4a94c4afac8..ff6f29f0932 100644 --- a/libs/viewer/src/ViewerGui.cpp +++ b/libs/viewer/src/ViewerGui.cpp @@ -133,6 +133,9 @@ static void computeToneMapPlot(ColorGradingSettings& settings, float* plot) { case ToneMapping::FILMIC: mapper = new FilmicToneMapper; break; + case ToneMapping::AGX: + mapper = new AgxToneMapper; + break; case ToneMapping::GENERIC: mapper = new GenericToneMapper( settings.genericToneMapper.contrast, @@ -193,7 +196,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\0Generic\0Display Range\0\0"); + "Linear\0ACES (legacy)\0ACES\0Filmic\0AgX\0Generic\0Display Range\0\0"); colorGrading.toneMapping = (decltype(colorGrading.toneMapping)) toneMapping; if (colorGrading.toneMapping == ToneMapping::GENERIC) { if (ImGui::CollapsingHeader("Tonemap parameters")) { From 5daa4b829251f101942ccf86cd27de5314efd52d Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Thu, 5 Oct 2023 22:44:36 -0700 Subject: [PATCH 2/5] Add Java support --- android/filament-android/src/main/cpp/ToneMapper.cpp | 5 +++++ .../java/com/google/android/filament/ToneMapper.java | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/android/filament-android/src/main/cpp/ToneMapper.cpp b/android/filament-android/src/main/cpp/ToneMapper.cpp index ea040538cd7..ebc1649a00e 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_nCreateAgxToneMapper(JNIEnv*, jclass) { + return (jlong) new AgxToneMapper(); +} + extern "C" JNIEXPORT jlong JNICALL Java_com_google_android_filament_ToneMapper_nCreateGenericToneMapper(JNIEnv*, jclass, jfloat contrast, jfloat midGrayIn, jfloat midGrayOut, jfloat hdrMax) { 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 15800562e3e..bc60d423b42 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 @@ -100,6 +100,15 @@ public Filmic() { } } + /** + * AgX tone mapping operator. + */ + public static class Agx extends ToneMapper { + public Agx() { + super(nCreateAgxToneMapper()); + } + } + /** * Generic tone mapping operator that gives control over the tone mapping * curve. This operator can be used to control the aesthetics of the final @@ -194,6 +203,7 @@ public void setHdrMax(float hdrMax) { private static native long nCreateACESToneMapper(); private static native long nCreateACESLegacyToneMapper(); private static native long nCreateFilmicToneMapper(); + private static native long nCreateAgxToneMapper(); private static native long nCreateGenericToneMapper( float contrast, float midGrayIn, float midGrayOut, float hdrMax); From 5f85dcfb5667d237c5631f11375f318c1e667970 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Fri, 6 Oct 2023 13:44:31 -0700 Subject: [PATCH 3/5] Switch to Blender matrices, add AgX look --- filament/include/filament/ToneMapper.h | 11 +++- filament/src/ToneMapper.cpp | 69 +++++++++++++++++++------- libs/viewer/include/viewer/Settings.h | 12 +++-- libs/viewer/src/Settings.cpp | 12 +++-- libs/viewer/src/ViewerGui.cpp | 7 ++- 5 files changed, 85 insertions(+), 26 deletions(-) diff --git a/filament/include/filament/ToneMapper.h b/filament/include/filament/ToneMapper.h index 4ec532f5887..068c9b6dc3d 100644 --- a/filament/include/filament/ToneMapper.h +++ b/filament/include/filament/ToneMapper.h @@ -116,10 +116,19 @@ struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper { }; struct UTILS_PUBLIC AgxToneMapper final : public ToneMapper { - AgxToneMapper() noexcept; + + enum class AgxLook : uint8_t { + NONE = 0, //!< Base contrast with no look applied + PUNCHY, //!< A punchy and more chroma laden look for sRGB displays + GOLDEN //!< A golden tinted, slightly washed look for BT.1886 displays + }; + + explicit AgxToneMapper(AgxLook look = AgxLook::NONE) noexcept; ~AgxToneMapper() noexcept final; math::float3 operator()(math::float3 x) const noexcept override; + + AgxLook look; }; /** diff --git a/filament/src/ToneMapper.cpp b/filament/src/ToneMapper.cpp index 040650561cc..b24f253e0a1 100644 --- a/filament/src/ToneMapper.cpp +++ b/filament/src/ToneMapper.cpp @@ -234,19 +234,22 @@ float3 FilmicToneMapper::operator()(math::float3 x) const noexcept { // AgX tone mapper //------------------------------------------------------------------------------ -DEFAULT_CONSTRUCTORS(AgxToneMapper) +AgxToneMapper::AgxToneMapper(AgxToneMapper::AgxLook look) noexcept : look(look) {} +AgxToneMapper::~AgxToneMapper() noexcept = default; -// Computed using the default configuration from: -// https://github.com/sobotka/SB2383-Configuration-Generation -// primaries_rotate = [4.5, -0.5, -2.0] -// primaries_scale = [0.15, 0.1, 0.1] +// These matrices taken from Blender's implementation of AgX, which works with Rec.2020 primaries. +// https://github.com/EaryChow/AgX_LUT_Gen/blob/main/AgXBaseRec2020.py constexpr mat3f AgXInsetMatrix { - 0.9135008, 0.08087188, 0.03593972, - 0.04474362, 0.81547429, 0.0342846, - 0.04175558, 0.10365383, 0.92977567 + 0.856627153315983, 0.137318972929847, 0.11189821299995, + 0.0951212405381588, 0.761241990602591, 0.0767994186031903, + 0.0482516061458583, 0.101439036467562, 0.811302368396859 }; - -constexpr mat3f AgxOutsetMatrix { inverse(AgXInsetMatrix) }; +constexpr mat3f AgXOutsetMatrixInv { + 0.899796955911611, 0.11142098895748, 0.11142098895748, + 0.0871996192028351, 0.875575586156966, 0.0871996192028349, + 0.013003424885555, 0.0130034248855548, 0.801379391839686 +}; +constexpr mat3f AgXOutsetMatrix { inverse(AgXOutsetMatrixInv) }; // LOG2_MIN = -10.0 // LOG2_MAX = +6.5 @@ -269,11 +272,40 @@ float3 agxDefaultContrastApprox(float3 x) { + 0.002857; } +// Adapted from https://iolite-engine.com/blog_posts/minimal_agx_implementation +float3 agxLook(float3 val, AgxToneMapper::AgxLook look) { + if (look == AgxToneMapper::AgxLook::NONE) { + return val; + } + + const float3 lw = float3(0.2126, 0.7152, 0.0722); + 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; + + if (look == AgxToneMapper::AgxLook::GOLDEN) { + slope = float3(1.0, 0.9, 0.5); + power = float3(0.8); + sat = 1.3; + } + if (look == AgxToneMapper::AgxLook::PUNCHY) { + slope = float3(1.0); + power = float3(1.35, 1.35, 1.35); + sat = 1.4; + } + + // ASC CDL + val = pow(val * slope + offset, power); + return luma + sat * (val - luma); +} + float3 AgxToneMapper::operator()(float3 v) const noexcept { - // TODO: It's unclear if we need to temporarily transform to 709 primaries again. - // The original AgX inset matrix assumes 709 primaries, but Blender's implementation of AgX - // keeps the color in Rec.2020. - // v = Rec2020_to_sRGB * v; + // Ensure no negative values + v = max(float3(0.0), v); v = AgXInsetMatrix * v; @@ -287,12 +319,13 @@ float3 AgxToneMapper::operator()(float3 v) const noexcept { // Apply sigmoid v = agxDefaultContrastApprox(v); - // Linearize - v = pow(v, 2.2); + // Apply AgX look + v = agxLook(v, look); - v = AgxOutsetMatrix * v; + v = AgXOutsetMatrix * v; - // v = sRGB_to_Rec2020 * v; + // Linearize + v = pow(max(float3(0.0), v), 2.2); return v; } diff --git a/libs/viewer/include/viewer/Settings.h b/libs/viewer/include/viewer/Settings.h index 8ebc38763f0..fc46d2887ed 100644 --- a/libs/viewer/include/viewer/Settings.h +++ b/libs/viewer/include/viewer/Settings.h @@ -115,8 +115,14 @@ struct GenericToneMapperSettings { float midGrayIn = 0.18f; float midGrayOut = 0.215f; float hdrMax = 10.0f; - bool operator!=(const GenericToneMapperSettings &rhs) const { return !(rhs == *this); } - bool operator==(const GenericToneMapperSettings &rhs) const; + bool operator!=(const GenericToneMapperSettings& rhs) const { return !(rhs == *this); } + bool operator==(const GenericToneMapperSettings& rhs) const; +}; + +struct AgxToneMapperSettings { + AgxToneMapper::AgxLook look = AgxToneMapper::AgxLook::NONE; + bool operator!=(const AgxToneMapperSettings& rhs) const { return !(rhs == *this); } + bool operator==(const AgxToneMapperSettings& rhs) const; }; struct ColorGradingSettings { @@ -128,7 +134,7 @@ struct ColorGradingSettings { filament::ColorGrading::QualityLevel quality = filament::ColorGrading::QualityLevel::MEDIUM; ToneMapping toneMapping = ToneMapping::ACES_LEGACY; bool padding0{}; - bool padding1{}; + AgxToneMapperSettings agxToneMapper; color::ColorSpace colorspace = Rec709-sRGB-D65; GenericToneMapperSettings genericToneMapper; math::float4 shadows{1.0f, 1.0f, 1.0f, 0.0f}; diff --git a/libs/viewer/src/Settings.cpp b/libs/viewer/src/Settings.cpp index 3ebb8d2d848..1d0ac0b348f 100644 --- a/libs/viewer/src/Settings.cpp +++ b/libs/viewer/src/Settings.cpp @@ -620,7 +620,7 @@ constexpr ToneMapper* createToneMapper(const ColorGradingSettings& settings) noe case ToneMapping::ACES_LEGACY: return new ACESLegacyToneMapper; case ToneMapping::ACES: return new ACESToneMapper; case ToneMapping::FILMIC: return new FilmicToneMapper; - case ToneMapping::AGX: return new AgxToneMapper; + case ToneMapping::AGX: return new AgxToneMapper(settings.agxToneMapper.look); case ToneMapping::GENERIC: return new GenericToneMapper( settings.genericToneMapper.contrast, settings.genericToneMapper.midGrayIn, @@ -857,7 +857,7 @@ static std::ostream& operator<<(std::ostream& out, const Settings& in) { << "}"; } -bool GenericToneMapperSettings::operator==(const GenericToneMapperSettings &rhs) const { +bool GenericToneMapperSettings::operator==(const GenericToneMapperSettings& rhs) const { static_assert(sizeof(GenericToneMapperSettings) == 16, "Please update Settings.cpp"); return contrast == rhs.contrast && midGrayIn == rhs.midGrayIn && @@ -865,7 +865,12 @@ bool GenericToneMapperSettings::operator==(const GenericToneMapperSettings &rhs) hdrMax == rhs.hdrMax; } -bool ColorGradingSettings::operator==(const ColorGradingSettings &rhs) const { +bool AgxToneMapperSettings::operator==(const AgxToneMapperSettings& rhs) const { + static_assert(sizeof(AgxToneMapperSettings) == 1, "Please update Settings.cpp"); + return look == rhs.look; +} + +bool ColorGradingSettings::operator==(const ColorGradingSettings& rhs) const { // If you had to fix the following codeline, then you likely also need to update the // implementation of operator==. static_assert(sizeof(ColorGradingSettings) == 312, "Please update Settings.cpp"); @@ -874,6 +879,7 @@ bool ColorGradingSettings::operator==(const ColorGradingSettings &rhs) const { quality == rhs.quality && toneMapping == rhs.toneMapping && genericToneMapper == rhs.genericToneMapper && + agxToneMapper == rhs.agxToneMapper && luminanceScaling == rhs.luminanceScaling && gamutMapping == rhs.gamutMapping && exposure == rhs.exposure && diff --git a/libs/viewer/src/ViewerGui.cpp b/libs/viewer/src/ViewerGui.cpp index ff6f29f0932..11443516209 100644 --- a/libs/viewer/src/ViewerGui.cpp +++ b/libs/viewer/src/ViewerGui.cpp @@ -134,7 +134,7 @@ static void computeToneMapPlot(ColorGradingSettings& settings, float* plot) { mapper = new FilmicToneMapper; break; case ToneMapping::AGX: - mapper = new AgxToneMapper; + mapper = new AgxToneMapper(settings.agxToneMapper.look); break; case ToneMapping::GENERIC: mapper = new GenericToneMapper( @@ -207,6 +207,11 @@ static void colorGradingUI(Settings& settings, float* rangePlot, float* curvePlo ImGui::SliderFloat("HDR max", &generic.hdrMax, 1.0f, 64.0f); } } + if (colorGrading.toneMapping == ToneMapping::AGX) { + int agxLook = (int) colorGrading.agxToneMapper.look; + ImGui::Combo("AgX Look", &agxLook, "None\0Punchy\0Golden\0\0"); + colorGrading.agxToneMapper.look = (decltype(colorGrading.agxToneMapper.look)) agxLook; + } computeToneMapPlot(colorGrading, toneMapPlot); From 24a3773784fb852e172615d8868bb0d59232bd87 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 10 Oct 2023 22:59:09 -0700 Subject: [PATCH 4/5] Add Android support for look --- .../src/main/cpp/ToneMapper.cpp | 4 +- .../google/android/filament/ToneMapper.java | 15 +++++- libs/viewer/src/Settings.cpp | 48 +++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/android/filament-android/src/main/cpp/ToneMapper.cpp b/android/filament-android/src/main/cpp/ToneMapper.cpp index ebc1649a00e..21c7e7ab24d 100644 --- a/android/filament-android/src/main/cpp/ToneMapper.cpp +++ b/android/filament-android/src/main/cpp/ToneMapper.cpp @@ -48,8 +48,8 @@ Java_com_google_android_filament_ToneMapper_nCreateFilmicToneMapper(JNIEnv*, jcl } extern "C" JNIEXPORT jlong JNICALL -Java_com_google_android_filament_ToneMapper_nCreateAgxToneMapper(JNIEnv*, jclass) { - return (jlong) new AgxToneMapper(); +Java_com_google_android_filament_ToneMapper_nCreateAgxToneMapper(JNIEnv*, jclass, jint look) { + return (jlong) new AgxToneMapper(AgxToneMapper::AgxLook(look)); } extern "C" JNIEXPORT jlong JNICALL 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 bc60d423b42..1a2936af882 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 @@ -104,8 +104,19 @@ public Filmic() { * AgX tone mapping operator. */ public static class Agx extends ToneMapper { + + public enum AgxLook { + NONE, + PUNCHY, + GOLDEN + } + public Agx() { - super(nCreateAgxToneMapper()); + this(AgxLook.NONE); + } + + public Agx(AgxLook look) { + super(nCreateAgxToneMapper(look.ordinal())); } } @@ -203,7 +214,7 @@ public void setHdrMax(float hdrMax) { private static native long nCreateACESToneMapper(); private static native long nCreateACESLegacyToneMapper(); private static native long nCreateFilmicToneMapper(); - private static native long nCreateAgxToneMapper(); + private static native long nCreateAgxToneMapper(int look); private static native long nCreateGenericToneMapper( float contrast, float midGrayIn, float midGrayOut, float hdrMax); diff --git a/libs/viewer/src/Settings.cpp b/libs/viewer/src/Settings.cpp index 1d0ac0b348f..22b64588b09 100644 --- a/libs/viewer/src/Settings.cpp +++ b/libs/viewer/src/Settings.cpp @@ -131,6 +131,36 @@ static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, GenericT return i; } +static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, AgxToneMapper::AgxLook* out) { + if (0 == compare(tokens[i], jsonChunk, "NONE")) { *out = AgxToneMapper::AgxLook::NONE; } + else if (0 == compare(tokens[i], jsonChunk, "PUNCHY")) { *out = AgxToneMapper::AgxLook::PUNCHY; } + else if (0 == compare(tokens[i], jsonChunk, "GOLDEN")) { *out = AgxToneMapper::AgxLook::GOLDEN; } + else { + slog.w << "Invalid AgxLook: '" << STR(tokens[i], jsonChunk) << "'" << io::endl; + } + return i + 1; +} + +static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, AgxToneMapperSettings* out) { + CHECK_TOKTYPE(tokens[i], JSMN_OBJECT); + int size = tokens[i++].size; + for (int j = 0; j < size; ++j) { + const jsmntok_t tok = tokens[i]; + CHECK_KEY(tok); + if (compare(tok, jsonChunk, "look") == 0) { + i = parse(tokens, i + 1, jsonChunk, &out->look); + } else { + slog.w << "Invalid AgX tone mapper key: '" << STR(tok, jsonChunk) << "'" << io::endl; + i = parse(tokens, i + 1); + } + if (i < 0) { + slog.e << "Invalid AgX tone mapper value: '" << STR(tok, jsonChunk) << "'" << io::endl; + return i; + } + } + return i; +} + static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, ColorGradingSettings* out) { CHECK_TOKTYPE(tokens[i], JSMN_OBJECT); int size = tokens[i++].size; @@ -147,6 +177,8 @@ static int parse(jsmntok_t const* tokens, int i, const char* jsonChunk, ColorGra i = parse(tokens, i + 1, jsonChunk, &out->toneMapping); } else if (compare(tok, jsonChunk, "genericToneMapper") == 0) { i = parse(tokens, i + 1, jsonChunk, &out->genericToneMapper); + } else if (compare(tok, jsonChunk, "agxToneMapper") == 0) { + i = parse(tokens, i + 1, jsonChunk, &out->agxToneMapper); } else if (compare(tok, jsonChunk, "luminanceScaling") == 0) { i = parse(tokens, i + 1, jsonChunk, &out->luminanceScaling); } else if (compare(tok, jsonChunk, "gamutMapping") == 0) { @@ -691,6 +723,21 @@ static std::ostream& operator<<(std::ostream& out, const GenericToneMapperSettin << "}"; } +static std::ostream& operator<<(std::ostream& out, AgxToneMapper::AgxLook in) { + switch (in) { + case AgxToneMapper::AgxLook::NONE: return out << "\"NONE\""; + case AgxToneMapper::AgxLook::PUNCHY: return out << "\"PUNCHY\""; + case AgxToneMapper::AgxLook::GOLDEN: return out << "\"GOLDEN\""; + } + return out << "\"INVALID\""; +} + +static std::ostream& operator<<(std::ostream& out, const AgxToneMapperSettings& in) { + return out << "{\n" + << "\"look\": " << (in.look) << ",\n" + << "}"; +} + static std::ostream& operator<<(std::ostream& out, const ColorGradingSettings& in) { return out << "{\n" << "\"enabled\": " << to_string(in.enabled) << ",\n" @@ -698,6 +745,7 @@ static std::ostream& operator<<(std::ostream& out, const ColorGradingSettings& i << "\"quality\": " << (in.quality) << ",\n" << "\"toneMapping\": " << (in.toneMapping) << ",\n" << "\"genericToneMapper\": " << (in.genericToneMapper) << ",\n" + << "\"agxToneMapper\": " << (in.agxToneMapper) << ",\n" << "\"luminanceScaling\": " << to_string(in.luminanceScaling) << ",\n" << "\"gamutMapping\": " << to_string(in.gamutMapping) << ",\n" << "\"exposure\": " << (in.exposure) << ",\n" From d31a5b26c8dba810b28c281d3ea4642371cd7eda Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 10 Oct 2023 23:06:51 -0700 Subject: [PATCH 5/5] More documentation --- .../google/android/filament/ToneMapper.java | 19 +++++++++++++++++++ filament/include/filament/ToneMapper.h | 8 ++++++++ 2 files changed, 27 insertions(+) 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 1a2936af882..58ffc501ec1 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 @@ -106,15 +106,34 @@ public Filmic() { public static class Agx extends ToneMapper { public enum AgxLook { + /** + * Base contrast with no look applied + */ NONE, + + /** + * A punchy and more chroma laden look for sRGB displays + */ PUNCHY, + + /** + * A golden tinted, slightly washed look for BT.1886 displays + */ GOLDEN } + /** + * Builds a new AgX tone mapper with no look applied. + */ public Agx() { this(AgxLook.NONE); } + /** + * Builds a new AgX tone mapper. + * + * @param look: an optional creative adjustment to contrast and saturation + */ public Agx(AgxLook look) { super(nCreateAgxToneMapper(look.ordinal())); } diff --git a/filament/include/filament/ToneMapper.h b/filament/include/filament/ToneMapper.h index 068c9b6dc3d..d220f47b662 100644 --- a/filament/include/filament/ToneMapper.h +++ b/filament/include/filament/ToneMapper.h @@ -115,6 +115,9 @@ struct UTILS_PUBLIC FilmicToneMapper final : public ToneMapper { 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 { @@ -123,6 +126,11 @@ struct UTILS_PUBLIC AgxToneMapper final : public ToneMapper { GOLDEN //!< A golden tinted, slightly washed look for BT.1886 displays }; + /** + * Builds a new AgX tone mapper. + * + * @param look an optional creative adjustment to contrast and saturation + */ explicit AgxToneMapper(AgxLook look = AgxLook::NONE) noexcept; ~AgxToneMapper() noexcept final;