From c24229f5c81847e72a57bced3192c79e05376ba5 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Tue, 23 Jun 2015 21:54:24 -0700 Subject: [PATCH 1/4] visual tests: still generate error report on ctrl-c --- test/visual/config.hpp | 23 +++++++++++++++++++++++ test/visual/run.cpp | 30 +++++++++++++++++++++--------- test/visual/runner.cpp | 4 ++++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/test/visual/config.hpp b/test/visual/config.hpp index 8353d4b2e..90727dd76 100644 --- a/test/visual/config.hpp +++ b/test/visual/config.hpp @@ -26,6 +26,7 @@ // stl #include #include +#include // boost #include @@ -33,6 +34,28 @@ namespace visual_tests { +class early_exit_error : public std::exception +{ +public: + early_exit_error() : + what_() {} + + early_exit_error( std::string const& what ) : + what_( what ) + { + } + + virtual ~early_exit_error() throw() {} + + virtual const char * what() const throw() + { + return what_.c_str(); + } + +protected: + mutable std::string what_; +}; + struct map_size { map_size(int _width, int _height) : width(_width), height(_height) { } diff --git a/test/visual/run.cpp b/test/visual/run.cpp index 1353bcc42..aedc65010 100644 --- a/test/visual/run.cpp +++ b/test/visual/run.cpp @@ -22,6 +22,8 @@ #include "runner.hpp" #include "config.hpp" +#include +#include #include #include @@ -31,6 +33,12 @@ #include "cleanup.hpp" // run_cleanup() +void signal_handler(int /*signal*/) +{ + throw visual_tests::early_exit_error("SIGINT"); +} + + int main(int argc, char** argv) { using namespace visual_tests; @@ -81,6 +89,10 @@ int main(int argc, char** argv) report_type report = vm.count("verbose") ? report_type((console_report())) : report_type((console_short_report())); result_list results; + std::signal(SIGINT,signal_handler); + + unsigned failed_count = 0; + try { if (vm.count("styles")) @@ -91,20 +103,20 @@ int main(int argc, char** argv) { results = run.test_all(report); } + + failed_count = mapnik::util::apply_visitor(summary_visitor(results), report); + + if (failed_count) + { + html_summary(results, output_dir); + } } - catch (std::exception & e) + catch (std::exception const& e) { - std::cerr << "Error runnig tests: " << e.what() << std::endl; + std::cerr << "Error running tests: " << e.what() << std::endl; return 1; } - unsigned failed_count = mapnik::util::apply_visitor(summary_visitor(results), report); - - if (failed_count) - { - html_summary(results, output_dir); - } - testing::run_cleanup(); return failed_count; diff --git a/test/visual/runner.cpp b/test/visual/runner.cpp index f2562c197..2274774ec 100644 --- a/test/visual/runner.cpp +++ b/test/visual/runner.cpp @@ -156,6 +156,10 @@ result_list runner::test_range(files_iterator begin, files_iterator end, std::re result_list r = test_one(file, defaults, report); std::move(r.begin(), r.end(), std::back_inserter(results)); } + catch (visual_tests::early_exit_error const&) + { + break; + } catch (std::exception const& ex) { result r; From 5a032ee98be4daa3587110acd7a1aa61aaaffa6b Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 24 Jun 2015 17:02:34 -0700 Subject: [PATCH 2/4] fix #2924 and #2412 - adds back support for minimum-path-length on lines (only supported lines in 2.3.x) - made text-largest-bbox-only work only on polygons (restores 2.3.x behavior) TODO: give more control: #1583 --- src/text/symbolizer_helpers.cpp | 67 ++++++++++++++++++++++----------- test/data-visual | 2 +- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/text/symbolizer_helpers.cpp b/src/text/symbolizer_helpers.cpp index 635606c58..99c7c0356 100644 --- a/src/text/symbolizer_helpers.cpp +++ b/src/text/symbolizer_helpers.cpp @@ -93,11 +93,27 @@ struct split_multi_geometries cont_.push_back(base_symbolizer_helper::geometry_cref(std::cref(pt))); } } + void operator() (geometry::line_string const& line) const + { + if (minimum_path_length_ > 0) + { + box2d bbox = t_.forward(geometry::envelope(line), prj_trans_); + if (bbox.width() >= minimum_path_length_) + { + cont_.push_back(base_symbolizer_helper::geometry_cref(std::cref(line))); + } + } + else + { + cont_.push_back(base_symbolizer_helper::geometry_cref(std::cref(line))); + } + } + void operator() (geometry::multi_line_string const& multi_line) const { for ( auto const& line : multi_line ) { - cont_.push_back(base_symbolizer_helper::geometry_cref(std::cref(line))); + (*this)(line); } } @@ -182,6 +198,7 @@ struct largest_bbox_first bool operator() (base_symbolizer_helper::geometry_cref const& g0, base_symbolizer_helper::geometry_cref const& g1) const { + // TODO - this has got to be expensive! Can we cache bbox's if there are repeated calls to same geom? box2d b0 = geometry::envelope(g0); box2d b1 = geometry::envelope(g1); return b0.width() * b0.height() > b1.width() * b1.height(); @@ -190,18 +207,26 @@ struct largest_bbox_first void base_symbolizer_helper::initialize_geometries() const { - bool largest_box_only = text_props_->largest_bbox_only; double minimum_path_length = text_props_->minimum_path_length; + auto const& geom = feature_.get_geometry(); util::apply_visitor(detail::split_multi_geometries - (geometries_to_process_, t_, prj_trans_, minimum_path_length ), feature_.get_geometry()); - // FIXME: return early if geometries_to_process_.empty() ? - if (largest_box_only) + (geometries_to_process_, t_, prj_trans_, minimum_path_length ), geom); + if (!geometries_to_process_.empty()) { - geometries_to_process_.sort(largest_bbox_first()); + auto type = geometry::geometry_type(geom); + if (type == geometry::geometry_types::Polygon || + type == geometry::geometry_types::MultiPolygon) + { + bool largest_box_only = text_props_->largest_bbox_only; + if (largest_box_only) + { + geometries_to_process_.sort(largest_bbox_first()); + geo_itr_ = geometries_to_process_.begin(); + geometries_to_process_.erase(++geo_itr_, geometries_to_process_.end()); + } + } geo_itr_ = geometries_to_process_.begin(); - geometries_to_process_.erase(++geo_itr_, geometries_to_process_.end()); } - geo_itr_ = geometries_to_process_.begin(); } void base_symbolizer_helper::initialize_points() const @@ -236,30 +261,30 @@ void base_symbolizer_helper::initialize_points() const // https://github.com/mapnik/mapnik/issues/1350 auto type = geometry::geometry_type(geom); - // FIXME: how to handle MultiLineString? + // note: split_multi_geometries is called above so the only likely types are: + // Point, LineString, and Polygon. if (type == geometry::geometry_types::LineString) { auto const& line = mapnik::util::get >(geom); geometry::line_string_vertex_adapter va(line); success = label::middle_point(va, label_x,label_y); } - else if (how_placed == POINT_PLACEMENT) + else if (how_placed == POINT_PLACEMENT || type == geometry::geometry_types::Point) { geometry::point pt; - geometry::centroid(geom, pt); - label_x = pt.x; - label_y = pt.y; - success = true; - } - else if (how_placed == INTERIOR_PLACEMENT) // polygon - { - if (type == geometry::geometry_types::Polygon) + if (geometry::centroid(geom, pt)) { - auto const& poly = mapnik::util::get >(geom); - geometry::polygon_vertex_adapter va(poly); - success = label::interior_position(va, label_x, label_y); + label_x = pt.x; + label_y = pt.y; + success = true; } } + else if (how_placed == INTERIOR_PLACEMENT && type == geometry::geometry_types::Polygon) + { + auto const& poly = mapnik::util::get >(geom); + geometry::polygon_vertex_adapter va(poly); + success = label::interior_position(va, label_x, label_y); + } else { MAPNIK_LOG_ERROR(symbolizer_helpers) << "ERROR: Unknown placement type in initialize_points()"; diff --git a/test/data-visual b/test/data-visual index 9702e69c4..37484c46f 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit 9702e69c49b60ff49da4c48d1e33e54289b9c4b1 +Subproject commit 37484c46fa0c765f85f08dee4f473614f25238c8 From 542830045490fa16d0a420bf25f7abc2dceb1d2d Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 24 Jun 2015 17:35:12 -0700 Subject: [PATCH 3/4] update more visual tests after 5a032ee98be4d --- test/data-visual | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/data-visual b/test/data-visual index 37484c46f..7080866ca 160000 --- a/test/data-visual +++ b/test/data-visual @@ -1 +1 @@ -Subproject commit 37484c46fa0c765f85f08dee4f473614f25238c8 +Subproject commit 7080866ca1b3523fead1a25c017bd87082806eb4 From b285bde44bb009d5c619d393bb4902cf96bd98d1 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 24 Jun 2015 17:37:02 -0700 Subject: [PATCH 4/4] Revert "visual tests: still generate error report on ctrl-c" This reverts commit c24229f5c81847e72a57bced3192c79e05376ba5. --- test/visual/config.hpp | 23 ----------------------- test/visual/run.cpp | 30 +++++++++--------------------- test/visual/runner.cpp | 4 ---- 3 files changed, 9 insertions(+), 48 deletions(-) diff --git a/test/visual/config.hpp b/test/visual/config.hpp index 90727dd76..8353d4b2e 100644 --- a/test/visual/config.hpp +++ b/test/visual/config.hpp @@ -26,7 +26,6 @@ // stl #include #include -#include // boost #include @@ -34,28 +33,6 @@ namespace visual_tests { -class early_exit_error : public std::exception -{ -public: - early_exit_error() : - what_() {} - - early_exit_error( std::string const& what ) : - what_( what ) - { - } - - virtual ~early_exit_error() throw() {} - - virtual const char * what() const throw() - { - return what_.c_str(); - } - -protected: - mutable std::string what_; -}; - struct map_size { map_size(int _width, int _height) : width(_width), height(_height) { } diff --git a/test/visual/run.cpp b/test/visual/run.cpp index aedc65010..1353bcc42 100644 --- a/test/visual/run.cpp +++ b/test/visual/run.cpp @@ -22,8 +22,6 @@ #include "runner.hpp" #include "config.hpp" -#include -#include #include #include @@ -33,12 +31,6 @@ #include "cleanup.hpp" // run_cleanup() -void signal_handler(int /*signal*/) -{ - throw visual_tests::early_exit_error("SIGINT"); -} - - int main(int argc, char** argv) { using namespace visual_tests; @@ -89,10 +81,6 @@ int main(int argc, char** argv) report_type report = vm.count("verbose") ? report_type((console_report())) : report_type((console_short_report())); result_list results; - std::signal(SIGINT,signal_handler); - - unsigned failed_count = 0; - try { if (vm.count("styles")) @@ -103,20 +91,20 @@ int main(int argc, char** argv) { results = run.test_all(report); } - - failed_count = mapnik::util::apply_visitor(summary_visitor(results), report); - - if (failed_count) - { - html_summary(results, output_dir); - } } - catch (std::exception const& e) + catch (std::exception & e) { - std::cerr << "Error running tests: " << e.what() << std::endl; + std::cerr << "Error runnig tests: " << e.what() << std::endl; return 1; } + unsigned failed_count = mapnik::util::apply_visitor(summary_visitor(results), report); + + if (failed_count) + { + html_summary(results, output_dir); + } + testing::run_cleanup(); return failed_count; diff --git a/test/visual/runner.cpp b/test/visual/runner.cpp index 2274774ec..f2562c197 100644 --- a/test/visual/runner.cpp +++ b/test/visual/runner.cpp @@ -156,10 +156,6 @@ result_list runner::test_range(files_iterator begin, files_iterator end, std::re result_list r = test_one(file, defaults, report); std::move(r.begin(), r.end(), std::back_inserter(results)); } - catch (visual_tests::early_exit_error const&) - { - break; - } catch (std::exception const& ex) { result r;