Updated the way that multiply_alpha works, such that it is not clamped at the multiplier, but rather at the result of the multiplier and alpha.

benchmark/build.py

@@ -43,6 +43,7 @@ benchmarks = [
     "test_font_registration.cpp",
     "test_rendering.cpp",
     "test_rendering_shared_map.cpp",
+#    "test_numeric_cast_vs_static_cast.cpp",
 ]
 for cpp_test in benchmarks:
     test_program = test_env_local.Program('out/'+cpp_test.replace('.cpp',''), source=[cpp_test])

benchmark/test_numeric_cast_vs_static_cast.cpp

@@ -0,0 +1,87 @@
+#include "bench_framework.hpp"
+// boost
+#include <boost/numeric/conversion/cast.hpp>
+
+static double STEP_NUM = 0.0000000001;
+static std::uint8_t START_NUM = 2;
+
+class test_static : public benchmark::test_case
+{
+    double step_;
+    std::uint8_t start_;
+public:
+    test_static(mapnik::parameters const& params)
+     : test_case(params),
+       step_(STEP_NUM),
+       start_(START_NUM) {}
+    bool validate() const
+    {
+        return true;
+    }
+    bool operator()() const
+    {
+        double value_ = 0.0;
+        std::uint8_t x;
+        for (std::size_t i=0;i<iterations_;++i) {
+            double c = static_cast<double>(start_) * value_;
+            if (c >= 256.0) c = 255.0;
+            if (c < 0.0) c = 0.0;
+            x = static_cast<std::uint8_t>(c);
+            value_ += step_;
+        }
+        return static_cast<double>(x) < (static_cast<double>(start_) * value_);
+    }
+};
+
+using boost::numeric::positive_overflow;
+using boost::numeric::negative_overflow;
+
+class test_numeric : public benchmark::test_case
+{
+    double step_;
+    std::uint8_t start_;
+public:
+    test_numeric(mapnik::parameters const& params)
+     : test_case(params),
+       step_(STEP_NUM),
+       start_(START_NUM) {}
+    bool validate() const
+    {
+        return true;
+    }
+    bool operator()() const
+    {
+        double value_ = 0.0;
+        std::uint8_t x;
+        for (std::size_t i=0;i<iterations_;++i) {
+            try {
+                x = boost::numeric_cast<std::uint8_t>(start_ * value_);
+            }
+            catch(negative_overflow&)
+            {
+                x = std::numeric_limits<std::uint8_t>::min();
+            }
+            catch(positive_overflow&)
+            {
+                x = std::numeric_limits<std::uint8_t>::max();
+            }
+            value_ += step_;
+        }
+        return static_cast<double>(x) < (static_cast<double>(start_) * value_);
+    }
+};
+
+int main(int argc, char** argv)
+{
+    mapnik::parameters params;
+    benchmark::handle_args(argc,argv,params);
+    {
+        test_static test_runner(params);
+        run(test_runner,"static_cast");
+    }
+    {
+        test_numeric test_runner(params);
+        run(test_runner,"numeric_cast");
+    }
+    return 0;
+}

src/image_util.cpp

@@ -713,7 +713,7 @@ namespace detail {
 struct visitor_multiply_alpha
 {
     visitor_multiply_alpha(float opacity)
-        : opacity_(clamp(opacity, 0.0f, 1.0f)) {}
+        : opacity_(opacity) {}
 
     void operator() (image_rgba8 & data) const
     {
@@ -724,7 +724,8 @@ struct visitor_multiply_alpha
             for (std::size_t x = 0; x < data.width(); ++x)
             {
                 pixel_type rgba = row_to[x];
-                pixel_type a = static_cast<uint8_t>(((rgba >> 24) & 0xff) * opacity_);
+                double new_a = static_cast<double>((rgba >> 24) & 0xff) * opacity_;
+                pixel_type a = static_cast<uint8_t>(clamp(new_a, 0.0, 255.0));
                 pixel_type r = rgba & 0xff;
                 pixel_type g = (rgba >> 8 ) & 0xff;
                 pixel_type b = (rgba >> 16) & 0xff;

test/unit/imaging/image_multiply_alpha.cpp

@@ -71,7 +71,7 @@ SECTION("test rgba8 overflow") {
     CHECK(static_cast<int>(out.red()) == 128);
     CHECK(static_cast<int>(out.green()) == 128);
     CHECK(static_cast<int>(out.blue()) == 128);
-    CHECK(static_cast<int>(out.alpha()) == 128);
+    CHECK(static_cast<int>(out.alpha()) == 255);
 
 } // END SECTION