[Merged by Bors] - Send emissive color to uniform as linear instead of sRGB

[coreh]

This produces more accurate results for the EmissiveStrengthTest glTF test case.

(Requires manually setting the emission, for now)

Before:

After:

Tagging @JMS55 as a co-author, since this fix is based on their experiments with emission.

Objective

• Have more accurate results for the EmissiveStrengthTest glTF test case.

Solution

• Make sure we send the emissive color as linear instead of sRGB.

---

Changelog

• Emission strength is now correctly interpreted by the StandardMaterial as linear instead of sRGB.

Migration Guide

• If you have previously manually specified emissive values with Color::rgb() and would like to retain the old visual results, you must now use Color::rgb_linear() instead;
• If you have previously manually specified emissive values with Color::rgb_linear() and would like to retain the old visual results, you'll need to apply a one-time gamma calculation to your channels manually to get the actual linear RGB value:
  • For channel values greater than 0.0031308, use (1.055 * value.powf(1.0 / 2.4)) - 0.055;
  • For channel values lower than or equal to 0.0031308, use value * 12.92;
• Otherwise, the results should now be more consistent with other tools/engines.

[coreh]

To replicate the results, download the test file from Khronos then apply this patch:

diff --git a/examples/tools/scene_viewer/main.rs b/examples/tools/scene_viewer/main.rs
index ee084e0af..ebfbb0937 100644
--- a/examples/tools/scene_viewer/main.rs
+++ b/examples/tools/scene_viewer/main.rs
@@ -6,6 +6,8 @@
 //! With no arguments it will load the `FlightHelmet` glTF model from the repository assets subdirectory.
 
 use bevy::{
+    core_pipeline::{bloom::BloomSettings, tonemapping::Tonemapping},
+    gltf::Gltf,
     math::Vec3A,
     prelude::*,
     render::primitives::{Aabb, Sphere},
@@ -78,8 +80,28 @@ fn setup_scene_after_load(
     mut scene_handle: ResMut<SceneHandle>,
     asset_server: Res<AssetServer>,
     meshes: Query<(&GlobalTransform, Option<&Aabb>), With<Handle<Mesh>>>,
+
+    gltf_assets: Res<Assets<Gltf>>,
+    mut materials: ResMut<Assets<StandardMaterial>>,
 ) {
     if scene_handle.is_loaded && !*setup {
+        let gltf = gltf_assets.get(&scene_handle.gltf_handle).unwrap();
+
+        let mh = gltf.named_materials.get("Emit1".into()).unwrap();
+        materials.get_mut(mh).unwrap().emissive = Color::rgb_linear(0.1, 0.5, 0.9);
+
+        let mh = gltf.named_materials.get("Emit2".into()).unwrap();
+        materials.get_mut(mh).unwrap().emissive = Color::rgb_linear(0.1, 0.5, 0.9) * 2.0;
+
+        let mh = gltf.named_materials.get("Emit4".into()).unwrap();
+        materials.get_mut(mh).unwrap().emissive = Color::rgb_linear(0.1, 0.5, 0.9) * 4.0;
+
+        let mh = gltf.named_materials.get("Emit8".into()).unwrap();
+        materials.get_mut(mh).unwrap().emissive = Color::rgb_linear(0.1, 0.5, 0.9) * 8.0;
+
+        let mh = gltf.named_materials.get("Emit16".into()).unwrap();
+        materials.get_mut(mh).unwrap().emissive = Color::rgb_linear(0.1, 0.5, 0.9) * 16.0;
+
         *setup = true;
         // Find an approximate bounding box of the scene from its meshes
         if meshes.iter().any(|(_, maybe_aabb)| maybe_aabb.is_none()) {
@@ -123,32 +145,35 @@ fn setup_scene_after_load(
                 )
                 .looking_at(Vec3::from(aabb.center), Vec3::Y),
                 camera: Camera {
-                    is_active: false,
+                    is_active: true,
+                    hdr: true,
                     ..default()
                 },
+                tonemapping: Tonemapping::BlenderFilmic,
                 ..default()
             },
-            EnvironmentMapLight {
-                diffuse_map: asset_server
-                    .load("assets/environment_maps/pisa_diffuse_rgb9e5_zstd.ktx2"),
-                specular_map: asset_server
-                    .load("assets/environment_maps/pisa_specular_rgb9e5_zstd.ktx2"),
-            },
+            BloomSettings::default(),
+            // EnvironmentMapLight {
+            //     diffuse_map: asset_server
+            //         .load("assets/environment_maps/pisa_diffuse_rgb9e5_zstd.ktx2"),
+            //     specular_map: asset_server
+            //         .load("assets/environment_maps/pisa_specular_rgb9e5_zstd.ktx2"),
+            // },
             camera_controller,
         ));
 
-        // Spawn a default light if the scene does not have one
-        if !scene_handle.has_light {
-            info!("Spawning a directional light");
-            commands.spawn(DirectionalLightBundle {
-                directional_light: DirectionalLight {
-                    shadows_enabled: false,
-                    ..default()
-                },
-                ..default()
-            });
-
-            scene_handle.has_light = true;
-        }
+        // // Spawn a default light if the scene does not have one
+        // if !scene_handle.has_light {
+        //     info!("Spawning a directional light");
+        //     commands.spawn(DirectionalLightBundle {
+        //         directional_light: DirectionalLight {
+        //             shadows_enabled: true,
+        //             ..default()
+        //         },
+        //         ..default()
+        //     });
+
+        //     scene_handle.has_light = true;
+        // }
     }
 }
diff --git a/examples/tools/scene_viewer/scene_viewer_plugin.rs b/examples/tools/scene_viewer/scene_viewer_plugin.rs
index 33277e6e5..897ed246d 100644
--- a/examples/tools/scene_viewer/scene_viewer_plugin.rs
+++ b/examples/tools/scene_viewer/scene_viewer_plugin.rs
@@ -12,7 +12,7 @@ use super::camera_controller_plugin::*;
 
 #[derive(Resource)]
 pub struct SceneHandle {
-    gltf_handle: Handle<Gltf>,
+    pub gltf_handle: Handle<Gltf>,
     scene_index: usize,
     #[cfg(feature = "animation")]
     animations: Vec<Handle<AnimationClip>>,

[JMS55]

Needs a migration guide/changelog, but code (the one line that there is) is good :)

[coreh]

Added the migration guide.

[JMS55]

Actually @coreh new change is needed: fix bloom example values, they're too bloom-y after this.

[coreh]

Let me fix that

[superdump]

The gltf loader does the right thing by interpreting the emissive factor values from the gltf as linear. However, the plain .into() in this PR was doing a linear to non-linear conversion, which was then being used as linear. The PR is correct.

[coreh]

Updated the guide with math extracted from this function and updated the bloom example using the same math to get the old results back:

[coreh]

Fixed two more examples. There's no real way to fix iter_combinations without removing the use of the ORANGE_RED constant, or replacing it with something else. But it's not really a rendering-focused example, and if anything one could argue that it makes it look more like the actual color 😆

Before:

After:

I guess I can use ORANGE instead?

[JMS55]

What about the bloom_2d example?

[coreh]

The bloom_2d example should be “safe” from this change, since it doesn't go through pbr_material.rs in any way.

Edit: Confirmed that it does look the same.

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• check-missing-features-in-docs
• ci
• markdownlint
• msrv
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12