From e361bcf66823daa9ae6cae571193a5ba9613c3de Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 16 Jun 2017 10:16:11 -0700 Subject: [core] Fix composite function approximation for non-integer stops --- include/mbgl/style/function/composite_function.hpp | 75 +++++++++++++++------- mapbox-gl-js | 2 +- src/mbgl/renderer/paint_property_binder.hpp | 10 +-- test/style/function/composite_function.test.cpp | 25 ++++++++ 4 files changed, 84 insertions(+), 28 deletions(-) diff --git a/include/mbgl/style/function/composite_function.hpp b/include/mbgl/style/function/composite_function.hpp index 1b785f4670..c3de0726c0 100644 --- a/include/mbgl/style/function/composite_function.hpp +++ b/include/mbgl/style/function/composite_function.hpp @@ -49,8 +49,16 @@ public: defaultValue(std::move(defaultValue_)) { } - std::tuple, Range> - coveringRanges(float zoom) const { + struct CoveringRanges { + float zoom; + Range coveringZoomRange; + Range coveringStopsRange; + }; + + // Return the relevant stop zoom values and inner stops that bracket a given zoom level. This + // is the first step toward evaluating the function, and is used for in the course of both partial + // evaluation of data-driven paint properties, and full evaluation of data-driven layout properties. + CoveringRanges coveringRanges(float zoom) const { return stops.match( [&] (const auto& s) { assert(!s.stops.empty()); @@ -63,7 +71,8 @@ public: minIt--; } - return std::make_tuple( + return CoveringRanges { + zoom, Range { minIt == s.stops.end() ? s.stops.rbegin()->first : minIt->first, maxIt == s.stops.end() ? s.stops.rbegin()->first : maxIt->first @@ -72,38 +81,49 @@ public: s.innerStops(minIt == s.stops.end() ? s.stops.rbegin()->second : minIt->second), s.innerStops(maxIt == s.stops.end() ? s.stops.rbegin()->second : maxIt->second) } - ); + }; } ); } + // Given a range of zoom values (typically two adjacent integer zoom levels, e.g. 5.0 and 6.0), + // return the covering ranges for both. This is used in the course of partial evaluation for + // data-driven paint properties. + Range rangeOfCoveringRanges(Range zoomRange) { + return Range { + coveringRanges(zoomRange.min), + coveringRanges(zoomRange.max) + }; + } + + // Given the covering ranges for range of zoom values (typically two adjacent integer zoom levels, + // e.g. 5.0 and 6.0), and a feature, return the results of fully evaluating the function for that + // feature at each of the two zoom levels. These two results are what go into the paint vertex buffers + // for vertices associated with this feature. The shader will interpolate between them at render time. template - Range evaluate(Range coveringStops, - const Feature& feature, - T finalDefaultValue) const { - optional v = feature.getValue(property); - if (!v) { - return { + Range evaluate(const Range& ranges, const Feature& feature, T finalDefaultValue) { + optional value = feature.getValue(property); + if (!value) { + return Range { defaultValue.value_or(finalDefaultValue), defaultValue.value_or(finalDefaultValue) }; } - auto eval = [&] (const auto& s) { - return s.evaluate(*v).value_or(defaultValue.value_or(finalDefaultValue)); - }; return Range { - coveringStops.min.match(eval), - coveringStops.max.match(eval) + evaluateFinal(ranges.min, *value, finalDefaultValue), + evaluateFinal(ranges.max, *value, finalDefaultValue) }; } - T evaluate(float zoom, const GeometryTileFeature& feature, T finalDefaultValue) const { - std::tuple, Range> ranges = coveringRanges(zoom); - Range resultRange = evaluate(std::get<1>(ranges), feature, finalDefaultValue); - return util::interpolate( - resultRange.min, - resultRange.max, - util::interpolationFactor(1.0f, std::get<0>(ranges), zoom)); + // Fully evaluate the function for a zoom value and feature. This is used when evaluating data-driven + // layout properties. + template + T evaluate(float zoom, const Feature& feature, T finalDefaultValue) const { + optional value = feature.getValue(property); + if (!value) { + return defaultValue.value_or(finalDefaultValue); + } + return evaluateFinal(coveringRanges(zoom), *value, finalDefaultValue); } friend bool operator==(const CompositeFunction& lhs, @@ -115,6 +135,17 @@ public: std::string property; Stops stops; optional defaultValue; + +private: + T evaluateFinal(const CoveringRanges& ranges, const Value& value, T finalDefaultValue) const { + auto eval = [&] (const auto& s) { + return s.evaluate(value).value_or(defaultValue.value_or(finalDefaultValue)); + }; + return util::interpolate( + ranges.coveringStopsRange.min.match(eval), + ranges.coveringStopsRange.max.match(eval), + util::interpolationFactor(1.0f, ranges.coveringZoomRange, ranges.zoom)); + } }; } // namespace style diff --git a/mapbox-gl-js b/mapbox-gl-js index 24938a6e67..a222ef16b5 160000 --- a/mapbox-gl-js +++ b/mapbox-gl-js @@ -1 +1 @@ -Subproject commit 24938a6e6767c880c709ee6ffb2d3ffbaad838cf +Subproject commit a222ef16b5a34b15a52e7c722dcdfceafdc62af4 diff --git a/src/mbgl/renderer/paint_property_binder.hpp b/src/mbgl/renderer/paint_property_binder.hpp index c22291767a..8ac6ce8cf1 100644 --- a/src/mbgl/renderer/paint_property_binder.hpp +++ b/src/mbgl/renderer/paint_property_binder.hpp @@ -189,11 +189,11 @@ public: CompositeFunctionPaintPropertyBinder(style::CompositeFunction function_, float zoom, T defaultValue_) : function(std::move(function_)), defaultValue(std::move(defaultValue_)), - coveringRanges(function.coveringRanges(zoom)) { + rangeOfCoveringRanges(function.rangeOfCoveringRanges({zoom, zoom + 1})) { } void populateVertexVector(const GeometryTileFeature& feature, std::size_t length) override { - Range range = function.evaluate(std::get<1>(coveringRanges), feature, defaultValue); + Range range = function.evaluate(rangeOfCoveringRanges, feature, defaultValue); this->statistics.add(range.min); this->statistics.add(range.max); AttributeValue value = zoomInterpolatedAttributeValue( @@ -217,7 +217,7 @@ public: } float interpolationFactor(float currentZoom) const override { - return util::interpolationFactor(1.0f, std::get<0>(coveringRanges), currentZoom); + return util::interpolationFactor(1.0f, { rangeOfCoveringRanges.min.zoom, rangeOfCoveringRanges.max.zoom }, currentZoom); } T uniformValue(const PossiblyEvaluatedPropertyValue& currentValue) const override { @@ -230,10 +230,10 @@ public: } private: - using InnerStops = typename style::CompositeFunction::InnerStops; style::CompositeFunction function; T defaultValue; - std::tuple, Range> coveringRanges; + using CoveringRanges = typename style::CompositeFunction::CoveringRanges; + Range rangeOfCoveringRanges; gl::VertexVector vertexVector; optional> vertexBuffer; }; diff --git a/test/style/function/composite_function.test.cpp b/test/style/function/composite_function.test.cpp index e0804d4b27..09b79a8b8f 100644 --- a/test/style/function/composite_function.test.cpp +++ b/test/style/function/composite_function.test.cpp @@ -44,3 +44,28 @@ TEST(CompositeFunction, ZoomInterpolation) { }), 0.0f) .evaluate(0.0f, oneInteger, -1.0f)) << "Should interpolate TO the first stop"; } + +TEST(CompositeFunction, Issue8460) { + CompositeFunction fn1("property", CompositeExponentialStops({ + {15.0f, {{uint64_t(1), 0.0f}}}, + {15.2f, {{uint64_t(1), 600.0f}}}, + }), 0.0f); + + EXPECT_NEAR( 0.0f, fn1.evaluate(15.0f, oneInteger, -1.0f), 0.00); + EXPECT_NEAR(300.0f, fn1.evaluate(15.1f, oneInteger, -1.0f), 0.01); + EXPECT_NEAR(600.0f, fn1.evaluate(15.2f, oneInteger, -1.0f), 0.00); + EXPECT_NEAR(600.0f, fn1.evaluate(16.0f, oneInteger, -1.0f), 0.00); + + CompositeFunction fn2("property", CompositeExponentialStops({ + {15.0f, {{uint64_t(1), 0.0f}}}, + {15.2f, {{uint64_t(1), 300.0f}}}, + {18.0f, {{uint64_t(1), 600.0f}}}, + }), 0.0f); + + EXPECT_NEAR( 0.0f, fn2.evaluate(15.0f, oneInteger, -1.0f), 0.00); + EXPECT_NEAR(150.0f, fn2.evaluate(15.1f, oneInteger, -1.0f), 0.01); + EXPECT_NEAR(300.0f, fn2.evaluate(15.2f, oneInteger, -1.0f), 0.00); + EXPECT_NEAR(385.71f, fn2.evaluate(16.0f, oneInteger, -1.0f), 0.01); + EXPECT_NEAR(600.0f, fn2.evaluate(18.0f, oneInteger, -1.0f), 0.00); + EXPECT_NEAR(600.0f, fn2.evaluate(19.0f, oneInteger, -1.0f), 0.00); +} -- cgit v1.2.1