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

RD-393 #10

Merged
merged 13 commits into from
Dec 15, 2024
Merged

RD-393 #10

merged 13 commits into from
Dec 15, 2024

Conversation

sebstryczek
Copy link
Contributor

@sebstryczek sebstryczek commented Nov 27, 2024

Objective

To be able the 3D plugin with SDK v3 (and globe projection).
https://maptiler.atlassian.net/browse/RD-393

Description

Maplibre introduced breaking changes to 3D models placement

Acceptance

Using examples

Checklist

  • I have added relevant info to the CHANGELOG.md

@sebstryczek sebstryczek changed the title Rd 393 [WIP] RD-393 Nov 27, 2024
@sebstryczek sebstryczek changed the title [WIP] RD-393 RD-393 Nov 28, 2024
@jonathanlurie
Copy link
Contributor

Very minor display bug that happens during the animated transition between globe and mercator. It does not even happen all the time, it looks like only with some specific heading range for the model and only with some camera angle. Super nice, no big deal, but it makes me think that maybe the internal ThreeJS scene should be visible = false or just not rendered during this short transition.

plane-globe.mp4

@jonathanlurie
Copy link
Contributor

Very minor display bug that happens during the animated transition between globe and mercator. It does not even happen all the time, it looks like only with some specific heading range for the model and only with some camera angle. Super nice, no big deal, but it makes me think that maybe the internal ThreeJS scene should be visible = false or just not rendered during this short transition.

plane-globe.mp4

This actually works OK as far as I tested:

render(_gl: WebGLRenderingContext | WebGL2RenderingContext, options: CustomRenderMethodInput) {
  // Not rendering during the animated transition
  if (options.defaultProjectionData.projectionTransition > EPSILON && options.defaultProjectionData.projectionTransition < (1 - EPSILON)) {
    return;
  }

...
}

Copy link
Contributor

@jonathanlurie jonathanlurie left a comment

Choose a reason for hiding this comment

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

Overall fine to me considering the current maplibre limitations. That being said, it should probably not be merged until SDK v3 (stable)

* The `defaultProjectionData` seems to be incorrect when the globe projection is used and the zoom is high (and projection is changing to mercator).
* Waiting for help: https://github.com/maplibre/maplibre-gl-js/issues/5117
*/
if ("_mercatorTransform" in this.map.transform === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best to avoid relying on private props as they are not guaranteed to stay in future versions

* Waiting for help: https://github.com/maplibre/maplibre-gl-js/issues/5117
*/
if ("_mercatorTransform" in this.map.transform === true) {
defaultProjectionData =
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering why can't we just use the regular options.defaultprojectionData or even reach it with this.map.transform.getProjectionDataForCustomLayer() and while starting to go down the rabbit hole, I ended up with the issue you posted on the MapLibre repo maplibre/maplibre-gl-js#5117 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

So i suppose we just leave this like that until it's being fixed

@sebstryczek sebstryczek changed the base branch from main to using-sdk-v3 December 10, 2024 17:19
@sebstryczek sebstryczek merged commit c71dbe7 into using-sdk-v3 Dec 15, 2024
1 check failed
@sebstryczek sebstryczek deleted the RD-393 branch December 15, 2024 18:31
sebstryczek added a commit that referenced this pull request Jan 7, 2025
* RD-393 (#10)

* Update image links + replace plugin by module (#13)

* Add position indicators models

* Install maptiler sdk 3

* Update 3D layer to sdk 3

* Add example

* Add comments

* Add offset

* Make linter happy

* Refactor mount everest example

* Update examples to sdk v3

* Make biome happy

* Remove break line

* Fix model position issue on projection change animation

---------

Co-authored-by: Jonathan Lurie <[email protected]>

* Make biome happy

* RD-453_telemetry (#12)

* Update image links + replace plugin by module (#13)

* Add position indicators models

* Install maptiler sdk 3

* Update 3D layer to sdk 3

* Add example

* Add comments

* Add offset

* Make linter happy

* Refactor mount everest example

* Update examples to sdk v3

* Make biome happy

* Remove break line

* Add telemetry module registration

* Make registration works for multiple map instances

* Simplify telemetry

* Update sdk to v3.0.0-rc.4

* Fix model position issue on projection change animation

---------

Co-authored-by: Jonathan Lurie <[email protected]>

* RD-492_maplibre-v5.0.0-pre10 (#15)

* Update sdk version

* Update examples

* Bump version

* Fix version

* Update dependencies

* Update demos

* Fix dev mode

* Clean up

---------

Co-authored-by: Jonathan Lurie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants