CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
13.03k stars 3.51k forks source link

remove defaultValue() and create DefaultValue namespace #12252

Open dave-b-b opened 1 month ago

dave-b-b commented 1 month ago

Description

Issue number and link

Issue: 11674 https://github.com/CesiumGS/cesium/issues/11674

Testing plan

The following tests failed: ✗ renders a point ✗ renders pnts with point size style

Author checklist

github-actions[bot] commented 1 month ago

Thank you for the pull request, @dave-b-b!

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

dave-b-b commented 1 month ago

@jjspace Can you delete the other commits I had? I started over because I couldn't get the tests to pass. This one is much cleaner. If I can get help on the two tests that are failing, then I can move forward with this one.

jjspace commented 1 month ago

@dave-b-b I'm a little confused on the state of this with https://github.com/CesiumGS/cesium/pull/12207 still open? should that PR be closed and only plan to merge this one? All the tests look like they're passing on this branch, which ones are you having issues with? I also still see a bunch of instances of defaultValue() being used on this branch, are there commits missing? is this ready to review?

dave-b-b commented 1 month ago

@jjspace Yes, I could not get the tests to pass on #12207, so I started over. You can delete that pull request because I redid everything here. I had two tests fail: ✗ renders a point ✗ renders pnts with point size style

They weren't passing on my machine. I'm almost done with this pull request. I'm trying to finish everything right now.

dave-b-b commented 1 month ago

@jjspace I finished and submitted the final pull request, but I'm not sure whether I should rebase/merge.

A few of the tests are failing, so I must have messed something up. Can you help with this?

  1. destroys resources after pixel datatype changes
    Renderer/FramebufferManager

  2. destroys resources after pixel format changes
    Renderer/FramebufferManager

  3. BalloonStyle: creates table from ExtendedData
    DataSources/KmlDataSource

  4. Parse Camera and LookAt on features
    DataSources/KmlDataSource

  5. recreates resources when HDR changes
    Scene/GlobeTranslucencyFramebuffer

  6. white
    Scene/PostProcessStageCollection HDR tonemapping Reinhard

  7. red
    Scene/PostProcessStageCollection HDR tonemapping Reinhard

  8. green
    Scene/PostProcessStageCollection HDR tonemapping Reinhard

  9. blue
    Scene/PostProcessStageCollection HDR tonemapping Reinhard

  10. white
    Scene/PostProcessStageCollection HDR tonemapping Filmic

  11. red
    Scene/PostProcessStageCollection HDR tonemapping Filmic

  12. green
    Scene/PostProcessStageCollection HDR tonemapping Filmic

  13. blue
    Scene/PostProcessStageCollection HDR tonemapping Filmic

  14. white
    Scene/PostProcessStageCollection HDR tonemapping ACES

  15. red
    Scene/PostProcessStageCollection HDR tonemapping ACES

  16. green
    Scene/PostProcessStageCollection HDR tonemapping ACES

  17. blue
    Scene/PostProcessStageCollection HDR tonemapping ACES

  18. white
    Scene/PostProcessStageCollection HDR tonemapping PBR Neutral

  19. grey
    Scene/PostProcessStageCollection HDR tonemapping PBR Neutral

  20. red
    Scene/PostProcessStageCollection HDR tonemapping PBR Neutral

  21. green
    Scene/PostProcessStageCollection HDR tonemapping PBR Neutral

  22. blue
    Scene/PostProcessStageCollection HDR tonemapping PBR Neutral

  23. renders with HDR when available
    Scene/Scene

  24. renders a point
    Scene/Vector3DTilePoints

  25. renders pnts with point size style
    Scene/Model/Model3DTileContent pnts

  26. changing specularEnvironmentMaps works
    Scene/Model/Model imageBasedLighting

dave-b-b commented 1 month ago

@jjspace Everything seems fine except two tests that I can't get to pass.

dave-b-b commented 1 month ago
Screenshot 2024-10-21 at 5 53 09 PM

@jjspace Not sure why this test fails in the pipeline, but it passes when I run it on my computer....

jjspace commented 1 month ago

@dave-b-b that statistics test failure is a known test failure we've been fighting in CI for a little bit. Don't worry about it https://github.com/CesiumGS/cesium/issues/11958

dave-b-b commented 1 month ago

@jjspace I think this is done now. I add something to the change log under 2024-12-01

dave-b-b commented 1 month ago

@jjspace Should I update the branch? I don't know what I should do. We already discussed the one test that is failing.

dave-b-b commented 1 month ago

@jjspace I didn't realize that rebasing would bring in changes from other pull requests. The only way for me to undo this would be to redo the vast majority of the pull request. Can you help me figure this out?