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

[core] Fix cubic-bezier interpolation for zooms outside stop range. #12826

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

ChrisLoer
Copy link
Contributor

Fixes issue #12812.
Using util::interpolationFactor handles the case where inputLevels.min == inputLevels.max, so it returns an interpolation factor of "0" instead of dividing by 0.

@nickidlugash, can you build this branch, try it out with your style, and make sure you get the results you expect?

@jfirebaugh, does this look right to you? I'll start work on a regression test for this on the gl js side...

Fixes issue #12812.
Using `util::interpolationFactor` handles the case where inputLevels.min == inputLevels.max, so it returns an interpolation factor of "0" instead of dividing by 0.
@ChrisLoer ChrisLoer requested a review from jfirebaugh September 5, 2018 19:10
@nickidlugash
Copy link
Contributor

@ChrisLoer I checked this out with the style in question (that uses cubic-bezier zoom-dependent interpolation for text-size), and it looks good to me (see screenshots below). 👍

As you requested in chat, I reviewed the visual behavior of the style after the last designated stop in the curve, and it looks as expected – the text-size stays static beyond the last zoom stop, which is the same behavior as with gl-js.

Thanks so much for getting this bug fix in so quickly 🙇

Also thanks to @riastrad for helping me with the local build to review this 🙇

screen shot 2018-09-06 at 1 43 49 pm

screen shot 2018-09-06 at 1 44 17 pm

screen shot 2018-09-06 at 1 44 11 pm

@ChrisLoer ChrisLoer merged commit 50f9381 into master Sep 6, 2018
@ChrisLoer ChrisLoer deleted the fix-bezier branch September 6, 2018 18:10
@friedbunny friedbunny added bug Core The cross-platform C++ core, aka mbgl labels Sep 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants