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

Refactors the Projection classed only #5163

Merged

Conversation

HarelM
Copy link
Collaborator

@HarelM HarelM commented Dec 6, 2024

This is another step by moving the projection code to vertical perspective and adding the transitionalable logic, but without using this logic.

@HarelM HarelM requested review from kubapelc, birkskyum and ibesora and removed request for kubapelc December 6, 2024 12:18
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 18 lines in your changes missing coverage. Please review.

Project coverage is 91.78%. Comparing base (2dcbea9) to head (7eebdf2).
Report is 1 commits behind head on projection-expression.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/geo/projection/globe_projection.ts 86.72% 15 Missing ⚠️
.../geo/projection/vertical_perspective_projection.ts 81.25% 3 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##           projection-expression    #5163      +/-   ##
=========================================================
- Coverage                  91.79%   91.78%   -0.02%     
=========================================================
  Files                        279      280       +1     
  Lines                      38651    38760     +109     
  Branches                    6783     6736      -47     
=========================================================
+ Hits                       35481    35575      +94     
- Misses                      3039     3055      +16     
+ Partials                     131      130       -1     

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

@HarelM HarelM merged commit dda90d8 into projection-expression Dec 6, 2024
13 of 15 checks passed
@HarelM HarelM deleted the projection-epression-projection-refactor branch December 6, 2024 13:20
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 <[email protected]>

* 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 <[email protected]>

* Final removal of TODOs and some minor clean up

* Update CHANGELOG.md

---------

Co-authored-by: Isaac Besora Vilardaga <[email protected]>
Co-authored-by: Jakub Pelc <[email protected]>
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