diff options
author | Anand Thakker <anandthakker@users.noreply.github.com> | 2017-06-09 06:28:14 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-06-09 06:28:14 -0400 |
commit | f59eb822e12b54f74e688a138dc8f4ede1feade8 (patch) | |
tree | 7d0d9039e96fd7ffef8ff89c85c7dcf639737da2 | |
parent | 66c2a2a0b7c42e1947f57a412ece66fa8eaac149 (diff) | |
download | qtlocation-mapboxgl-f59eb822e12b54f74e688a138dc8f4ede1feade8.tar.gz |
Fix undefined memory access in getCoveringRanges() (#9227)
* Add simple unit tests for SymbolSizeBinder
* Fix bug in symbol size uniform value calculation
For camera functions we were setting the zoom levels in "covering ranges" to
`[(zoom stop <= tile zoom), (zoom stop >= 1 + tile zoom)]`, but then evaluating
the function at `[tile_zoom, tile_zoom + 1]`.
* Check for it != end() before accessing it->first
-rw-r--r-- | cmake/test-files.cmake | 1 | ||||
-rw-r--r-- | include/mbgl/style/function/composite_function.hpp | 2 | ||||
-rw-r--r-- | src/mbgl/programs/symbol_program.hpp | 7 | ||||
-rw-r--r-- | test/programs/symbol_program.test.cpp | 61 |
4 files changed, 67 insertions, 4 deletions
diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index c0673bedef..093fc56dbd 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -40,6 +40,7 @@ set(MBGL_TEST_FILES # programs test/programs/binary_program.test.cpp + test/programs/symbol_program.test.cpp # renderer test/renderer/group_by_layout.test.cpp diff --git a/include/mbgl/style/function/composite_function.hpp b/include/mbgl/style/function/composite_function.hpp index 9275ccdf8f..1b785f4670 100644 --- a/include/mbgl/style/function/composite_function.hpp +++ b/include/mbgl/style/function/composite_function.hpp @@ -59,7 +59,7 @@ public: // lower_bound yields first element >= zoom, but we want the *last* // element <= zoom, so if we found a stop > zoom, back up by one. - if (minIt != s.stops.begin() && minIt->first > zoom) { + if (minIt != s.stops.begin() && minIt != s.stops.end() && minIt->first > zoom) { minIt--; } diff --git a/src/mbgl/programs/symbol_program.hpp b/src/mbgl/programs/symbol_program.hpp index 48928988e6..f466a076ef 100644 --- a/src/mbgl/programs/symbol_program.hpp +++ b/src/mbgl/programs/symbol_program.hpp @@ -121,7 +121,7 @@ Range<float> getCoveringStops(Stops s, float lowerZoom, float upperZoom) { // lower_bound yields first element >= lowerZoom, but we want the *last* // element <= lowerZoom, so if we found a stop > lowerZoom, back up by one. - if (minIt != s.stops.begin() && minIt->first > lowerZoom) { + if (minIt != s.stops.begin() && minIt != s.stops.end() && minIt->first > lowerZoom) { minIt--; } return Range<float> { @@ -144,9 +144,10 @@ public: : layoutSize(function_.evaluate(tileZoom + 1)) { function_.stops.match( [&] (const style::ExponentialStops<float>& stops) { + const auto& zoomLevels = getCoveringStops(stops, tileZoom, tileZoom + 1); coveringRanges = std::make_tuple( - getCoveringStops(stops, tileZoom, tileZoom + 1), - Range<float> { function_.evaluate(tileZoom), function_.evaluate(tileZoom + 1) } + zoomLevels, + Range<float> { function_.evaluate(zoomLevels.min), function_.evaluate(zoomLevels.max) } ); functionInterpolationBase = stops.base; }, diff --git a/test/programs/symbol_program.test.cpp b/test/programs/symbol_program.test.cpp new file mode 100644 index 0000000000..ef1e71c269 --- /dev/null +++ b/test/programs/symbol_program.test.cpp @@ -0,0 +1,61 @@ +#include <mbgl/test/util.hpp> + +#include <mbgl/programs/symbol_program.hpp> + +using namespace mbgl; + +TEST(SymbolProgram, SymbolSizeBinder) { + auto binder = SymbolSizeBinder::create(5.0f, 12.0f, 0.0f); + auto uniformValues = binder->uniformValues(5.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 12.0f); + EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 12.0f); + + binder = SymbolSizeBinder::create(1.0f, style::CameraFunction<float>(style::ExponentialStops<float>({ + {0.0f, 8.0f}, + {10.0f, 18.0f} + }, 1.0f)), 0.0f); + uniformValues = binder->uniformValues(1.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 9.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 10.0f); + + binder = SymbolSizeBinder::create(0.0f, style::CameraFunction<float>(style::ExponentialStops<float>({ + {1.0f, 8.0f}, + {11.0f, 18.0f} + }, 1.0f)), 0.0f); + uniformValues = binder->uniformValues(0.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 8.0f); + EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 8.0f); + + binder = SymbolSizeBinder::create(12.0f, style::CameraFunction<float>(style::ExponentialStops<float>({ + {1.0f, 8.0f}, + {11.0f, 18.0f} + }, 1.0f)), 0.0f); + uniformValues = binder->uniformValues(12.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_size>().t, 18.0f); + EXPECT_EQ(uniformValues.get<uniforms::u_layout_size>().t, 18.0f); + + binder = SymbolSizeBinder::create(0.0f, style::SourceFunction<float>("x", style::ExponentialStops<float>({ + {1.0f, 8.0f}, + {11.0f, 18.0f} + }, 1.0f)), 0.0f); + uniformValues = binder->uniformValues(12.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, true); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, false); + + binder = SymbolSizeBinder::create(5.0f, style::CompositeFunction<float>("x", style::CompositeExponentialStops<float>({ + {1.0f, {{0.0f, 8.0f}, {100.0f, 18.0f}}}, + {11.0f, {{0.0f, 12.0f}, {100.0f, 24.9f}}} + }, 1.0f)), 0.0f); + uniformValues = binder->uniformValues(5.5f); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_zoom_constant>().t, false); + EXPECT_EQ(uniformValues.get<uniforms::u_is_size_feature_constant>().t, false); + EXPECT_EQ(uniformValues.get<uniforms::u_size_t>().t, 0.45f); +} |