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

Refactor camera helper to split the logic for vertical-perspective and mercator #5162

Merged
merged 5 commits into from
Dec 6, 2024

Conversation

HarelM
Copy link
Collaborator

@HarelM HarelM commented Dec 6, 2024

Launch Checklist

This is in the same direction as but only for the camera helpers for this PR as the other PR was too big and too complicated.

Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 76.50131% with 90 lines in your changes missing coverage. Please review.

Project coverage is 91.80%. Comparing base (dda90d8) to head (584beb2).
Report is 1 commits behind head on projection-expression.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...o/projection/vertical_perspective_camera_helper.ts 73.63% 82 Missing ⚠️
src/geo/projection/camera_helper.ts 94.00% 3 Missing ⚠️
src/geo/projection/globe_camera_helper.ts 82.35% 3 Missing ⚠️
src/geo/projection/projection_factory.ts 60.00% 2 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           projection-expression    #5162      +/-   ##
=========================================================
+ Coverage                  91.79%   91.80%   +0.01%     
=========================================================
  Files                        280      281       +1     
  Lines                      38749    38824      +75     
  Branches                    6754     6740      -14     
=========================================================
+ Hits                       35569    35644      +75     
+ Misses                      3052     3048       -4     
- Partials                     128      132       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HarelM HarelM merged commit 6275dd2 into projection-expression Dec 6, 2024
14 of 15 checks passed
@HarelM HarelM deleted the projection-expression-camera-helper branch December 6, 2024 15:11
HarelM added a commit that referenced this pull request Dec 12, 2024
* Initial commit to move the logic of transition out to a new file

* Remove one line method

* Fix build test

* Fix projection data bad refactoring

* Fix render and size tests

* Fix failing unit test

* Uncomment unit tests

* Fix typo, remove unneeded details provider initialization. Added current transfrom getter

* Fix typo

* Add back error correction for latitude.

* Refactors the `Projection` classed only (#5163)

* This refactors the projection class only, without touching other parts of the code.

* remove mercator from the code, revert more changes

* Refactor camera helper to split the logic for vertical-perspective and mercator (#5162)

* Refactor camera helper

* Use the helper in the factory.

* Fix build test

* Fix according to code review

* Use `Transitionalable` infrastructure for transfrom globeness changes. (#5164)

* Remove duplicate code

* Fix lint, add some methods for original branch

* Add projection definition in factory

* Add has transition to projection

* Fix transition value

* Remove isRenderingDirty and use hasTransition when needed

* Remove the last part of the animation handling in the transform class

* More clean-up

* Remove some "TODO"s.

* Remove reference to globe projection in globe transform

* Rename newFrame with recalculateCache

* Remove unneeded ifs

* Add support for arbitrary projection definitions

* Uses mercator matrix when globe is using mercator transform

* Improve handling of fog martix in globe transform

---------

Co-authored-by: Isaac Besora Vilardaga <isaac.besora@gmail.com>

* Globe custom layer fixes (#5150)

* Simplify custom layer ProjectionData code in mercator transform

* Fix globe tiles example

* Fix globe projection shader

* Add custom layer 3D model after globe->mercator transition render test

* Fix custom layer 3D models not rendering when globe transitions to mercator

* Move camera to center distance to helper

# Conflicts:
#	src/geo/projection/globe_transform.ts
#	src/geo/projection/mercator_transform.ts
#	src/geo/transform_helper.ts
#	src/geo/transform_interface.ts

* Synchronize globe+mercator near and far Z to prevent 3D model disappearing during projection transition

* Expose getProjectionData and getMatrixForModel for custom layers, don't use internal API in examples

* VerticalPerspectiveTransform should have a valid getProjectionDataForCustomLayer implementation

* Add changelog entry

* Fix missing docs

* Fix docs again

* Review feedback

* Move nearZfarZoverride to transform helper

* Update build size

* Review feedback

* Change near/far Z override API

* Custom layers use internal API

* Fix globe custom tiles example

* Update build size

* Fix typo

---------

Co-authored-by: HarelM <harel.mazor@gmail.com>

* Final removal of TODOs and some minor clean up

* Update CHANGELOG.md

---------

Co-authored-by: Isaac Besora Vilardaga <isaac.besora@gmail.com>
Co-authored-by: Jakub Pelc <57600346+kubapelc@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants