Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: change transformation setters call order #2391

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

oxidase
Copy link
Contributor

@oxidase oxidase commented May 15, 2024

The order of setters in applyViewStateToTransform has a side effect which leads to wrong camera transformation. In a case of map.jumpTo({center: [12,34], zoom: 15}) from {center: [0,0], zoom: 0} the current order of setters in case of maplibre-gl leads to the following:

tr.center = new center.constructor(v.longitude, v.latitude);  /* (12, 34); */
  -> const {center, zoom} = this.getConstrained(this.center /* [12, 34]*/ , this.zoom /* 0 */);
       this.center = center;  /* [12, 0] */
       this.zoom = zoom;  /* 0 */

tr.zoom = v.zoom; /* 15 */
  -> const {center, zoom} = this.getConstrained(this.center /* [12, 0]*/ , this.zoom /* 15 */);
       this.center = center;  /* [12, 0] */
       this.zoom = zoom;  /* 15 */

But expected would be to have {center: [12,34], zoom: 15} instead of {center: [12,0], zoom: 15}.

The changed order leads to the following setters sequence

tr.zoom = v.zoom; /* 15 */
  -> const {center, zoom} = this.getConstrained(this.center /* [0, 0]*/ , this.zoom /* 15 */);
       this.center = center;  /* [0, 0] */
       this.zoom = zoom;  /* 15 */

tr.center = new center.constructor(v.longitude, v.latitude);  /* (12, 34); */
  -> const {center, zoom} = this.getConstrained(this.center /* [12, 34]*/ , this.zoom /* 15 */);
       this.center = center;  /* [12, 34] */
       this.zoom = zoom;  /* 15 */

@oxidase oxidase force-pushed the fix/constraints_order branch from 9e00104 to 398ce27 Compare May 15, 2024 09:00
@Pessimistress Pessimistress merged commit 2c369d0 into visgl:master Jul 5, 2024
2 checks passed
@oxidase oxidase deleted the fix/constraints_order branch July 5, 2024 19:57
@gabeboning
Copy link

@Pessimistress this change hasn't made it into a release yet, and would fix a bug I am experiencing. Can you cut a release that would include this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants