Globe custom layer fixes #5150

kubapelc · 2024-12-03T13:37:44Z

fixes 3D model disappearing at high zooms when globe is used Incorrect custom layer render methods argument args.defaultProjectionData.mainMatrix value using globe projection with high zoom level #5117 + adds a render test for this issue
fixes 3D model disappearing during projection transition, adds internal API for setting nearZ and farZ on transforms to help with this
fixes the globe-custom-tiles.html example
fixes that some examples were using internal API, exposes the needed functions in custom layer args

…rcator

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

…aring during projection transition

…'t use internal API in examples

…CustomLayer implementation

codecov · 2024-12-03T13:42:25Z

Codecov Report

Attention: Patch coverage is 88.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.81%. Comparing base (ea4f31a) to head (48dee24).
Report is 1 commits behind head on projection-expression.

Files with missing lines	Patch %	Lines
src/geo/transform_helper.ts	73.91%	6 Missing ⚠️
src/geo/projection/globe_transform.ts	91.66%	2 Missing ⚠️
src/geo/projection/mercator_transform.ts	94.11%	2 Missing ⚠️
...c/geo/projection/vertical_perspective_transform.ts	89.47%	2 Missing ⚠️

Additional details and impacted files

@@                    Coverage Diff                    @@
##           projection-expression    #5150      +/-   ##
=========================================================
- Coverage                  91.82%   91.81%   -0.01%     
=========================================================
  Files                        281      281              
  Lines                      38712    38751      +39     
  Branches                    6745     6757      +12     
=========================================================
+ Hits                       35548    35581      +33     
- Misses                      3036     3042       +6     
  Partials                     128      128

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

src/geo/projection/globe_transform.ts

src/style/style_layer/custom_style_layer.ts

test/examples/globe-3d-model.html

src/shaders/_projection_globe.vertex.glsl

src/geo/transform_helper.ts

src/geo/transform_interface.ts

src/geo/projection/globe_transform.ts

HarelM · 2024-12-03T20:36:51Z

I've added some comments. Thanks!

HarelM · 2024-12-08T08:58:24Z

I think the following PR:

Use Transitionalable infrastructure for transfrom globeness changes. #5164

Will cause this PR to not exactly match the changes I've made.
I can easily incorporate some of the fixes here, but I'm still having an issue with the API around setting far and near Z, I would prefer to make the near/farZ either auto calculated or allow setting them from the outside - i.e. only add a flag to disable their calculations, what do you think?

…bapelc/globe-custom-layer-fix2 # Conflicts: # src/geo/projection/globe_transform.ts

kubapelc · 2024-12-11T09:28:03Z

I can help with merge conflicts with #5164 if there are any. The changes don't seem to overlap too much.

…bapelc/globe-custom-layer-fix2 # Conflicts: # CHANGELOG.md # src/geo/projection/globe_transform.ts # src/geo/projection/mercator_transform.ts # src/geo/projection/vertical_perspective_transform.ts # src/geo/transform_interface.ts

src/geo/projection/globe_transform.ts

src/geo/transform_interface.ts

HarelM · 2024-12-11T10:29:32Z

I've added a few comments.
Also there's a need to resolve conflicts on this branch.
Let me know when those can be fixed and I'll merge this to the upstream branch and merge that branch into main.

HarelM · 2024-12-11T14:35:38Z

src/geo/projection/mercator_transform.ts

-        // seems to solve z-fighting issues in deck.gl while not clipping buildings too close to the camera.
-        this._nearZ = this._helper._height / 50;
+
+        if (this._helper.autoCalculateNearFarZ) {


I was under the impression other variables are used later on, since this is not the case I would consider moving this to an internal method, it will be a lot more readable.

HarelM · 2024-12-11T14:42:10Z

src/geo/projection/mercator_transform.ts

    }

    public get cameraPosition(): vec3 { return this._cameraPosition; }
    public get projectionMatrix(): mat4 { return this._projectionMatrix; }
    public get modelViewProjectionMatrix(): mat4 { return this._viewProjMatrix; }
    public get inverseProjectionMatrix(): mat4 { return this._invProjMatrix; }
-    public get nearZ(): number { return this._nearZ; }
-    public get farZ(): number { return this._farZ; }
+    public get nearZ(): number { return this._helper.nearZ; }


Move these methods to be with the other helper methods in the file

HarelM · 2024-12-11T14:53:02Z

I'll merge this and fix what's left in the other branch, THANKS!!!

* 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>

kubapelc and others added 10 commits December 3, 2024 12:10

Simplify custom layer ProjectionData code in mercator transform

64b3fdd

Fix globe tiles example

0a99eae

Fix globe projection shader

589c701

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

ea41f44

Fix custom layer 3D models not rendering when globe transitions to me…

bc20299

…rcator

Move camera to center distance to helper

3360e43

# 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 disappe…

2f0cea3

…aring during projection transition

Expose getProjectionData and getMatrixForModel for custom layers, don…

541599a

…'t use internal API in examples

VerticalPerspectiveTransform should have a valid getProjectionDataFor…

4368c65

…CustomLayer implementation

Add changelog entry

2e0614d

kubapelc mentioned this pull request Dec 3, 2024

Allow modifying farZ camera plane variable #5091

Closed

kubapelc added 2 commits December 3, 2024 15:05

Fix missing docs

3837d33

Fix docs again

17130ae