BuildingSymbolizer: apply new stroke/fill properties

- change `render_building_symbolizer` namespace disguised as struct
  into class holding computed rendering parameters

- unify extraction of symbolizer properties (height, fill, etc.)

- miterlimit is a ratio of lengths (or alternatively, 1/sin(angle/2)),
  it should not depend on scale_factor;
  besides, the minimum sensible value is 1.0 (because sin(x) <= 1)
  and greater values produce spikes around walls with large |dy/dx|
  ratio due to the way they're stroked

include/mapnik/renderer_common/process_building_symbolizer.hpp

@@ -24,6 +24,8 @@
 #define MAPNIK_RENDERER_COMMON_PROCESS_BUILDING_SYMBOLIZER_HPP
 
 #include <mapnik/feature.hpp>
+#include <mapnik/renderer_common.hpp>
+#include <mapnik/symbolizer_base.hpp>
 #include <mapnik/vertex_adapters.hpp>
 #include <mapnik/path.hpp>
 #include <mapnik/transform_path_adapter.hpp>
@@ -41,20 +43,99 @@ struct render_building_symbolizer
     using transform_path_type = transform_path_adapter<view_transform, vertex_adapter_type>;
     using roof_type = agg::conv_transform<transform_path_type>;
 
+private:
+
+    using size_t = std::size_t;
+    using uint8_t = std::uint8_t;
+
+    renderer_common const& rencom_;
+    double const height_;
+
+public:
+
+    color fill_color;
+    color wall_fill_color;
+    color stroke_color;
+    color base_stroke_color;
+    double stroke_width = symbolizer_default<double, keys::stroke_width>::value();
+
+    render_building_symbolizer(building_symbolizer const& sym,
+                               feature_impl const& feature,
+                               renderer_common const& rencom)
+      : rencom_(rencom)
+      , height_(get<double, keys::height>(sym, feature, rencom_.vars_)
+                * rencom_.scale_factor_)
+      , stroke_width(get<double, keys::stroke_width>(sym, feature, rencom_.vars_)
+                     * rencom_.scale_factor_)
+    {
+        // colors are not always needed so they're not extracted here
+    }
+
+    void setup_colors(building_symbolizer const& sym,
+                      feature_impl const& feature)
+    {
+        auto const& vars = rencom_.vars_;
+
+        // `fill` default is defined in `symbolizer_default` specialization
+        fill_color = get<color, keys::fill>(sym, feature, vars);
+
+        // `wall-fill` defaults to dimmed `fill` [backward compatibility]
+        if (auto opt = get_optional<color>(sym, keys::wall_fill, feature, vars))
+        {
+            wall_fill_color = *opt;
+        }
+        else
+        {
+            wall_fill_color = fill_color;
+            safe_mul(wall_fill_color.red_, 0.8);
+            safe_mul(wall_fill_color.green_, 0.8);
+            safe_mul(wall_fill_color.blue_, 0.8);
+        }
+
+        // `stroke` defaults to dimmed `fill` [backward compatibility]
+        if (auto opt = get_optional<color>(sym, keys::stroke, feature, vars))
+        {
+            stroke_color = *opt;
+        }
+        else
+        {
+            stroke_color = fill_color;
+            safe_mul(stroke_color.red_, 0.8);
+            safe_mul(stroke_color.green_, 0.8);
+            safe_mul(stroke_color.blue_, 0.8);
+        }
+
+        // `base-stroke` defaults to `stroke` [backward compatibility]
+        if (auto opt = get_optional<color>(sym, keys::base_stroke, feature, vars))
+        {
+            base_stroke_color = *opt;
+        }
+        else
+        {
+            base_stroke_color = stroke_color;
+        }
+
+        double fill_opacity = get<double, keys::fill_opacity>(sym, feature, vars);
+        safe_mul(fill_color.alpha_, fill_opacity);
+        safe_mul(wall_fill_color.alpha_, fill_opacity);
+
+        double stroke_opacity = get<double, keys::stroke_opacity>(sym, feature, vars);
+        safe_mul(stroke_color.alpha_, stroke_opacity);
+        safe_mul(base_stroke_color.alpha_, stroke_opacity);
+    }
+
     template <typename F1, typename F2, typename F3>
-    static void apply(feature_impl const& feature,
-                      proj_transform const& prj_trans,
-                      view_transform const& view_trans,
-                      double height,
-                      F1 face_func, F2 frame_func, F3 roof_func)
+    void apply(feature_impl const& feature,
+               proj_transform const& prj_trans,
+               F1 face_func, F2 frame_func, F3 roof_func)
     {
         auto const& geom = feature.get_geometry();
         if (geom.is<geometry::polygon<double>>())
         {
             auto const& poly = geom.get<geometry::polygon<double>>();
             vertex_adapter_type va(poly);
-            transform_path_type transformed(view_trans, va, prj_trans);
-            make_building(transformed, height, face_func, frame_func, roof_func);
+            transform_path_type transformed(rencom_.t_, va, prj_trans);
+            make_building(transformed, face_func, frame_func, roof_func);
         }
         else if (geom.is<geometry::multi_polygon<double>>())
         {
@@ -62,31 +143,31 @@ struct render_building_symbolizer
             for (auto const& poly : multi_poly)
             {
                 vertex_adapter_type va(poly);
-                transform_path_type transformed(view_trans, va, prj_trans);
-                make_building(transformed, height, face_func, frame_func, roof_func);
+                transform_path_type transformed(rencom_.t_, va, prj_trans);
+                make_building(transformed, face_func, frame_func, roof_func);
             }
         }
     }
 
 private:
     template <typename F>
-    static void render_face(double x0, double y0, double x, double y, double height, F const& face_func, path_type & frame)
+    void render_face(double x0, double y0, double x, double y, F & face_func, path_type & frame)
     {
         path_type faces(path_type::types::Polygon);
         faces.move_to(x0, y0);
         faces.line_to(x, y);
-        faces.line_to(x, y - height);
-        faces.line_to(x0, y0 - height);
-        face_func(faces);
+        faces.line_to(x, y - height_);
+        faces.line_to(x0, y0 - height_);
+        face_func(faces, wall_fill_color);
 
         frame.move_to(x0, y0);
         frame.line_to(x, y);
-        frame.line_to(x, y - height);
-        frame.line_to(x0, y0 - height);
+        frame.line_to(x, y - height_);
+        frame.line_to(x0, y0 - height_);
     }
 
     template <typename Geom, typename F1, typename F2, typename F3>
-    static void make_building(Geom & poly, double height, F1 const& face_func, F2 const& frame_func, F3 const& roof_func)
+    void make_building(Geom & poly, F1 & face_func, F2 & frame_func, F3 & roof_func)
     {
         path_type frame(path_type::types::LineString);
         double ring_begin_x, ring_begin_y;
@@ -103,21 +184,33 @@ private:
             }
             else if (cm == SEG_LINETO)
             {
-                render_face(x0, y0, x, y, height, face_func, frame);
+                render_face(x0, y0, x, y, face_func, frame);
             }
             else if (cm == SEG_CLOSE)
             {
-                render_face(x0, y0, ring_begin_x, ring_begin_y, height, face_func, frame);
+                render_face(x0, y0, ring_begin_x, ring_begin_y, face_func, frame);
             }
             x0 = x;
             y0 = y;
         }
 
-        frame_func(frame);
+        frame_func(frame, wall_fill_color);
 
-        agg::trans_affine_translation tr(0, -height);
+        agg::trans_affine_translation tr(0, -height_);
         roof_type roof(poly, tr);
-        roof_func(roof);
+        roof_func(roof, fill_color);
+    }
+
+    static void safe_mul(uint8_t & v, double opacity)
+    {
+        if (opacity <= 0)
+        {
+            v = 0;
+        }
+        else if (opacity < 1)
+        {
+            v = static_cast<uint8_t>(v * opacity + 0.5);
+        }
     }
 };
 

src/agg/process_building_symbolizer.cpp

@@ -63,17 +63,10 @@ void agg_renderer<T0,T1>::process(building_symbolizer const& sym,
     agg::rendering_buffer buf(current_buffer.bytes(), current_buffer.width(), current_buffer.height(), current_buffer.row_size());
     agg::pixfmt_rgba32_pre pixf(buf);
     ren_base renb(pixf);
-
-    value_double opacity = get<value_double,keys::fill_opacity>(sym,feature, common_.vars_);
-    color const& fill = get<color, keys::fill>(sym, feature, common_.vars_);
-    unsigned r=fill.red();
-    unsigned g=fill.green();
-    unsigned b=fill.blue();
-    unsigned a=fill.alpha();
     renderer ren(renb);
     agg::scanline_u8 sl;
+    render_building_symbolizer rebus{sym, feature, common_};
 
-    ras_ptr->reset();
     double gamma = get<value_double, keys::gamma>(sym, feature, common_.vars_);
     gamma_method_enum gamma_method = get<gamma_method_enum, keys::gamma_method>(sym, feature, common_.vars_);
     if (gamma != gamma_ || gamma_method != gamma_method_)
@@ -83,33 +76,34 @@ void agg_renderer<T0,T1>::process(building_symbolizer const& sym,
         gamma_ = gamma;
     }
 
-    double height = get<double, keys::height>(sym, feature, common_.vars_) * common_.scale_factor_;
+    rebus.setup_colors(sym, feature);
 
-    render_building_symbolizer::apply(
-        feature, prj_trans, common_.t_, height,
-        [&,r,g,b,a,opacity](path_type const& faces)
+    rebus.apply(
+        feature, prj_trans,
+        [&](path_type const& faces, color const& c)
         {
             vertex_adapter va(faces);
+            ras_ptr->reset();
             ras_ptr->add_path(va);
-            ren.color(agg::rgba8_pre(int(r*0.8), int(g*0.8), int(b*0.8), int(a * opacity)));
+            ren.color(agg::rgba8_pre(c.red(), c.green(), c.blue(), c.alpha()));
             agg::render_scanlines(*ras_ptr, sl, ren);
-            this->ras_ptr->reset();
         },
-        [&,r,g,b,a,opacity](path_type const& frame)
+        [&](path_type const& frame, color const& c)
         {
             vertex_adapter va(frame);
             agg::conv_stroke<vertex_adapter> stroke(va);
-            stroke.width(common_.scale_factor_);
-            stroke.miter_limit(common_.scale_factor_ / 2.0);
-            ras_ptr->add_path(stroke);
-            ren.color(agg::rgba8_pre(int(r*0.8), int(g*0.8), int(b*0.8), int(a * opacity)));
-            agg::render_scanlines(*ras_ptr, sl, ren);
+            stroke.width(rebus.stroke_width);
+            stroke.miter_limit(1.0);
             ras_ptr->reset();
+            ras_ptr->add_path(stroke);
+            ren.color(agg::rgba8_pre(c.red(), c.green(), c.blue(), c.alpha()));
+            agg::render_scanlines(*ras_ptr, sl, ren);
         },
-        [&,r,g,b,a,opacity](render_building_symbolizer::roof_type & roof)
+        [&](render_building_symbolizer::roof_type & roof, color const& c)
         {
+            ras_ptr->reset();
             ras_ptr->add_path(roof);
-            ren.color(agg::rgba8_pre(r, g, b, int(a * opacity)));
+            ren.color(agg::rgba8_pre(c.red(), c.green(), c.blue(), c.alpha()));
             agg::render_scanlines(*ras_ptr, sl, ren);
         });
 }

src/cairo/process_building_symbolizer.cpp

@@ -44,36 +44,34 @@ void cairo_renderer<T>::process(building_symbolizer const& sym,
                                   proj_transform const& prj_trans)
 {
     cairo_save_restore guard(context_);
-    composite_mode_e comp_op = get<composite_mode_e, keys::comp_op>(sym, feature, common_.vars_);
-    mapnik::color fill = get<color, keys::fill>(sym, feature, common_.vars_);
-    value_double opacity = get<value_double, keys::fill_opacity>(sym, feature, common_.vars_);
-    value_double height = get<value_double, keys::height>(sym, feature, common_.vars_);
+    render_building_symbolizer rebus{sym, feature, common_};
 
+    composite_mode_e comp_op = get<composite_mode_e, keys::comp_op>(sym, feature, common_.vars_);
     context_.set_operator(comp_op);
 
-    render_building_symbolizer::apply(
-        feature, prj_trans, common_.t_, height,
-        [&](path_type const& faces)
+    rebus.setup_colors(sym, feature);
+
+    rebus.apply(
+        feature, prj_trans,
+        [&](path_type const& faces, color const& c)
         {
             vertex_adapter va(faces);
-            context_.set_color(fill.red()  * 0.8 / 255.0, fill.green() * 0.8 / 255.0,
-                               fill.blue() * 0.8 / 255.0, fill.alpha() * opacity / 255.0);
+            context_.set_color(c);
             context_.add_path(va);
             context_.fill();
         },
-        [&](path_type const& frame)
+        [&](path_type const& frame, color const& c)
         {
             vertex_adapter va(frame);
-            context_.set_color(fill.red()  * 0.8 / 255.0, fill.green() * 0.8/255.0,
-                              fill.blue() * 0.8 / 255.0, fill.alpha() * opacity / 255.0);
-            context_.set_line_width(common_.scale_factor_);
-            context_.set_miter_limit(common_.scale_factor_ / 2.0);
+            context_.set_color(c);
+            context_.set_line_width(rebus.stroke_width);
+            context_.set_miter_limit(1.0);
             context_.add_path(va);
             context_.stroke();
         },
-        [&](render_building_symbolizer::roof_type & roof)
+        [&](render_building_symbolizer::roof_type & roof, color const& c)
         {
-            context_.set_color(fill, opacity);
+            context_.set_color(c);
             context_.add_path(roof);
             context_.fill();
         });

src/grid/process_building_symbolizer.cpp

@@ -65,32 +65,32 @@ void grid_renderer<T>::process(building_symbolizer const& sym,
     grid_renderer_base_type renb(pixf);
     renderer_type ren(renb);
 
-    ras_ptr->reset();
+    render_building_symbolizer rebus{sym, feature, common_};
 
-    double height = get<value_double>(sym, keys::height, feature, common_.vars_, 0.0);
-
-    render_building_symbolizer::apply(
-        feature, prj_trans, common_.t_, height,
-        [&](path_type const& faces)
+    rebus.apply(
+        feature, prj_trans,
+        [&](path_type const& faces, color const& )
         {
             vertex_adapter va(faces);
+            ras_ptr->reset();
             ras_ptr->add_path(va);
             ren.color(color_type(feature.id()));
             agg::render_scanlines(*ras_ptr, sl, ren);
-            ras_ptr->reset();
         },
-        [&](path_type const& frame)
+        [&](path_type const& frame, color const& )
         {
             vertex_adapter va(frame);
             agg::conv_stroke<vertex_adapter> stroke(va);
-            stroke.miter_limit(common_.scale_factor_ / 2.0);
+            stroke.width(rebus.stroke_width);
+            stroke.miter_limit(1.0);
+            ras_ptr->reset();
             ras_ptr->add_path(stroke);
             ren.color(color_type(feature.id()));
             agg::render_scanlines(*ras_ptr, sl, ren);
-            ras_ptr->reset();
         },
-        [&](render_building_symbolizer::roof_type & roof)
+        [&](render_building_symbolizer::roof_type & roof, color const& )
         {
+            ras_ptr->reset();
             ras_ptr->add_path(roof);
             ren.color(color_type(feature.id()));
             agg::render_scanlines(*ras_ptr, sl, ren);