Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Do not consider X axis when constraining scale #13463

Merged
merged 3 commits into from
Nov 29, 2018

Conversation

brunoabinader
Copy link
Member

Follow-up to #12611, back then I did not realize that GL JS's behavior was to not consider X axis when constraining scale. This PR fixes this edge case, and uses the newly added basic-v9/z0-narrow-y render test to verify parity with GL JS.

This fix uncovered an issue with heatmaps, where features outside the tile extent where considered in still rendering mode (@mourner you were right in #12611 (comment) - it was indeed two points being rendered, one inside the tile extent, and the other outside).

@brunoabinader brunoabinader added GL JS parity For feature parity with Mapbox GL JS Core The cross-platform C++ core, aka mbgl labels Nov 28, 2018
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

Was it intentional to include the fill-extrusion-vertical-gradient commit?

@brunoabinader
Copy link
Member Author

Noting that I ended up implementing support for fill-extrusion-vertical-gradient (mapbox/mapbox-gl-js#6841), 99% of the code was auto-generated, I only needed to add a single line manually here for it to work.

Fixes #13462.

platform/ios/CHANGELOG.md Outdated Show resolved Hide resolved
platform/macos/CHANGELOG.md Outdated Show resolved Hide resolved
platform/darwin/src/MGLFillExtrusionStyleLayer.h Outdated Show resolved Hide resolved
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

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

Android changes LGTM!

@brunoabinader brunoabinader force-pushed the constrain-scale-no-y-axis branch 2 times, most recently from aba8c9a to cc95a75 Compare November 29, 2018 10:27
@brunoabinader brunoabinader merged commit 369d1f0 into master Nov 29, 2018
@brunoabinader brunoabinader deleted the constrain-scale-no-y-axis branch November 29, 2018 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl GL JS parity For feature parity with Mapbox GL JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants