From d29a0f18b1fb9c4a278a80696d6469086d5c7773 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 12 Jun 2015 00:02:59 -0700 Subject: [PATCH 1/3] fix pixel_cast by avoiding comparing across sign<->unsigned - refs #2893 --- include/mapnik/pixel_cast.hpp | 119 ++++++++++++++++++++++++++++++++-- 1 file changed, 113 insertions(+), 6 deletions(-) diff --git a/include/mapnik/pixel_cast.hpp b/include/mapnik/pixel_cast.hpp index efa76d9bc..d5c022d37 100644 --- a/include/mapnik/pixel_cast.hpp +++ b/include/mapnik/pixel_cast.hpp @@ -23,23 +23,130 @@ #ifndef MAPNIK_PIXEL_CAST_HPP #define MAPNIK_PIXEL_CAST_HPP +#include #include namespace mapnik { +namespace detail { + + template + struct numeric_compare; + + template + struct numeric_compare_same_sign + { + using sizeup = typename std::conditional= sizeof(S), T, S>::type; + + static inline bool less(T t, S s) { + return static_cast(t) < static_cast(s); + } + + static inline bool greater(T t, S s) { + return static_cast(t) > static_cast(s); + } + }; + + template + struct numeric_compare::value && !std::is_floating_point::value && + ((std::is_unsigned::value && std::is_unsigned::value) + || (std::is_signed::value && std::is_signed::value))> + ::type> : numeric_compare_same_sign + {}; + + + template + struct numeric_compare::value && !std::is_floating_point::value && + std::is_integral::value && std::is_signed::value && std::is_unsigned::value>::type> + { + static inline bool less(T t, S s) { + return (t < static_cast(0)) ? true : static_cast(t) < static_cast(s); + } + + static inline bool greater(T t, S s) { + return (t < static_cast(0)) ? false : static_cast(t) > static_cast(s); + } + }; + + template + struct numeric_compare::value && !std::is_floating_point::value && + std::is_integral::value && std::is_unsigned::value && std::is_signed::value>::type> + { + static inline bool less(T t, S s) { + return (s < static_cast(0)) ? false : static_cast(t) < static_cast(s); + } + + static inline bool greater(T t, S s) { + return (s < static_cast(0)) ? true : static_cast(t) > static_cast(s); + } + }; + + template + struct numeric_compare::value && std::is_floating_point::value>::type> + { + static inline bool less(T t, S s) { + return t < s; + } + + static inline bool greater(T t, S s) { + return t > s; + } + }; + + template + struct numeric_compare::value && std::is_integral::value>::type> + { + static inline bool less(T t, S s) { + return less(static_cast(t),static_cast(s)); + } + + static inline bool greater(T t, S s) { + return greater(static_cast(t),static_cast(s)); + } + }; + + + template + struct numeric_compare::value && std::is_floating_point::value>::type> + { + static inline bool less(T t, S s) { + return less(static_cast(t),static_cast(s)); + } + + static inline bool greater(T t, S s) { + return greater(static_cast(t),static_cast(s)); + } + }; + + template + inline bool less(T t, S s) { + return numeric_compare::less(t,s); + } + + template + inline bool greater(T t, S s) { + return numeric_compare::greater(t,s); + } +} + + template inline T pixel_cast(S s) { - using namespace boost::numeric; - if (s > bounds::highest() ) + static T max_val = boost::numeric::bounds::highest(); + if (detail::greater(s,max_val)) { - return bounds::highest(); + return max_val; } - else if (s < bounds::lowest()) + static T min_val = boost::numeric::bounds::lowest(); + if (detail::less(s,min_val)) { - return bounds::lowest(); + return min_val; + } + else + { + return static_cast(s); } - else return static_cast(s); } } // ns mapnik From 8e89b788dfa3f5f48e96258ab063859c9bb4b508 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 12 Jun 2015 00:12:28 -0700 Subject: [PATCH 2/3] Add a a few tests for set/get pixel - refs #2893 --- test/unit/imaging/image_set_pixel.cpp | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 test/unit/imaging/image_set_pixel.cpp diff --git a/test/unit/imaging/image_set_pixel.cpp b/test/unit/imaging/image_set_pixel.cpp new file mode 100644 index 000000000..ed7ceaffc --- /dev/null +++ b/test/unit/imaging/image_set_pixel.cpp @@ -0,0 +1,28 @@ +#include "catch.hpp" + +// mapnik +#include +#include +#include +#include + + +TEST_CASE("image set_pixel") { + +SECTION("test gray32") { + mapnik::image_gray32 im(256,256); + mapnik::set_pixel(im, 0, 0, -1); + auto pixel = mapnik::get_pixel(im, 0, 0); + INFO( pixel ); + CHECK( pixel == 0 ); +} + +SECTION("test gray8s") { + mapnik::image_gray8s im(256,256); + mapnik::set_pixel(im, 0, 0, std::numeric_limits::max()+1); + auto pixel = mapnik::get_pixel(im, 0, 0); + INFO( pixel ); + CHECK( (int)pixel == (int)std::numeric_limits::max() ); +} + +} \ No newline at end of file From 15041ba334e84575f7485eb3387a224ce5da94ef Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 12 Jun 2015 13:40:47 +0100 Subject: [PATCH 3/3] replace boost::numeric::bounds with detail::bounds where highest/lowest return constexpr ( since c++11 std::numeric_limits::min/max return `static constexpr T`) prefer if/else if.. over if/if...) --- include/mapnik/pixel_cast.hpp | 209 ++++++++++++++++++---------------- 1 file changed, 113 insertions(+), 96 deletions(-) diff --git a/include/mapnik/pixel_cast.hpp b/include/mapnik/pixel_cast.hpp index d5c022d37..23d465975 100644 --- a/include/mapnik/pixel_cast.hpp +++ b/include/mapnik/pixel_cast.hpp @@ -24,122 +24,139 @@ #define MAPNIK_PIXEL_CAST_HPP #include -#include namespace mapnik { namespace detail { - template - struct numeric_compare; +template +struct numeric_compare; - template - struct numeric_compare_same_sign - { - using sizeup = typename std::conditional= sizeof(S), T, S>::type; +template +struct numeric_compare_same_sign +{ + using sizeup = typename std::conditional= sizeof(S), T, S>::type; - static inline bool less(T t, S s) { - return static_cast(t) < static_cast(s); - } - - static inline bool greater(T t, S s) { - return static_cast(t) > static_cast(s); - } - }; - - template - struct numeric_compare::value && !std::is_floating_point::value && - ((std::is_unsigned::value && std::is_unsigned::value) - || (std::is_signed::value && std::is_signed::value))> - ::type> : numeric_compare_same_sign - {}; - - - template - struct numeric_compare::value && !std::is_floating_point::value && - std::is_integral::value && std::is_signed::value && std::is_unsigned::value>::type> - { - static inline bool less(T t, S s) { - return (t < static_cast(0)) ? true : static_cast(t) < static_cast(s); - } - - static inline bool greater(T t, S s) { - return (t < static_cast(0)) ? false : static_cast(t) > static_cast(s); - } - }; - - template - struct numeric_compare::value && !std::is_floating_point::value && - std::is_integral::value && std::is_unsigned::value && std::is_signed::value>::type> - { - static inline bool less(T t, S s) { - return (s < static_cast(0)) ? false : static_cast(t) < static_cast(s); - } - - static inline bool greater(T t, S s) { - return (s < static_cast(0)) ? true : static_cast(t) > static_cast(s); - } - }; - - template - struct numeric_compare::value && std::is_floating_point::value>::type> - { - static inline bool less(T t, S s) { - return t < s; - } - - static inline bool greater(T t, S s) { - return t > s; - } - }; - - template - struct numeric_compare::value && std::is_integral::value>::type> - { - static inline bool less(T t, S s) { - return less(static_cast(t),static_cast(s)); - } - - static inline bool greater(T t, S s) { - return greater(static_cast(t),static_cast(s)); - } - }; - - - template - struct numeric_compare::value && std::is_floating_point::value>::type> - { - static inline bool less(T t, S s) { - return less(static_cast(t),static_cast(s)); - } - - static inline bool greater(T t, S s) { - return greater(static_cast(t),static_cast(s)); - } - }; - - template - inline bool less(T t, S s) { - return numeric_compare::less(t,s); + static inline bool less(T t, S s) { + return static_cast(t) < static_cast(s); } - template - inline bool greater(T t, S s) { - return numeric_compare::greater(t,s); + static inline bool greater(T t, S s) { + return static_cast(t) > static_cast(s); } +}; + +template +struct numeric_compare::value && !std::is_floating_point::value && + ((std::is_unsigned::value && std::is_unsigned::value) + || (std::is_signed::value && std::is_signed::value))> + ::type> : numeric_compare_same_sign +{}; + + +template +struct numeric_compare::value && !std::is_floating_point::value && + std::is_integral::value && std::is_signed::value && std::is_unsigned::value>::type> +{ + static inline bool less(T t, S s) { + return (t < static_cast(0)) ? true : static_cast(t) < static_cast(s); + } + + static inline bool greater(T t, S s) { + return (t < static_cast(0)) ? false : static_cast(t) > static_cast(s); + } +}; + +template +struct numeric_compare::value && !std::is_floating_point::value && + std::is_integral::value && std::is_unsigned::value && std::is_signed::value>::type> +{ + static inline bool less(T t, S s) { + return (s < static_cast(0)) ? false : static_cast(t) < static_cast(s); + } + + static inline bool greater(T t, S s) { + return (s < static_cast(0)) ? true : static_cast(t) > static_cast(s); + } +}; + +template +struct numeric_compare::value && std::is_floating_point::value>::type> +{ + static inline bool less(T t, S s) { + return t < s; + } + + static inline bool greater(T t, S s) { + return t > s; + } +}; + +template +struct numeric_compare::value && std::is_integral::value>::type> +{ + static inline bool less(T t, S s) { + return less(static_cast(t),static_cast(s)); + } + + static inline bool greater(T t, S s) { + return greater(static_cast(t),static_cast(s)); + } +}; + + +template +struct numeric_compare::value && std::is_floating_point::value>::type> +{ + static inline bool less(T t, S s) { + return less(static_cast(t),static_cast(s)); + } + + static inline bool greater(T t, S s) { + return greater(static_cast(t),static_cast(s)); + } +}; + +template +inline bool less(T t, S s) { + return numeric_compare::less(t,s); } +template +inline bool greater(T t, S s) { + return numeric_compare::greater(t,s); +} + +// floats +template +struct bounds +{ + static constexpr T lowest() { return static_cast(-std::numeric_limits::max());} + static constexpr T highest() { return std::numeric_limits::max();} +}; + +// integers +template +struct bounds::is_integer>::type > +{ + static constexpr T lowest() { return std::numeric_limits::min();} + static constexpr T highest() { return std::numeric_limits::max();} +}; + +} // ns detail + template inline T pixel_cast(S s) { - static T max_val = boost::numeric::bounds::highest(); + static constexpr auto max_val = detail::bounds::highest(); + static constexpr auto min_val = detail::bounds::lowest(); + if (detail::greater(s,max_val)) { return max_val; } - static T min_val = boost::numeric::bounds::lowest(); - if (detail::less(s,min_val)) + else if (detail::less(s,min_val)) { return min_val; }