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

Allow changing the HDR tonemap #12160

Merged
merged 10 commits into from
Sep 3, 2024
Merged

Allow changing the HDR tonemap #12160

merged 10 commits into from
Sep 3, 2024

Conversation

jjspace
Copy link
Contributor

@jjspace jjspace commented Aug 27, 2024

Description

  • Expose the tonemapper property of PostProcessStageCollection to allow changing the tonemap used when HDR is turned on
    • Updated the HDR sandcastle to have a dropdown to switch this
  • Create a new tonemap for the PBR Neutral Tonemap from Khronos
    • I think this one looks really good so I switched to use it as the default but I'm open to changing that.
  • Need to confirm how to test this as I got the color it's rendering but I don't know if it's correct...

Issue number and link

part of #12126

Testing plan

  • Check the updated HDR sandcastle and test the various tonemaps (local, ci)

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@jjspace jjspace requested a review from ggetz August 27, 2024 21:01
Copy link

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

Copy link
Contributor

@ggetz ggetz left a comment

Choose a reason for hiding this comment

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

Thanks @jjspace! The new tonemapper looks great!

@lilleyse
Copy link
Contributor

We should switch to the Khronos PBR Neutral Tone Mapper for models as well. Should be a small change here:

// In HDR mode, the frame buffer is in linear color space. The
// post-processing stages (see PostProcessStageCollection) will handle
// tonemapping. However, if HDR is not enabled, we must tonemap else large
// values may be clamped to 1.0
#ifndef HDR
color = czm_acesTonemapping(color);
#endif

@jjspace
Copy link
Contributor Author

jjspace commented Aug 30, 2024

We should switch to the Khronos PBR Neutral Tone Mapper for models as well. Should be a small change here:

Yup! This is already included, thanks @lilleyse

@jjspace
Copy link
Contributor Author

jjspace commented Aug 30, 2024

@ggetz updated based on your comments, thanks. The biggest thing I've changed was a slight overhaul on the tests to actually test multiple colors as you suggested using small helper function. It may change the stacktrace a bit as you mentioned but the extra withContext helps a lot tracking down what number actually failed and it'd be really messy to duplicate those everywhere

2024-08-30_11-52

Opened #12175 to track the bug I found with the modified Reinhard tonemap

Copy link
Contributor

@ggetz ggetz left a comment

Choose a reason for hiding this comment

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

Thanks @jjspace, this is really close!

CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Show resolved Hide resolved
packages/engine/Source/Scene/PostProcessStageCollection.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/PostProcessStageCollection.js Outdated Show resolved Hide resolved
@ggetz
Copy link
Contributor

ggetz commented Aug 30, 2024

@jjspace I see quite a few failing specs when running locally:

Failing test logs

1) renders tileset with custom up and forward axes
     Scene/Cesium3DTileset
     Expected 107 to be less than 64.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:705:27 <- Build/Specs/SpecList.js:147593:27
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 107 to be less than 64.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:706:27 <- Build/Specs/SpecList.js:147594:27
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

2) sets colorBlendMode
     Scene/Cesium3DTileset
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3335:25 <- Build/Specs/SpecList.js:149683:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3336:25 <- Build/Specs/SpecList.js:149684:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 58 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3351:25 <- Build/Specs/SpecList.js:149694:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3364:25 <- Build/Specs/SpecList.js:149704:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3384:25 <- Build/Specs/SpecList.js:149719:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 48 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3398:25 <- Build/Specs/SpecList.js:149730:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 94 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3420:25 <- Build/Specs/SpecList.js:149747:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 251 to be less than 251.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3428:25 <- Build/Specs/SpecList.js:149753:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 106 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3431:25 <- Build/Specs/SpecList.js:149756:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3439:25 <- Build/Specs/SpecList.js:149762:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3440:25 <- Build/Specs/SpecList.js:149763:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3449:25 <- Build/Specs/SpecList.js:149770:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3462:25 <- Build/Specs/SpecList.js:149780:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

3) sets colorBlendMode when vertex texture fetch is not supported
     Scene/Cesium3DTileset
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3335:25 <- Build/Specs/SpecList.js:149683:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3336:25 <- Build/Specs/SpecList.js:149684:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 58 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3351:25 <- Build/Specs/SpecList.js:149694:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3364:25 <- Build/Specs/SpecList.js:149704:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3384:25 <- Build/Specs/SpecList.js:149719:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 48 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3398:25 <- Build/Specs/SpecList.js:149730:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 94 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3420:25 <- Build/Specs/SpecList.js:149747:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 251 to be less than 251.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3428:25 <- Build/Specs/SpecList.js:149753:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 106 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3431:25 <- Build/Specs/SpecList.js:149756:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3439:25 <- Build/Specs/SpecList.js:149762:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3440:25 <- Build/Specs/SpecList.js:149763:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3449:25 <- Build/Specs/SpecList.js:149770:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3462:25 <- Build/Specs/SpecList.js:149780:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

4) sets colorBlendMode for textured tileset
     Scene/Cesium3DTileset
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3335:25 <- Build/Specs/SpecList.js:149683:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3336:25 <- Build/Specs/SpecList.js:149684:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 58 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3351:25 <- Build/Specs/SpecList.js:149694:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3364:25 <- Build/Specs/SpecList.js:149704:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3384:25 <- Build/Specs/SpecList.js:149719:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 48 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3398:25 <- Build/Specs/SpecList.js:149730:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 93 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3420:25 <- Build/Specs/SpecList.js:149747:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 251 to be less than 251.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3428:25 <- Build/Specs/SpecList.js:149753:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 105 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3431:25 <- Build/Specs/SpecList.js:149756:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3439:25 <- Build/Specs/SpecList.js:149762:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3440:25 <- Build/Specs/SpecList.js:149763:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3449:25 <- Build/Specs/SpecList.js:149770:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3462:25 <- Build/Specs/SpecList.js:149780:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

5) sets colorBlendMode for instanced tileset
     Scene/Cesium3DTileset
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3335:25 <- Build/Specs/SpecList.js:149683:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3336:25 <- Build/Specs/SpecList.js:149684:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 58 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3351:25 <- Build/Specs/SpecList.js:149694:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3364:25 <- Build/Specs/SpecList.js:149704:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3384:25 <- Build/Specs/SpecList.js:149719:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 48 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3398:25 <- Build/Specs/SpecList.js:149730:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 94 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3420:25 <- Build/Specs/SpecList.js:149747:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 251 to be less than 251.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3428:25 <- Build/Specs/SpecList.js:149753:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 106 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3431:25 <- Build/Specs/SpecList.js:149756:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3439:25 <- Build/Specs/SpecList.js:149762:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3440:25 <- Build/Specs/SpecList.js:149763:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3449:25 <- Build/Specs/SpecList.js:149770:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3462:25 <- Build/Specs/SpecList.js:149780:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

6) sets colorBlendMode for vertex color tileset
     Scene/Cesium3DTileset
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3335:25 <- Build/Specs/SpecList.js:149683:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3336:25 <- Build/Specs/SpecList.js:149684:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 58 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3351:25 <- Build/Specs/SpecList.js:149694:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3364:25 <- Build/Specs/SpecList.js:149704:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3384:25 <- Build/Specs/SpecList.js:149719:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 48 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3398:25 <- Build/Specs/SpecList.js:149730:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 94 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3420:25 <- Build/Specs/SpecList.js:149747:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 251 to be less than 251.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3428:25 <- Build/Specs/SpecList.js:149753:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 106 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3431:25 <- Build/Specs/SpecList.js:149756:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3439:25 <- Build/Specs/SpecList.js:149762:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 115 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3440:25 <- Build/Specs/SpecList.js:149763:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 73 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3449:25 <- Build/Specs/SpecList.js:149770:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 43 to be less than 25.
    at <Jasmine>
    at packages/engine/Specs/Scene/Cesium3DTilesetSpec.js:3462:25 <- Build/Specs/SpecList.js:149780:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

8) grey
     Scene/PostProcessStageCollection HDR tonemapping Reinhard
     without HDR: Expected to render [127,127,127,255], but actually rendered [128,128,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:436:11 <- Build/Specs/SpecList.js:212721:11)
    at <Jasmine>

9) grey
     Scene/PostProcessStageCollection HDR tonemapping Modified Reinhard
     without HDR: Expected to render [127,127,127,255], but actually rendered [128,128,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:478:11 <- Build/Specs/SpecList.js:212754:11)
    at <Jasmine>

10) red
     Scene/PostProcessStageCollection HDR tonemapping Modified Reinhard
     without HDR: Expected to render [127,0,0,255], but actually rendered [128,0,0,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:485:11 <- Build/Specs/SpecList.js:212761:11)
    at <Jasmine>

11) green
     Scene/PostProcessStageCollection HDR tonemapping Modified Reinhard
     without HDR: Expected to render [0,127,0,255], but actually rendered [0,128,0,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:492:11 <- Build/Specs/SpecList.js:212768:11)
    at <Jasmine>

12) blue
     Scene/PostProcessStageCollection HDR tonemapping Modified Reinhard
     without HDR: Expected to render [0,0,127,255], but actually rendered [0,0,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:499:11 <- Build/Specs/SpecList.js:212775:11)
    at <Jasmine>

13) grey
     Scene/PostProcessStageCollection HDR tonemapping Filmic
     without HDR: Expected to render [127,127,127,255], but actually rendered [128,128,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:512:11 <- Build/Specs/SpecList.js:212787:11)
    at <Jasmine>

14) grey
     Scene/PostProcessStageCollection HDR tonemapping ACES
     without HDR: Expected to render [127,127,127,255], but actually rendered [128,128,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:530:11 <- Build/Specs/SpecList.js:212804:11)
    at <Jasmine>

15) grey
     Scene/PostProcessStageCollection HDR tonemapping PBR Neutral
     without HDR: Expected to render [127,127,127,255], but actually rendered [128,128,128,255].
    at <Jasmine>
    at validateTonemapper (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:414:50 <- Build/Specs/SpecList.js:212703:51)
    at UserContext.<anonymous> (packages/engine/Specs/Scene/PostProcessStageCollectionSpec.js:553:11 <- Build/Specs/SpecList.js:212826:11)
    at <Jasmine>

16) renders with HDR when available
     Scene/Scene
     Expected 250 to equal 0.
    at <Jasmine>
    at packages/engine/Specs/Scene/SceneSpec.js:839:25 <- Build/Specs/SpecList.js:221163:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>
     Expected 250 to equal 0.
    at <Jasmine>
    at packages/engine/Specs/Scene/SceneSpec.js:840:25 <- Build/Specs/SpecList.js:221164:25
    at compare (Specs/addDefaultMatchers.js:314:13 <- Build/Specs/karma-main.js:296:13)
    at <Jasmine>

@jjspace
Copy link
Contributor Author

jjspace commented Aug 30, 2024

@ggetz thanks for catching those, I forgot to run all tests and had only verified the new ones work. It seems HDR is not possible in CI? if it is then i'm surprised those ones passed...? Either way they should all be updated now with the new colors as we discussed in person.
I also updated the changelog to hopefully account for everything now.

@ggetz
Copy link
Contributor

ggetz commented Sep 3, 2024

Thanks @jjspace! I pushed a small change to the specs to allow for a 1-byte tolerance in the render tests. This allowed all the specs to pass on my machine.

@ggetz
Copy link
Contributor

ggetz commented Sep 3, 2024

Looks awesome! 😎

@ggetz ggetz merged commit c191e08 into main Sep 3, 2024
9 checks passed
@ggetz ggetz deleted the hdr-tonemap-options branch September 3, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants