From 8ebd80436031213628ac9f37f9bc938dc813497e Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Mon, 30 Sep 2013 13:30:16 -0700 Subject: [PATCH 01/17] remove tabs --- include/mapnik/feature_style_processor.hpp | 20 +++++++++---------- .../mapnik/feature_style_processor_impl.hpp | 8 +++----- tests/python_tests/webp_encoding_test.py | 6 +++--- tests/visual_tests/styles/list.xml | 4 ++-- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/include/mapnik/feature_style_processor.hpp b/include/mapnik/feature_style_processor.hpp index 6a652150c..cbabce091 100644 --- a/include/mapnik/feature_style_processor.hpp +++ b/include/mapnik/feature_style_processor.hpp @@ -98,16 +98,16 @@ private: * \brief prepare features for rendering asynchronously. */ void prepare_layer(layer_rendering_material & mat, - feature_style_context_map & ctx_map, - Processor & p, - projection const& proj0, - double scale, - double scale_denom, - unsigned width, - unsigned height, - box2d const& extent, - int buffer_size, - std::set& names); + feature_style_context_map & ctx_map, + Processor & p, + projection const& proj0, + double scale, + double scale_denom, + unsigned width, + unsigned height, + box2d const& extent, + int buffer_size, + std::set& names); /*! * \brief render features list queued when they are available. diff --git a/include/mapnik/feature_style_processor_impl.hpp b/include/mapnik/feature_style_processor_impl.hpp index 2bd7fda97..6c6965c90 100644 --- a/include/mapnik/feature_style_processor_impl.hpp +++ b/include/mapnik/feature_style_processor_impl.hpp @@ -45,7 +45,6 @@ #include #include - // boost #include #include @@ -519,9 +518,9 @@ void feature_style_processor::prepare_layer(layer_rendering_material template void feature_style_processor::render_material(layer_rendering_material & mat, Processor & p ) { - std::vector & active_styles = mat.active_styles_; - std::vector & featureset_ptr_list = mat.featureset_ptr_list_; - if (featureset_ptr_list.empty()) + std::vector & active_styles = mat.active_styles_; + std::vector & featureset_ptr_list = mat.featureset_ptr_list_; + if (featureset_ptr_list.empty()) { // The datasource wasn't querried because of early return // but we have to apply compositing operations on styles @@ -530,7 +529,6 @@ void feature_style_processor::render_material(layer_rendering_materia p.start_style_processing(*style); p.end_style_processing(*style); } - return; } diff --git a/tests/python_tests/webp_encoding_test.py b/tests/python_tests/webp_encoding_test.py index 7b1f6462e..685ce461c 100644 --- a/tests/python_tests/webp_encoding_test.py +++ b/tests/python_tests/webp_encoding_test.py @@ -18,9 +18,9 @@ if mapnik.has_webp(): os.makedirs(tmp_dir) opts = [ - 'webp', - 'webp:quality=64', - 'webp:alpha=false' + 'webp', + 'webp:quality=64', + 'webp:alpha=false' ] diff --git a/tests/visual_tests/styles/list.xml b/tests/visual_tests/styles/list.xml index 50a189bd1..d1332be80 100644 --- a/tests/visual_tests/styles/list.xml +++ b/tests/visual_tests/styles/list.xml @@ -15,8 +15,8 @@ [name] - 'S'+[nr] - [nr] + 'S'+[nr] + [nr] From ae3f682564dd298a07ccfa5e4dd3769782821b98 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 30 Sep 2013 13:32:24 +0100 Subject: [PATCH 02/17] add missin copyright notice --- plugins/input/geojson/geojson_featureset.hpp | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/plugins/input/geojson/geojson_featureset.hpp b/plugins/input/geojson/geojson_featureset.hpp index f8b37d66d..ea5e2a7bd 100644 --- a/plugins/input/geojson/geojson_featureset.hpp +++ b/plugins/input/geojson/geojson_featureset.hpp @@ -1,3 +1,25 @@ +/***************************************************************************** + * + * This file is part of Mapnik (c++ mapping toolkit) + * + * Copyright (C) 2013 Artem Pavlenko + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + *****************************************************************************/ + #ifndef GEOJSON_FEATURESET_HPP #define GEOJSON_FEATURESET_HPP From 32d053abf6cedae794aaa1b5cfebab895888a39c Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 10:13:54 -0700 Subject: [PATCH 03/17] only build pgsql2sqlite if pg_config is available --- SConstruct | 3 +++ 1 file changed, 3 insertions(+) diff --git a/SConstruct b/SConstruct index 08c403c10..7f299d38d 100644 --- a/SConstruct +++ b/SConstruct @@ -1449,6 +1449,9 @@ if not preconfigured: env.AppendUnique(LIBS='sqlite3') env.AppendUnique(CPPPATH = os.path.realpath(env['SQLITE_INCLUDES'])) env.AppendUnique(LIBPATH = os.path.realpath(env['SQLITE_LIBS'])) + if 'pq' not in env['LIBS']: + if not conf.parse_pg_config('PG_CONFIG'): + env['PGSQL2SQLITE'] = False if not SQLITE_HAS_RTREE: env['SKIPPED_DEPS'].append('pgsql2sqlite_rtree') env['PGSQL2SQLITE'] = False From 61dde4b09320108408a4122f75cce09d75629faa Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 11:47:47 -0700 Subject: [PATCH 04/17] fix several -Wsign-compare warnings --- src/cairo_context.cpp | 4 ++-- src/font_engine_freetype.cpp | 2 +- src/jpeg_reader.cpp | 2 +- src/png_reader.cpp | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cairo_context.cpp b/src/cairo_context.cpp index cf04116f3..413f76032 100644 --- a/src/cairo_context.cpp +++ b/src/cairo_context.cpp @@ -436,7 +436,7 @@ void cairo_context::add_text(text_path const& path, path.rewind(); - for (int iii = 0; iii < path.num_nodes(); iii++) + for (std::size_t iii = 0; iii < path.num_nodes(); ++iii) { char_info_ptr c; double x, y, angle; @@ -470,7 +470,7 @@ void cairo_context::add_text(text_path const& path, path.rewind(); - for (int iii = 0; iii < path.num_nodes(); iii++) + for (std::size_t iii = 0; iii < path.num_nodes(); ++iii) { char_info_ptr c; double x, y, angle; diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 9917de86c..75669d5f3 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -545,7 +545,7 @@ box2d text_renderer::prepare_glyphs(text_path const& path) bbox.xMin = bbox.yMin = 32000; // Initialize these so we can tell if we bbox.xMax = bbox.yMax = -32000; // properly grew the bbox later - for (int i = 0; i < path.num_nodes(); ++i) + for (std::size_t i = 0; i < path.num_nodes(); ++i) { char_info_ptr c; double x, y, angle; diff --git a/src/jpeg_reader.cpp b/src/jpeg_reader.cpp index 36aa63ac4..705f8e010 100644 --- a/src/jpeg_reader.cpp +++ b/src/jpeg_reader.cpp @@ -162,7 +162,7 @@ void jpeg_reader::skip(j_decompress_ptr cinfo, long count) if (count <= 0) return; //A zero or negative skip count should be treated as a no-op. jpeg_stream_wrapper* wrap = reinterpret_cast(cinfo->src); - if (wrap->manager.bytes_in_buffer > 0 && count < wrap->manager.bytes_in_buffer) + if (wrap->manager.bytes_in_buffer > 0 && count < static_cast(wrap->manager.bytes_in_buffer)) { wrap->manager.bytes_in_buffer -= count; wrap->manager.next_input_byte = &wrap->buffer[BUF_SIZE - wrap->manager.bytes_in_buffer]; diff --git a/src/png_reader.cpp b/src/png_reader.cpp index 4a732b109..f328c7475 100644 --- a/src/png_reader.cpp +++ b/src/png_reader.cpp @@ -112,7 +112,8 @@ void png_reader::png_read_data(png_structp png_ptr, png_bytep data, png_size_ { input_stream * fin = reinterpret_cast(png_get_io_ptr(png_ptr)); fin->read(reinterpret_cast(data), length); - if (fin->gcount() != length) + std::streamsize read_count = fin->gcount(); + if (read_count < 0 || static_cast(read_count) != length) { png_error(png_ptr, "Read Error"); } From 81f14b8d36b3839937510a877a3804a35d795151 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 21:21:55 -0700 Subject: [PATCH 05/17] Add currently failing test for #2000 --- .../agg_rasterize_integer_overflow.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/python_tests/agg_rasterize_integer_overflow.py diff --git a/tests/python_tests/agg_rasterize_integer_overflow.py b/tests/python_tests/agg_rasterize_integer_overflow.py new file mode 100644 index 000000000..309ae9971 --- /dev/null +++ b/tests/python_tests/agg_rasterize_integer_overflow.py @@ -0,0 +1,36 @@ +from nose.tools import * +from utilities import run_all +import mapnik +import json + +# geojson box of the world +geojson = { "type": "Feature", "properties": { }, "geometry": { "type": "Polygon", "coordinates": [ [ [ -17963313.143242701888084, -6300857.11560364998877 ], [ -17963313.143242701888084, 13071343.332991421222687 ], [ 7396658.353099936619401, 13071343.332991421222687 ], [ 7396658.353099936619401, -6300857.11560364998877 ], [ -17963313.143242701888084, -6300857.11560364998877 ] ] ] } } + +def test_that_coordinates_do_not_overflow_and_polygon_is_rendered(): + expected_color = mapnik.Color('white') + ds = mapnik.MemoryDatasource() + context = mapnik.Context() + ds.add_feature(mapnik.Feature.from_geojson(json.dumps(geojson),context)) + s = mapnik.Style() + r = mapnik.Rule() + sym = mapnik.PolygonSymbolizer() + sym.fill = expected_color + sym.clip = False + r.symbols.append(sym) + s.rules.append(r) + lyr = mapnik.Layer('Layer') + lyr.datasource = ds + lyr.styles.append('style') + m = mapnik.Map(256,256) + m.append_style('style',s) + m.layers.append(lyr) + # 17/20864/45265.png + m.zoom_to_box(mapnik.Box2d(-13658379.710221574,6197514.253362091,-13657768.213995293,6198125.749588372)) + # works 15/5216/11316.png + #m.zoom_to_box(mapnik.Box2d(-13658379.710221574,6195679.764683247,-13655933.72531645,6198125.749588372)) + im = mapnik.Image(256,256) + mapnik.render(m,im) + eq_(im.get_pixel(128,128),expected_color.packed()) + +if __name__ == "__main__": + run_all(eval(x) for x in dir() if x.startswith("test_")) From d84443b4f334435b2f1bcf5bca450ecb60fb9f5f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 21:29:22 -0700 Subject: [PATCH 06/17] fix unsigned integer overflow when passing args to composite_bitmap --- src/font_engine_freetype.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 75669d5f3..9e368d9f4 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -614,7 +614,7 @@ void text_renderer::render(pixel_position const& pos) { FT_Error error; FT_Vector start; - unsigned height = pixmap_.height(); + int height = pixmap_.height(); start.x = static_cast(pos.x * (1 << 6)); start.y = static_cast((height - pos.y) * (1 << 6)); From 8722def9842a46c9ac5555913578c518e311b75c Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 21:30:19 -0700 Subject: [PATCH 07/17] fix unsigned integer overflow warnings in agg - refs #1679 --- deps/agg/include/agg_array.h | 3 ++- deps/agg/include/agg_rasterizer_cells_aa.h | 21 ++++++++++++++------- deps/agg/include/agg_rendering_buffer.h | 3 ++- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/deps/agg/include/agg_array.h b/deps/agg/include/agg_array.h index 7c20271b2..80e02c205 100644 --- a/deps/agg/include/agg_array.h +++ b/deps/agg/include/agg_array.h @@ -516,10 +516,11 @@ namespace agg if(m_num_blocks) { T** blk = m_blocks + m_num_blocks - 1; - while(m_num_blocks--) + while(m_num_blocks > 0) { pod_allocator::deallocate(*blk, block_size); --blk; + --m_num_blocks; } } pod_allocator::deallocate(m_blocks, m_max_blocks); diff --git a/deps/agg/include/agg_rasterizer_cells_aa.h b/deps/agg/include/agg_rasterizer_cells_aa.h index 0997feeca..43be00cc9 100755 --- a/deps/agg/include/agg_rasterizer_cells_aa.h +++ b/deps/agg/include/agg_rasterizer_cells_aa.h @@ -131,10 +131,11 @@ namespace agg if(m_num_blocks) { cell_type** ptr = m_cells + m_num_blocks - 1; - while(m_num_blocks--) + while(m_num_blocks > 0) { pod_allocator::deallocate(*ptr, cell_block_size); ptr--; + --m_num_blocks; } pod_allocator::deallocate(m_cells, m_max_blocks); } @@ -663,23 +664,26 @@ namespace agg cell_type* cell_ptr; unsigned nb = m_num_cells >> cell_block_shift; unsigned i; - while(nb--) + while(nb > 0) { cell_ptr = *block_ptr++; i = cell_block_size; - while(i--) + while(i > 0) { m_sorted_y[cell_ptr->y - m_min_y].start++; ++cell_ptr; + --i; } + --nb; } cell_ptr = *block_ptr++; i = m_num_cells & cell_block_mask; - while(i--) + while(i > 0) { m_sorted_y[cell_ptr->y - m_min_y].start++; ++cell_ptr; + --i; } // Convert the Y-histogram into the array of starting indexes @@ -694,27 +698,30 @@ namespace agg // Fill the cell pointer array sorted by Y block_ptr = m_cells; nb = m_num_cells >> cell_block_shift; - while(nb--) + while(nb > 0) { cell_ptr = *block_ptr++; i = cell_block_size; - while(i--) + while(i > 0) { sorted_y& curr_y = m_sorted_y[cell_ptr->y - m_min_y]; m_sorted_cells[curr_y.start + curr_y.num] = cell_ptr; ++curr_y.num; ++cell_ptr; + --i; } + --nb; } cell_ptr = *block_ptr++; i = m_num_cells & cell_block_mask; - while(i--) + while(i > 0) { sorted_y& curr_y = m_sorted_y[cell_ptr->y - m_min_y]; m_sorted_cells[curr_y.start + curr_y.num] = cell_ptr; ++curr_y.num; ++cell_ptr; + --i; } // Finally arrange the X-arrays diff --git a/deps/agg/include/agg_rendering_buffer.h b/deps/agg/include/agg_rendering_buffer.h index e43899ecc..e9e278b90 100644 --- a/deps/agg/include/agg_rendering_buffer.h +++ b/deps/agg/include/agg_rendering_buffer.h @@ -186,10 +186,11 @@ namespace agg T** rows = &m_rows[0]; - while(height--) + while(height > 0) { *rows++ = row_ptr; row_ptr += stride; + --height; } } From ac961feb64fb88a0f415e892f0e2ec6607d90374 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 21:30:49 -0700 Subject: [PATCH 08/17] expose mapnik.Color.packed to get unsigned rgba value --- bindings/python/mapnik_color.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/bindings/python/mapnik_color.cpp b/bindings/python/mapnik_color.cpp index 82611ff48..1fcde115f 100644 --- a/bindings/python/mapnik_color.cpp +++ b/bindings/python/mapnik_color.cpp @@ -83,6 +83,7 @@ void export_color () .def(self != self) .def_pickle(color_pickle_suite()) .def("__str__",&color::to_string) + .def("packed",&color::rgba) .def("to_hex_string",&color::to_hex_string, "Returns the hexadecimal representation of this color.\n" "\n" From a25826fb13eb7f074ba86880af37dbeb30112381 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 21:35:27 -0700 Subject: [PATCH 09/17] tests: use black background in test --- tests/python_tests/agg_rasterize_integer_overflow.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/python_tests/agg_rasterize_integer_overflow.py b/tests/python_tests/agg_rasterize_integer_overflow.py index 309ae9971..477dae211 100644 --- a/tests/python_tests/agg_rasterize_integer_overflow.py +++ b/tests/python_tests/agg_rasterize_integer_overflow.py @@ -22,6 +22,7 @@ def test_that_coordinates_do_not_overflow_and_polygon_is_rendered(): lyr.datasource = ds lyr.styles.append('style') m = mapnik.Map(256,256) + m.background_color = mapnik.Color('black') m.append_style('style',s) m.layers.append(lyr) # 17/20864/45265.png From 9d8be8ea21ea7b96be7b1e78592d827683c3f7ac Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 1 Oct 2013 22:03:15 -0700 Subject: [PATCH 10/17] fix one clear case of unsigned overflow in comp_op_rgba_minus - refs #1679 --- deps/agg/include/agg_pixfmt_rgba.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/deps/agg/include/agg_pixfmt_rgba.h b/deps/agg/include/agg_pixfmt_rgba.h index e314b2531..8400c5c43 100644 --- a/deps/agg/include/agg_pixfmt_rgba.h +++ b/deps/agg/include/agg_pixfmt_rgba.h @@ -686,9 +686,9 @@ namespace agg } if(sa) { - calc_type dr = p[Order::R] - sr; - calc_type dg = p[Order::G] - sg; - calc_type db = p[Order::B] - sb; + calc_type dr = (sr > p[Order::R]) ? 0 : p[Order::R] - sr; + calc_type dg = (sg > p[Order::G]) ? 0 : p[Order::G] - sg; + calc_type db = (sb > p[Order::B]) ? 0 : p[Order::B] - sb; p[Order::R] = (dr > base_mask) ? 0 : dr; p[Order::G] = (dg > base_mask) ? 0 : dg; p[Order::B] = (db > base_mask) ? 0 : db; From 9aa596401ad31d39bd770d7143c9b7514d35b405 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 09:41:05 -0700 Subject: [PATCH 11/17] rename the agg int overflow test so it actually runs under nose --- ...ger_overflow.py => agg_rasterizer_integer_overflow_test.py} | 3 +++ 1 file changed, 3 insertions(+) rename tests/python_tests/{agg_rasterize_integer_overflow.py => agg_rasterizer_integer_overflow_test.py} (97%) diff --git a/tests/python_tests/agg_rasterize_integer_overflow.py b/tests/python_tests/agg_rasterizer_integer_overflow_test.py similarity index 97% rename from tests/python_tests/agg_rasterize_integer_overflow.py rename to tests/python_tests/agg_rasterizer_integer_overflow_test.py index 477dae211..1aed0b907 100644 --- a/tests/python_tests/agg_rasterize_integer_overflow.py +++ b/tests/python_tests/agg_rasterizer_integer_overflow_test.py @@ -1,3 +1,6 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + from nose.tools import * from utilities import run_all import mapnik From 0624ec31dcc48e4624d16299832e5f2fc1119078 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 15:14:52 -0700 Subject: [PATCH 12/17] benchmark: add conv_clip_polygon + very basic validation for clipping tests --- benchmark/run.cpp | 155 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 149 insertions(+), 6 deletions(-) diff --git a/benchmark/run.cpp b/benchmark/run.cpp index 29b4ba235..69bcc6d35 100644 --- a/benchmark/run.cpp +++ b/benchmark/run.cpp @@ -408,10 +408,88 @@ struct test8 } }; +#include "agg_conv_clip_polygon.h" #include +#include +#include + +#include "agg_conv_clip_polygon.h" + +struct test11a +{ + unsigned iter_; + unsigned threads_; + std::string wkt_in_; + mapnik::box2d extent_; + typedef agg::conv_clip_polygon conv_clip; + test11a(unsigned iterations, + unsigned threads, + std::string wkt_in, + mapnik::box2d const& extent) + : iter_(iterations), + threads_(threads), + wkt_in_(wkt_in), + extent_(extent) { + + } + + bool validate() + { + std::string expected_wkt("Polygon((181 286.666667,233 454,315 340,421 446,463 324,559 466,631 321.320755,631 234.386861,528 178,394 229,329 138,212 134,183 228,200 264,181 238.244444),(313 190,440 256,470 248,510 305,533 237,613 263,553 397,455 262,405 378,343 287,249 334,229 191,313 190,313 190))"); + boost::ptr_vector paths; + if (!mapnik::from_wkt(wkt_in_, paths)) + { + throw std::runtime_error("Failed to parse WKT"); + } + BOOST_FOREACH (geometry_type & geom , paths) + { + conv_clip clipped(geom); + clipped.clip_box( + extent_.minx(), + extent_.miny(), + extent_.maxx(), + extent_.maxy()); + unsigned cmd; + double x,y; + mapnik::geometry_type geom2(mapnik::Polygon); + while ((cmd = clipped.vertex(&x, &y)) != SEG_END) { + geom2.push_vertex(x,y,(mapnik::CommandType)cmd); + } + std::string wkt; + bool result = mapnik::util::to_wkt(wkt,geom2); + if (result) { + return (wkt == expected_wkt); + } + } + return false; + } + void operator()() + { + boost::ptr_vector paths; + if (!mapnik::from_wkt(wkt_in_, paths)) + { + throw std::runtime_error("Failed to parse WKT"); + } + for (unsigned i=0;i struct test11 { @@ -433,7 +511,39 @@ struct test11 bool validate() { - return true; + std::string expected_wkt("Polygon((212 134,329 138,394 229,528 178,631 234.4,631 321.3,559 466,463 324,421 446,315 340,233 454,181 286.7,181 238.2,200 264,183 228),(313 190,229 191,249 334,343 287,405 378,455 262,553 397,613 263,533 237,510 305,470 248,440 256))"); + boost::ptr_vector paths; + if (!mapnik::from_wkt(wkt_in_, paths)) + { + throw std::runtime_error("Failed to parse WKT"); + } + agg::path_storage ps; + ps.move_to(extent_.minx(), extent_.miny()); + ps.line_to(extent_.minx(), extent_.maxy()); + ps.line_to(extent_.maxx(), extent_.maxy()); + ps.line_to(extent_.maxx(), extent_.miny()); + ps.close_polygon(); + BOOST_FOREACH (geometry_type & geom , paths) + { + poly_clipper clipped(geom,ps, + agg::clipper_and, + agg::clipper_non_zero, + agg::clipper_non_zero, + 1); + clipped.rewind(0); + unsigned cmd; + double x,y; + mapnik::geometry_type geom2(mapnik::Polygon); + while ((cmd = clipped.vertex(&x, &y)) != SEG_END) { + geom2.push_vertex(x,y,(mapnik::CommandType)cmd); + } + std::string wkt; + bool result = mapnik::util::to_wkt(wkt,geom2); + if (result) { + return (wkt == expected_wkt); + } + } + return false; } void operator()() { @@ -473,7 +583,6 @@ struct test12 unsigned iter_; unsigned threads_; std::string wkt_in_; - mapnik::box2d extent_; typedef mapnik::polygon_clipper poly_clipper; test12(unsigned iterations, @@ -489,7 +598,28 @@ struct test12 bool validate() { - return true; + std::string expected_wkt("Polygon((181 286.666667,233 454,315 340,421 446,463 324,559 466,631 321.320755,631 234.386861,528 178,394 229,329 138,212 134,183 228,200 264,181 238.244444,181 286.666667),(313 190,440 256,470 248,510 305,533 237,613 263,553 397,455 262,405 378,343 287,249 334,229 191,313 190))"); + boost::ptr_vector paths; + if (!mapnik::from_wkt(wkt_in_, paths)) + { + throw std::runtime_error("Failed to parse WKT"); + } + BOOST_FOREACH ( geometry_type & geom , paths) + { + poly_clipper clipped(extent_, geom); + unsigned cmd; + double x,y; + mapnik::geometry_type geom2(mapnik::Polygon); + while ((cmd = clipped.vertex(&x, &y)) != SEG_END) { + geom2.push_vertex(x,y,(mapnik::CommandType)cmd); + } + std::string wkt; + bool result = mapnik::util::to_wkt(wkt,geom2); + if (result) { + return (wkt == expected_wkt); + } + } + return false; } void operator()() { @@ -669,6 +799,18 @@ int main( int argc, char** argv) // POLYGON ((181 286.6666666666667, 233 454, 315 340, 421 446, 463 324, 559 466, 631 321.3207547169811, 631 234.38686131386862, 528 178, 394 229, 329 138, 212 134, 183 228, 200 264, 181 238.24444444444444, 181 286.6666666666667),(313 190, 440 256, 470 248, 510 305, 533 237, 613 263, 553 397, 455 262, 405 378, 343 287, 249 334, 229 191, 313 190)) mapnik::box2d clipping_box(181,106,631,470); + + { + std::string filename_("benchmark/data/polygon.wkt"); + std::ifstream in(filename_.c_str(),std::ios_base::in | std::ios_base::binary); + if (!in.is_open()) + throw std::runtime_error("could not open: '" + filename_ + "'"); + std::string wkt_in( (std::istreambuf_iterator(in) ), + (std::istreambuf_iterator()) ); + test11a runner(10000,10,wkt_in,clipping_box); + benchmark(runner,"clipping polygon with conv_clip_polygon"); + } + { std::string filename_("benchmark/data/polygon.wkt"); std::ifstream in(filename_.c_str(),std::ios_base::in | std::ios_base::binary); @@ -680,6 +822,7 @@ int main( int argc, char** argv) benchmark(runner,"clipping polygon with agg_conv_clipper"); } + { std::string filename_("benchmark/data/polygon.wkt"); std::ifstream in(filename_.c_str(),std::ios_base::in | std::ios_base::binary); @@ -687,7 +830,6 @@ int main( int argc, char** argv) throw std::runtime_error("could not open: '" + filename_ + "'"); std::string wkt_in( (std::istreambuf_iterator(in) ), (std::istreambuf_iterator()) ); - test12 runner(10000,10,wkt_in,clipping_box); benchmark(runner,"clipping polygon with mapnik::polygon_clipper"); } @@ -699,8 +841,9 @@ int main( int argc, char** argv) } unsigned face_count = mapnik::freetype_engine::face_names().size(); test13 runner(1000,10); - benchmark(runner, (boost::format("font_engihe: created %ld faces in ") % (face_count * 1000 * 10)).str()); + benchmark(runner, (boost::format("font_engine: created %ld faces in ") % (face_count * 1000 * 10)).str()); } + std::cout << "...benchmark done\n"; return 0; } From f91ae9f51c7aab17844f2b8864cdd2f83ec508b4 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 16:09:32 -0700 Subject: [PATCH 13/17] benchmark: add new rendering test harness and use to compare clipping impact on speeds at high and low zoom --- benchmark/run.cpp | 77 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/benchmark/run.cpp b/benchmark/run.cpp index 69bcc6d35..0ebabe0ee 100644 --- a/benchmark/run.cpp +++ b/benchmark/run.cpp @@ -674,6 +674,53 @@ struct test13 } }; +#include +#include +#include +#include + +struct test14 +{ + unsigned iter_; + unsigned threads_; + std::string xml_; + mapnik::box2d extent_; + test14(unsigned iterations, + unsigned threads, + std::string const& xml, + mapnik::box2d const& extent) + : iter_(iterations), + threads_(threads), + xml_(xml), + extent_(extent) + {} + + bool validate() + { + mapnik::Map m(256,256); + mapnik::load_map(m,xml_); + m.zoom_to_box(extent_); + mapnik::image_32 im(m.width(),m.height()); + mapnik::agg_renderer ren(m,im); + ren.apply(); + //mapnik::save_to_file(im,"test.png"); + return true; + } + + void operator()() + { + mapnik::Map m(256,256); + mapnik::load_map(m,xml_); + m.zoom_to_box(extent_); + for (unsigned i=0;i ren(m,im); + ren.apply(); + } + } +}; + int main( int argc, char** argv) { if (argc > 0) { @@ -689,6 +736,7 @@ int main( int argc, char** argv) } } } + mapnik::datasource_cache::instance().register_datasources("./plugins/input/"); try { std::cout << "starting benchmark…\n"; @@ -844,6 +892,35 @@ int main( int argc, char** argv) benchmark(runner, (boost::format("font_engine: created %ld faces in ") % (face_count * 1000 * 10)).str()); } + { + test14 runner(500,10, + "benchmark/data/polygon_rendering_clip.xml", + mapnik::box2d(-20037508.3428,-8317435.0606,20037508.3428,18399242.7298)); + benchmark(runner, "rendering polygon with clipping at full extent"); + } + + { + test14 runner(500,10, + "benchmark/data/polygon_rendering_no_clip.xml", + mapnik::box2d(-20037508.3428,-8317435.0606,20037508.3428,18399242.7298)); + benchmark(runner, "rendering polygon without clipping at full extent"); + } + + { + // note: bbox below is for 16/10491/22911.png + test14 runner(500,10, + "benchmark/data/polygon_rendering_clip.xml", + mapnik::box2d(-13622912.929097254,6026906.8062295765,-13621689.93664469,6028129.79868214)); + benchmark(runner, "rendering polygon with clipping at z16 extent"); + } + + { + test14 runner(500,10, + "benchmark/data/polygon_rendering_no_clip.xml", + mapnik::box2d(-13622912.929097254,6026906.8062295765,-13621689.93664469,6028129.79868214)); + benchmark(runner, "rendering polygon without clipping at z16 extent"); + } + std::cout << "...benchmark done\n"; return 0; } From f847a67ee9bd7a5b3b18fa228074613bc9f368a3 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 16:55:11 -0700 Subject: [PATCH 14/17] fix span_image_resample_rgba --- deps/agg/include/agg_span_image_filter_rgba.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/deps/agg/include/agg_span_image_filter_rgba.h b/deps/agg/include/agg_span_image_filter_rgba.h index 134a80255..4c19b2371 100644 --- a/deps/agg/include/agg_span_image_filter_rgba.h +++ b/deps/agg/include/agg_span_image_filter_rgba.h @@ -895,9 +895,9 @@ namespace agg if(fg[3] < 0) fg[3] = 0; if(fg[order_type::A] > base_mask) fg[order_type::A] = base_mask; - if(fg[order_type::R] > fg[order_type::R]) fg[order_type::R] = fg[order_type::R]; - if(fg[order_type::G] > fg[order_type::G]) fg[order_type::G] = fg[order_type::G]; - if(fg[order_type::B] > fg[order_type::B]) fg[order_type::B] = fg[order_type::B]; + if(fg[order_type::R] > fg[order_type::A]) fg[order_type::R] = fg[order_type::A]; + if(fg[order_type::G] > fg[order_type::A]) fg[order_type::G] = fg[order_type::A]; + if(fg[order_type::B] > fg[order_type::A]) fg[order_type::B] = fg[order_type::A]; span->r = (value_type)fg[order_type::R]; span->g = (value_type)fg[order_type::G]; From 371d79774abba1badb9cc68e10e67f9628ca7ce9 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 17:08:22 -0700 Subject: [PATCH 15/17] avoid integer overflows in agg by using clamping in agg:iround with agg::rasterizer_sl_clip_int_sat - closes #2000 --- include/mapnik/agg_rasterizer.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mapnik/agg_rasterizer.hpp b/include/mapnik/agg_rasterizer.hpp index 1f93c556f..5e82f03f1 100644 --- a/include/mapnik/agg_rasterizer.hpp +++ b/include/mapnik/agg_rasterizer.hpp @@ -31,7 +31,7 @@ namespace mapnik { -struct rasterizer : agg::rasterizer_scanline_aa<>, mapnik::noncopyable {}; +struct rasterizer : agg::rasterizer_scanline_aa, mapnik::noncopyable {}; } From fdf9288ba7f6618a0f5682ab2d863d0d23bd9811 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 17:16:58 -0700 Subject: [PATCH 16/17] add xml needed for f91ae9f51c7aab --- benchmark/data/polygon_rendering_clip.xml | 14 ++++++++++++++ benchmark/data/polygon_rendering_no_clip.xml | 14 ++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 benchmark/data/polygon_rendering_clip.xml create mode 100644 benchmark/data/polygon_rendering_no_clip.xml diff --git a/benchmark/data/polygon_rendering_clip.xml b/benchmark/data/polygon_rendering_clip.xml new file mode 100644 index 000000000..98f0ec6e9 --- /dev/null +++ b/benchmark/data/polygon_rendering_clip.xml @@ -0,0 +1,14 @@ + + + + style + + ../../tests/data/shp/world_merc.shp + shape + + + \ No newline at end of file diff --git a/benchmark/data/polygon_rendering_no_clip.xml b/benchmark/data/polygon_rendering_no_clip.xml new file mode 100644 index 000000000..8d478e079 --- /dev/null +++ b/benchmark/data/polygon_rendering_no_clip.xml @@ -0,0 +1,14 @@ + + + + style + + ../../tests/data/shp/world_merc.shp + shape + + + \ No newline at end of file From 83fde93411dbe3ee36f27d9415bc8d600907face Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 2 Oct 2013 17:20:20 -0700 Subject: [PATCH 17/17] add note about #2000 to changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f05070ea9..f41b5fdb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ Released ... Summary: TODO +- Fixed rendering of large shapes at high zoom levels, which might dissapear due to integer overflow. This + bug was previously fixable when geometries were clipped, but would, until now, re-appear if clipping was turned + off for a symbolizer (#2000) + - Added single color argument support to `colorize-alpha` to allow colorizing alpha with one color. - Added `color-to-alpha` `image-filter` to allow for applying alpha in proportion to color similiarity (#2023)