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

Cubic bezier interpolation not respected for some layout property values #12812

Closed
nickidlugash opened this issue Sep 4, 2018 · 4 comments
Closed
Labels
bug Core The cross-platform C++ core, aka mbgl

Comments

@nickidlugash
Copy link
Contributor

nickidlugash commented Sep 4, 2018

Steps to reproduce

  1. Include a layout property value in your style that uses a cubic-bezier interpolation, e.g. text-size or text-letter-spacing. Can be either zoom or data dependent.

E.g.:

"text-size": [
          "interpolate",
          [ "cubic-bezier", 0.85, 0.7, 0.65, 1 ],
          [ "zoom" ],
          4,
          [ "step", [ "get", "symbolrank" ], 10, 6, 9.5, 7, 9 ],
          9,
          [ "step", [ "get", "symbolrank" ], 24, 6, 18, 7, 14 ]
        ],

Non-nested expressions also exhibit this behavior, e.g.

"text-size": [
          "interpolate",
          [ "cubic-bezier", 0.85, 0.7, 0.65, 1 ],
          [ "zoom" ],
          4,
          10,
          9,
          24
        ],

Please ping me for a copy of the stylesheet in question if needed, as I do not want to post it publicly.

Expected behavior

Value varies according to the interpolation defined in the style (as it does on mapbox-gl-js).

Actual behavior

All values for features in that style layer seems to just be the last stop value. This is happening in iOS, MacOS, and Android.

From testing various style definitions:

  • This bug occurred for the layout properties text-size and text-letter-spacing.
  • This bug didn't occur for the paint property text-color.
  • This bug didn't occur for one example of a cubic-bezier interpolation for text-size, and I don't know why.

Configuration

Mapbox SDK versions: Mapbox Maps SDK for iOS 4.4.0-alpha.1, Mapbox Maps SDK for Android 6.4.0 (I think – the versions in the latest Mapbox Studio Preview apps)

/cc @mapbox/gl-core @mapbox/maps-ios @mapbox/maps-android

@nickidlugash nickidlugash added bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Android Mapbox Maps SDK for Android labels Sep 4, 2018
@1ec5 1ec5 added Core The cross-platform C++ core, aka mbgl and removed Android Mapbox Maps SDK for Android iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Sep 4, 2018
@ChrisLoer
Copy link
Contributor

Just to be clear, the problem is not that interpolation isn't working in between integer zoom levels (text-size is a weird special case, and text-letter-spacing just can't do anything at all since we don't have a way to dynamically re-do layout at render time)? The problem is that even at integer zoom levels these layout properties aren't picking up the interpolated value you'd expect?

@nickidlugash
Copy link
Contributor Author

The problem is that even at integer zoom levels these layout properties aren't picking up the interpolated value you'd expect?

Yes. The behavior on native (I think) is that the last value is just used across all features in the style layer.

@ChrisLoer
Copy link
Contributor

So we do some special case evaluation for symbol sizes here:

// Even though we could get the exact value of the camera function
// at z = currentZoom, we intentionally do not: instead, we interpolate
// between the camera function values at a pair of zoom stops covering
// [tileZoom, tileZoom + 1] in order to be consistent with this
// restriction on composite functions.
const Range<float>& zoomLevels = std::get<0>(*coveringRanges);
const Range<float>& sizeLevels = std::get<1>(*coveringRanges);
float t = util::clamp(
expression->interpolationFactor(zoomLevels, currentZoom),
0.0f, 1.0f
);
size = sizeLevels.min + t * (sizeLevels.max - sizeLevels.min);

In your country-label example above, when you're at a low zoom (let's say 2), that zoomLevels will just be the first stop, and its min/max will be 4/4.

That gets passed into the CubicBezierInterpolator here:

double interpolationFactor(const Range<double>& inputLevels, const double input) const {
return ub.solve(input / (inputLevels.max - inputLevels.min), 1e-6);
}

... and since max - min == 0, it solves for infinity, which leads to an interpolation factor of 1, which leads to it choosing the size for the next stop (size 24 at zoom 9, instead of size 10 at zoom 4).

In contrast, GL JS uses the exponentialInterpolation function with a base of 1:

https://github.com/mapbox/mapbox-gl-js/blob/3978b6f35720dcc23970b6856f2b4c9d8e272d01/src/style-spec/expression/definitions/interpolate.js#L54

It's the same as native's util::interpolationFactor:

float interpolationFactor(float base, Range<float> range, float z) {
const float zoomDiff = range.max - range.min;
const float zoomProgress = z - range.min;
if (zoomDiff == 0) {
return 0;
} else if (base == 1.0f) {
return zoomProgress / zoomDiff;
} else {
return (std::pow(static_cast<double>(base), zoomProgress) - 1) /
(std::pow(static_cast<double>(base), zoomDiff) - 1);
}
}

The zoomDiff == 0 case there handles the case where our current zoom is entirely below the first stop, by forcing an interpolation factor of 0.

I think the fix is just to use the same interpolation factor in gl-native. You can tell from my very literal description of the problem that I don't have the strongest conceptual grasp of how this code is supposed to work.

I'll put together a PR and hopefully you can experiment to see you get the behavior you expect with it?

ChrisLoer added a commit that referenced this issue Sep 5, 2018
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 added a commit that referenced this issue Sep 6, 2018
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
Copy link
Contributor

Fixed with #12826.

ChrisLoer added a commit that referenced this issue Sep 6, 2018
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 added a commit that referenced this issue Sep 6, 2018
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.
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

No branches or pull requests

3 participants