From ca99ef3af79b02dbb1569fcb10767e7c8a549d59 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Sun, 14 Jun 2026 14:49:14 +0200 Subject: [PATCH] More changes to pass around logger to parse-related calls --- .../ifcopenshell/__init__.py | 31 +++-- src/ifcopenshell-python/ifcopenshell/draw.py | 7 +- .../ifcopenshell/geom/main.py | 21 ++- src/ifcparse/IfcLogger.cpp | 28 +++- src/ifcparse/IfcLogger.h | 26 ++-- src/ifcwrap/IfcGeomWrapper.i | 60 ++++----- src/ifcwrap/IfcParseWrapper.i | 9 +- src/svgfill/CMakeLists.txt | 2 +- src/svgfill/src/arrange_polygons.cpp | 120 ++++++++++++------ src/svgfill/src/svgfill.h | 4 +- 10 files changed, 203 insertions(+), 105 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/__init__.py b/src/ifcopenshell-python/ifcopenshell/__init__.py index 2c93e8175e..c50ff343e9 100644 --- a/src/ifcopenshell-python/ifcopenshell/__init__.py +++ b/src/ifcopenshell-python/ifcopenshell/__init__.py @@ -132,10 +132,20 @@ class SchemaError(Error): @overload def open( - path: Union[os.PathLike, str], format: SupportedFormat = None, *, should_stream: Literal[False] = False + path: Union[os.PathLike, str], + format: SupportedFormat = None, + *, + should_stream: Literal[False] = False, + logger: Optional[logger] = None, ) -> Union[_file, sqlite]: ... @overload -def open(path: Union[os.PathLike, str], format: SupportedFormat = None, *, should_stream: Literal[True]) -> _stream: ... +def open( + path: Union[os.PathLike, str], + format: SupportedFormat = None, + *, + should_stream: Literal[True], + logger: Optional[logger] = None, +) -> _stream: ... @overload def open( path: Union[os.PathLike, str], @@ -143,6 +153,7 @@ def open( *, should_stream: bool = False, readonly: bool = False, + logger: Optional[logger] = None, ) -> Union[_file, sqlite, _stream]: ... def open( path: Union[os.PathLike, str], @@ -151,11 +162,13 @@ def open( readonly: bool = False, mmap: bool = False, bypass_types: Optional[Sequence[str]] = None, + logger: Optional[logger] = None, ) -> Union[_file, sqlite, _stream]: """Loads an IFC dataset from a filepath :param should_stream: Whether to open the file in streaming mode. Could be useful for reading large files. + :param logger: Logger that receives native parser messages. You can specify a file format. If no format is given, it is guessed from its extension. @@ -179,8 +192,10 @@ def open( raise FileNotFoundError(f"Path does not exist: '{path}'.") if format is None: format = guess_format(path) + if logger is None: + logger = ifcopenshell_wrapper.logger.Root() if format == ".ifcXML": - f = ifcopenshell_wrapper.parse_ifcxml(str(path.absolute())) + f = ifcopenshell_wrapper.parse_ifcxml(str(path.absolute()), logger) if f: return file(f) raise OSError(f"Failed to parse .ifcXML file from {path}") @@ -189,7 +204,7 @@ def open( with zipfile.ZipFile(path) as zf: for name in zf.namelist(): if Path(name).suffix.lower() in (".ifc", ".ifcxml"): - return open(zf.extract(name, unzipped_path)) + return open(zf.extract(name, unzipped_path), logger=logger) else: raise LookupError(f"No .ifc or .ifcXML file found in {path}") if format == ".ifcSQLite": @@ -197,9 +212,9 @@ def open( if should_stream: return stream(path) if readonly: # Temporary conditional see #7131. Remove once newer builds don't segfault on Linux. - f = ifcopenshell_wrapper.open(str(path.absolute()), readonly=readonly) + f = ifcopenshell_wrapper.open(str(path.absolute()), readonly, logger) elif bypass_types: - f = ifcopenshell_wrapper.file(ifcopenshell_wrapper.uninitialized_tag()) + f = ifcopenshell_wrapper.file(ifcopenshell_wrapper.uninitialized_tag(), logger) for ty in bypass_types: f.bypass_type(ty) if mmap: @@ -209,9 +224,9 @@ def open( f.initialize(str(path.absolute())) elif mmap: # mmap parameter is only available for builds with USE_MMAP, not used in our main builds - f = ifcopenshell_wrapper.open(str(path.absolute()), mmap=mmap) # ty: ignore[unknown-argument] + f = ifcopenshell_wrapper.open(str(path.absolute()), mmap=mmap, logger=logger) # ty: ignore[unknown-argument] else: - f = ifcopenshell_wrapper.open(str(path.absolute())) + f = ifcopenshell_wrapper.open(str(path.absolute()), False, logger) return file(f) diff --git a/src/ifcopenshell-python/ifcopenshell/draw.py b/src/ifcopenshell-python/ifcopenshell/draw.py index f4ea932933..b7bc4db365 100644 --- a/src/ifcopenshell-python/ifcopenshell/draw.py +++ b/src/ifcopenshell-python/ifcopenshell/draw.py @@ -105,7 +105,10 @@ def main( iterators: Sequence[ifcopenshell.geom.iterator] = (), merge_projection: bool = True, progress_function: Callable = DO_NOTHING, + logger=None, ): + if logger is None: + logger = ifcopenshell.logger.Root() def by_guid(g): for f in files: @@ -147,7 +150,7 @@ def main( iterator_kwargs["include"] = list( filter(has_selected_parent, sum((f.by_type(x) for x in iterator_kwargs["include"]), [])) ) - return ifcopenshell.geom.iterator(geom_settings, f, **iterator_kwargs) + return ifcopenshell.geom.iterator(geom_settings, f, logger=logger, **iterator_kwargs) # We have to keep the iterator in memory because otherwise # the styles are cleared up. @@ -458,7 +461,6 @@ def main( g1.appendChild(g2) if settings.arrange_spaces or settings.arrange_zones: - if settings.storey_filter: # delete storey groups not selected by filter # sometimes happens in case of elements protruding multiple stories @@ -541,6 +543,7 @@ def main( arranged = W.arrange_polygons( *filter(None, (ARRANGE_POLYGON_SETTINGS,)), polies, # ty: ignore[too-many-positional-arguments] + logger, ) svg_data_3 = W.polygons_to_svg(arranged, False) dom3 = parseString(svg_data_3) diff --git a/src/ifcopenshell-python/ifcopenshell/geom/main.py b/src/ifcopenshell-python/ifcopenshell/geom/main.py index 574d19d904..8161512836 100644 --- a/src/ifcopenshell-python/ifcopenshell/geom/main.py +++ b/src/ifcopenshell-python/ifcopenshell/geom/main.py @@ -299,13 +299,16 @@ class iterator(ifcopenshell_wrapper.Iterator): include: Optional[Union[list[entity_instance], list[str]]] = None, exclude: Optional[Union[list[entity_instance], list[str]]] = None, geometry_library: GEOMETRY_LIBRARY = "opencascade", + logger=None, ): self.settings = settings + if logger is None: + logger = ifcopenshell_wrapper.logger.Root() if isinstance(file_or_filename, file): self.file = file file_or_filename = file_or_filename.wrapped_data else: - file_or_filename = self.file = open(file_or_filename) + file_or_filename = self.file = open(file_or_filename, logger=logger) if include is not None and exclude is not None: raise ValueError("include and exclude cannot be specified simultaneously") @@ -334,11 +337,17 @@ class iterator(ifcopenshell_wrapper.Iterator): initializer = ifcopenshell_wrapper.construct_iterator_with_include_exclude self.this = initializer( - geometry_library, self.settings, file_or_filename, include_or_exclude, include is not None, num_threads + geometry_library, + self.settings, + file_or_filename, + include_or_exclude, + include is not None, + num_threads, + logger, ) else: self.this = ifcopenshell_wrapper.construct_iterator( - geometry_library, self.settings, file_or_filename, num_threads + geometry_library, self.settings, file_or_filename, num_threads, logger ) if has_occ: @@ -564,6 +573,7 @@ def iterate( cache: Optional[str] = None, serializer_settings: Optional[serializer_settings] = None, geometry_library: GEOMETRY_LIBRARY = "opencascade", + logger=None, ) -> Generator[IteratorOutput, None, None]: ... @overload def iterate( @@ -577,6 +587,7 @@ def iterate( cache: Optional[str] = None, serializer_settings: Optional[serializer_settings] = None, geometry_library: GEOMETRY_LIBRARY = "opencascade", + logger=None, ) -> Generator[tuple[int, IteratorOutput], None, None]: ... @overload def iterate( @@ -590,6 +601,7 @@ def iterate( cache: Optional[str] = None, serializer_settings: Optional[serializer_settings] = None, geometry_library: GEOMETRY_LIBRARY = "opencascade", + logger=None, ) -> Generator[Union[IteratorOutput, tuple[int, IteratorOutput]], None, None]: ... def iterate( settings: settings, @@ -602,13 +614,14 @@ def iterate( cache: Optional[str] = None, serializer_settings: Optional[serializer_settings] = None, geometry_library: GEOMETRY_LIBRARY = "opencascade", + logger=None, ) -> Generator[Union[IteratorOutput, tuple[int, IteratorOutput]], None, None]: """Get a geometry iterator for the provided file. :param cache: .h5 cache filepath (might not exist, will be created). :param serializer_settings: Settings for cache serializer. Required if `cache` is provided. """ - it = iterator(settings, file_or_filename, num_threads, include, exclude, geometry_library) + it = iterator(settings, file_or_filename, num_threads, include, exclude, geometry_library, logger) if cache: assert serializer_settings, "`serializer_settings` argument is not optional if `cache` is provided." hdf5_cache = serializers.hdf5(cache, settings, serializer_settings) diff --git a/src/ifcparse/IfcLogger.cpp b/src/ifcparse/IfcLogger.cpp index 0b454b8563..200d61e636 100644 --- a/src/ifcparse/IfcLogger.cpp +++ b/src/ifcparse/IfcLogger.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -131,6 +132,31 @@ void json_message(T& out, const IfcUtil::IfcBaseClass* current_product, Logger:: } } // namespace +log_message::log_message( + int severity, + const char (&code_prefix)[4], + uint16_t code_number, + const std::string& timestamp, + const std::string& message, + const IfcUtil::IfcBaseInterface* inst, + const IfcUtil::IfcBaseClass* current_product) + : severity(severity) + , timestamp(timestamp) + , message(message) +{ + snprintf(code, 7, "%s%03u", code_prefix, code_number); + if (inst) { + std::ostringstream oss; + inst->as()->toString(oss); + instance = oss.str(); + } + if (current_product) { + std::ostringstream oss; + current_product->toString(oss); + product = oss.str(); + } +} + Logger& Logger::Root() { static Logger logger; return logger; @@ -199,7 +225,7 @@ void Logger::Message(Logger::Severity type, const char (&code_prefix)[4], uint16 } if (format_ == FMT_INMEMORY) { - log_messages_.emplace_back(type, code_prefix, code_number, message, instance, current_product()); + log_messages_.emplace_back(type, code_prefix, code_number, get_time(), message, instance, current_product()); } else if (((log2_ != nullptr) || (wlog2_ != nullptr))) { if (format_ == FMT_PLAIN) { if (log2_ != nullptr) { diff --git a/src/ifcparse/IfcLogger.h b/src/ifcparse/IfcLogger.h index 19fcdd9738..280cfcd036 100644 --- a/src/ifcparse/IfcLogger.h +++ b/src/ifcparse/IfcLogger.h @@ -37,24 +37,16 @@ class IFC_PARSE_API log_message { public: char code[7]; int severity; - std::string message, instance, product; + std::string timestamp, message, instance, product; - log_message(int severity, const char (&code_prefix)[4], uint16_t code_number, const std::string& message, const IfcUtil::IfcBaseInterface* inst = 0, const IfcUtil::IfcBaseClass* current_product = 0) - : severity(severity) - , message(message) - { - snprintf(code, 7, "%s%03u", code_prefix, code_number); - if (inst) { - std::ostringstream oss; - inst->data().toString(nullptr, nullptr, 0, oss, true); - instance = oss.str(); - } - if (current_product) { - std::ostringstream oss; - current_product->toString(oss); - product = oss.str(); - } - } + log_message( + int severity, + const char (&code_prefix)[4], + uint16_t code_number, + const std::string& timestamp, + const std::string& message, + const IfcUtil::IfcBaseInterface* inst = 0, + const IfcUtil::IfcBaseClass* current_product = 0); }; class IFC_PARSE_API Logger { diff --git a/src/ifcwrap/IfcGeomWrapper.i b/src/ifcwrap/IfcGeomWrapper.i index a9cf372dc1..10d9083403 100644 --- a/src/ifcwrap/IfcGeomWrapper.i +++ b/src/ifcwrap/IfcGeomWrapper.i @@ -637,31 +637,31 @@ struct ShapeRTTI : public boost::static_visitor // I couldn't get the vector typemap to be applied when %extending Iterator constructor. // anyway it does not matter as SWIG generates C code without actual constructors %inline %{ - IfcGeom::Iterator* construct_iterator(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, int num_threads) { - return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings), settings, file, num_threads); - } - - IfcGeom::Iterator* construct_iterator_with_include_exclude(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads) { - std::set elems_set(elems.begin(), elems.end()); - IfcGeom::entity_filter ef{ include, false, elems_set }; - return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings), settings, file, {ef}, num_threads); - } - - IfcGeom::Iterator* construct_iterator_with_include_exclude_globalid(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads) { - std::set elems_set(elems.begin(), elems.end()); - IfcGeom::attribute_filter af; - af.attribute_name = "GlobalId"; - af.populate(elems_set); - af.include = include; - return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings), settings, file, {af}, num_threads); - } - - IfcGeom::Iterator* construct_iterator_with_include_exclude_id(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads) { - std::set elems_set(elems.begin(), elems.end()); - IfcGeom::instance_id_filter af(include, false, elems_set); - return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings), settings, file, {af}, num_threads); - } -%} + IfcGeom::Iterator* construct_iterator(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, int num_threads, Logger& logger = Logger::Root()) { + return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings, logger), settings, file, num_threads, logger); + } + + IfcGeom::Iterator* construct_iterator_with_include_exclude(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads, Logger& logger = Logger::Root()) { + std::set elems_set(elems.begin(), elems.end()); + IfcGeom::entity_filter ef{ include, false, elems_set }; + return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings, logger), settings, file, {ef}, num_threads, logger); + } + + IfcGeom::Iterator* construct_iterator_with_include_exclude_globalid(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads, Logger& logger = Logger::Root()) { + std::set elems_set(elems.begin(), elems.end()); + IfcGeom::attribute_filter af; + af.attribute_name = "GlobalId"; + af.populate(elems_set); + af.include = include; + return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings, logger), settings, file, {af}, num_threads, logger); + } + + IfcGeom::Iterator* construct_iterator_with_include_exclude_id(const std::string& geometry_library, ifcopenshell::geometry::Settings settings, IfcParse::IfcFile* file, std::vector elems, bool include, int num_threads, Logger& logger = Logger::Root()) { + std::set elems_set(elems.begin(), elems.end()); + IfcGeom::instance_id_filter af(include, false, elems_set); + return new IfcGeom::Iterator(ifcopenshell::geometry::kernels::construct(file, geometry_library, settings, logger), settings, file, {af}, num_threads, logger); + } +%} %extend IfcGeom::Representation::Triangulation { @@ -1288,11 +1288,11 @@ ifcopenshell::geometry::taxonomy::item::ptr try_upcast(PyObject* obj0, swig_type } } - std::vector arrange_polygons(svgfill::arrange_polygon_settings settings, const std::vector& polygons) { - std::vector r; - if (svgfill::arrange_polygons(settings, polygons, r)) { - return r; - } else { + std::vector arrange_polygons(svgfill::arrange_polygon_settings settings, const std::vector& polygons, Logger& logger = Logger::Root()) { + std::vector r; + if (svgfill::arrange_polygons(settings, polygons, r, logger)) { + return r; + } else { throw std::runtime_error("Failed to arrange polygons"); } } diff --git a/src/ifcwrap/IfcParseWrapper.i b/src/ifcwrap/IfcParseWrapper.i index ab750543ef..9826c03da2 100644 --- a/src/ifcwrap/IfcParseWrapper.i +++ b/src/ifcwrap/IfcParseWrapper.i @@ -712,10 +712,10 @@ private: %newobject stream_from_string; %inline %{ - IfcParse::IfcFile* open(const std::string& fn, bool readonly=false) { + IfcParse::IfcFile* open(const std::string& fn, bool readonly=false, Logger& logger=Logger::Root()) { IfcParse::IfcFile* f; Py_BEGIN_ALLOW_THREADS; - f = new IfcParse::IfcFile(fn, IfcParse::FT_AUTODETECT, readonly); + f = new IfcParse::IfcFile(fn, IfcParse::FT_AUTODETECT, readonly, logger); Py_END_ALLOW_THREADS; return f; } @@ -1238,8 +1238,11 @@ private: } %pythoncode %{ severity_string = property(severity_string) + def to_dict(self): + keys = ("timestamp", "severity", "code", "message", "instance", "product") + return dict(zip(keys, self.to_tuple())) def to_tuple(self): - return self.severity_string, self.code, self.message, self.instance + return self.timestamp, self.severity_string, self.code, self.message, self.instance, self.product def __eq__(self, other): return type(self) == type(other) and self.to_tuple() == other.to_tuple() def __hash__(self): diff --git a/src/svgfill/CMakeLists.txt b/src/svgfill/CMakeLists.txt index 0d9764013a..ea4e64636f 100644 --- a/src/svgfill/CMakeLists.txt +++ b/src/svgfill/CMakeLists.txt @@ -49,7 +49,7 @@ file(GLOB LIB_H_FILES src/*.h) file(GLOB LIB_CPP_FILES src/svgfill.cpp src/arrange_polygons.cpp) set(LIB_SRC_FILES ${LIB_H_FILES} ${LIB_CPP_FILES}) add_library(svgfill ${LIB_SRC_FILES}) -target_link_libraries(svgfill ${Boost_LIBRARIES} ${BCRYPT_LIBRARIES} LibXml2::LibXml2 IFCOPENSHELL_CGAL) +target_link_libraries(svgfill ${Boost_LIBRARIES} ${BCRYPT_LIBRARIES} LibXml2::LibXml2 IFCOPENSHELL_CGAL IfcParse) set_target_properties(svgfill PROPERTIES PUBLIC_HEADER "${LIB_H_FILES}") add_executable(svgfill_exe src/main.cpp) diff --git a/src/svgfill/src/arrange_polygons.cpp b/src/svgfill/src/arrange_polygons.cpp index 260b22809e..5a99125adb 100644 --- a/src/svgfill/src/arrange_polygons.cpp +++ b/src/svgfill/src/arrange_polygons.cpp @@ -5,6 +5,8 @@ #include "svgfill.h" #endif +#include "../../ifcparse/IfcLogger.h" + #include #include #include @@ -1602,7 +1604,8 @@ std::map> snap_points_to_box_axes( DebugWriter& debug, const CenterLineGraphData& graph, const std::vector& boxes, - const K::FT& max_projection_distance) { + const K::FT& max_projection_distance, + Logger& logger) { std::vector snapped_points(graph.points.size()); for (size_t i = 0; i < graph.points.size(); ++i) { @@ -1685,7 +1688,11 @@ std::map> snap_points_to_box_axes( debug.write_segment(graph.points[i], best.projection, "snap_candidate_4"); } else { snapped_points[i] = graph.points[i]; - std::cout << "Warning: snapping distance exceeding distance: " << std::sqrt(CGAL::to_double((snapped_points[i] - best.projection).squared_length())) << " > " << max_projection_distance << std::endl; + std::ostringstream message; + message << "Snapping distance exceeds maximum distance: " + << std::sqrt(CGAL::to_double((snapped_points[i] - best.projection).squared_length())) + << " > " << max_projection_distance; + logger.Message(Logger::LOG_WARNING, "ARR", 1, message.str()); } } @@ -1711,7 +1718,8 @@ Graph2D join_segment_runs( DebugWriter& debug, const std::map>& line_graph, const std::map>& midpoint_to_segment, - const K::FT& max_projection_distance) { + const K::FT& max_projection_distance, + Logger& logger) { auto graph = make_center_line_graph_data(line_graph, midpoint_to_segment); auto runs = runs_from_graph(graph); runs.erase(std::remove_if(runs.begin(), runs.end(), [](const LineRun& run) { @@ -1742,7 +1750,7 @@ Graph2D join_segment_runs( } debug.write_polygons(run_polygons, "merged_boxes"); - auto snapped_graph = snap_points_to_box_axes(debug, graph, boxes, max_projection_distance); + auto snapped_graph = snap_points_to_box_axes(debug, graph, boxes, max_projection_distance, logger); return Graph2D(snapped_graph); } @@ -2239,7 +2247,9 @@ extend_end_vertices_based_on_input_simple( DebugWriter& debug_output, const Graph2D& G, const Polygon_list& outer_perimiter, - const K::FT& max_projection_distance, int pass) + const K::FT& max_projection_distance, + int pass, + Logger& logger) { auto max_intersection_distance = max_projection_distance / 4; @@ -2389,9 +2399,9 @@ extend_end_vertices_based_on_input_simple( } } if (within_any_perimeter) { - std::cout << "Within boundary but still no solution given" << std::endl; + logger.Message(Logger::LOG_WARNING, "ARR", 2, "Within boundary but no projection or intersection solution was found"); } else { - std::cout << "Outside of all boundaries" << std::endl; + logger.Message(Logger::LOG_WARNING, "ARR", 3, "Point is outside all boundaries"); } return boost::optional{}; }; @@ -2404,13 +2414,18 @@ extend_end_vertices_based_on_input_simple( auto& M = it->first; if (auto result = process_point(M, *it->second.begin())) { if (*result == M) { - std::cout << "Point already on perimeter (" << M.x() << " " << M.y() << ")" << std::endl; + std::ostringstream message; + message << "Point is already on perimeter (" << M.x() << " " << M.y() << ")"; + logger.Message(Logger::LOG_NOTICE, "ARR", 4, message.str()); continue; } auto d = (M - *result).squared_length(); solutions.emplace_back(d, M, *it->second.begin()); } else { - std::cout << "Unable to find projection or intersection point for interior boundary pass " << pass << " [round 1] (" << M.x() << " " << M.y() << ")" << std::endl; + std::ostringstream message; + message << "Unable to find projection or intersection point for interior boundary pass " + << pass << " [round 1] (" << M.x() << " " << M.y() << ")"; + logger.Message(Logger::LOG_WARNING, "ARR", 5, message.str()); } } } @@ -2424,12 +2439,17 @@ extend_end_vertices_based_on_input_simple( debug_output.write_segment(point, *result, "exterior_constructed_segment"); auto d = CGAL::squared_distance(point, *result); - std::cout << "Distance: " << std::sqrt(CGAL::to_double(d)) << std::endl; + std::ostringstream message; + message << "Projection or intersection distance: " << std::sqrt(CGAL::to_double(d)); + logger.Message(Logger::LOG_DEBUG, "ARR", 6, message.str()); validation_segments.emplace_back(to_3d(point), to_3d(*result)); auto inserted_it = std::prev(validation_segments.end()); validation_tree.insert(inserted_it, validation_segments.end()); } else { - std::cout << "Unable to find projection or intersection point for interior boundary pass " << pass << " [round 2] (" << point.x() << " " << point.y() << ")" << std::endl; + std::ostringstream message; + message << "Unable to find projection or intersection point for interior boundary pass " + << pass << " [round 2] (" << point.x() << " " << point.y() << ")"; + logger.Message(Logger::LOG_WARNING, "ARR", 7, message.str()); } } @@ -2528,7 +2548,7 @@ class Segment_2_less { } }; -std::vector arrangement_cell_iou(DebugWriter& debug_output, Arrangement_2& left, Arrangement_2& right) { +std::vector arrangement_cell_iou(DebugWriter& debug_output, Arrangement_2& left, Arrangement_2& right, Logger& logger) { using Walk_pl = CGAL::Arr_walk_along_line_point_location; Walk_pl walk_pl(right); @@ -2610,7 +2630,7 @@ std::vector arrangement_cell_iou(DebugWriter& debug_output, Arrangement_2 if (visited_faces_on_right.count(*v) > 0) { // Maybe we should be more permissive, try some other points etc. return_values.push_back(0); - std::cout << "Already visited face on right, skipping point\n"; + logger.Message(Logger::LOG_WARNING, "ARR", 8, "Already visited face on right; skipping point"); } else { // convert arr facet to polygon with holes auto polygon_exterior = circ_to_poly((*v)->outer_ccb()); @@ -2648,7 +2668,7 @@ std::vector arrangement_cell_iou(DebugWriter& debug_output, Arrangement_2 max_deviation_poly_pair = {pwh.outer_boundary(), pwh_right.outer_boundary()}; } } else { - std::cout << "No intersection, skipping point\n"; + logger.Message(Logger::LOG_WARNING, "ARR", 9, "No intersection; skipping point"); return_values.push_back(0); } } @@ -2670,7 +2690,7 @@ std::vector arrangement_cell_iou(DebugWriter& debug_output, Arrangement_2 return return_values; } -void clean_noisy_paths(DebugWriter& debug_output, Arrangement_2& arr, SegmentLookup& segment_lookup, double& threshold) { +void clean_noisy_paths(DebugWriter& debug_output, Arrangement_2& arr, SegmentLookup& segment_lookup, double& threshold, Logger& logger) { using SK = CGAL::Simple_cartesian; CGAL::Cartesian_converter C{}; @@ -2886,7 +2906,7 @@ void clean_noisy_paths(DebugWriter& debug_output, Arrangement_2& arr, SegmentLoo } } if (!removed) { - std::cerr << "Warning: unable to locate edge for removal, skipping" << std::endl; + logger.Message(Logger::LOG_WARNING, "ARR", 10, "Unable to locate edge for removal; skipping"); } } @@ -2971,7 +2991,7 @@ void clean_noisy_paths(DebugWriter& debug_output, Arrangement_2& arr, SegmentLoo #else auto arr_copy = arr; process_modifications(arr_copy, to_remove_this_path, to_insert_this_path); - auto ious = arrangement_cell_iou(arr, arr_copy); + auto ious = arrangement_cell_iou(debug_output, arr, arr_copy, logger); for (auto& iou : ious) { std::cerr << " - cell iou: " << CGAL::to_double(iou) << std::endl; } @@ -3303,28 +3323,36 @@ class timer { public: class entry { public: - entry() {} + entry() : logger_(nullptr) {} - entry(std::map::const_iterator start_it) - : start_it(start_it) {} + entry( + std::map::const_iterator start_it, + Logger& logger) + : start_it(start_it) + , logger_(&logger) {} void stop() { - if (start_it) { + if (start_it && logger_) { auto end = std::chrono::high_resolution_clock::now(); auto duration = std::chrono::duration(end - start_it.value()->second).count(); - std::cerr << "Timing for " << start_it.value()->first << ": " << duration << " ms" << std::endl; + std::ostringstream message; + message << "Timing for " << start_it.value()->first << ": " << duration << " ms"; + logger_->Message(Logger::LOG_PERF, "ARR", 11, message.str()); } } private: std::optional::const_iterator> start_it; + Logger* logger_; }; - timer(bool enabled = true) : enabled_(enabled) {} + timer(Logger& logger, bool enabled = true) + : logger_(logger) + , enabled_(enabled) {} entry start(const std::string& name) { if (enabled_) { - return entry(timings_.insert({name, std::chrono::high_resolution_clock::now()}).first); + return entry(timings_.insert({name, std::chrono::high_resolution_clock::now()}).first, logger_); } else { return entry(); } @@ -3336,6 +3364,7 @@ class timer { std::chrono::high_resolution_clock::time_point> timings_; + Logger& logger_; bool enabled_; }; @@ -3351,7 +3380,12 @@ size_t delete_same_facet_edge_pairs(Arrangement_2& arr) { return n_deleted; } -void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std::vector& input_polygons_, std::vector& output_polygons, double polygon_offset_distance = -1.) { +void arrange_cgal_polygons( + svgfill::arrange_polygon_settings settings, + const std::vector& input_polygons_, + std::vector& output_polygons, + Logger& logger, + double polygon_offset_distance = -1.) { static const double OVERLAP_RESOLUTION_DISTANCE = 1.e-1; // even larger amount of inset so that outer perimeter is safely within all input polygons even when overlap resolution is applied @@ -3371,7 +3405,7 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std debug_output = DebugWriter(false, ""); } - timer timer(settings.debug_output); + timer timer(logger, settings.debug_output); auto t0 = timer.start("input"); @@ -3578,7 +3612,7 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std for (int i = 0; i < 2; ++i) { auto it = line_graph.find(e.first); if (it == line_graph.end()) { - std::cerr << "Warning: unable to locate vertex for elimination, skipping" << std::endl; + logger.Message(Logger::LOG_WARNING, "ARR", 12, "Unable to locate vertex for elimination; skipping"); continue; } auto& neighbours = it->second; @@ -3607,7 +3641,7 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std }; if (settings.line_cleaning_algo == 0) { - G = join_segment_runs(debug_output, line_graph, midpoint_to_segment, subdivision_length * 4); + G = join_segment_runs(debug_output, line_graph, midpoint_to_segment, subdivision_length * 4, logger); Arrangement_2 arr; G.to_arrangement(arr); Graph2D G2; @@ -3629,8 +3663,8 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std bool fallback_to_line_cleaning_algo_1 = false; if (settings.line_cleaning_algo == 0) { - segments1 = extend_end_vertices_based_on_input_simple(debug_output, G, outer_perimiter, subdivision_length * 16, 0); - segments2 = extend_end_vertices_based_on_input_simple(debug_output, G_orig, outer_perimiter, subdivision_length * 16, 1); + segments1 = extend_end_vertices_based_on_input_simple(debug_output, G, outer_perimiter, subdivision_length * 16, 0, logger); + segments2 = extend_end_vertices_based_on_input_simple(debug_output, G_orig, outer_perimiter, subdivision_length * 16, 1, logger); Arrangement_2 arr_clean; G.to_arrangement(arr_clean); @@ -3668,7 +3702,7 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std debug_output.write_polygons(arr_clean, "iou_left"); debug_output.write_polygons(arr_orig, "iou_right"); - auto ious = arrangement_cell_iou(debug_output, arr_clean, arr_orig); + auto ious = arrangement_cell_iou(debug_output, arr_clean, arr_orig, logger); /* for (auto& iou : ious) { std::cout << " " << CGAL::to_double(iou - 1); @@ -3679,7 +3713,10 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std auto it = std::min_element(ious.begin(), ious.end()); if (it != ious.end() && (*it < 0.45)) { - std::cerr << "Significant difference between cleaned and original arrangement, using original for topology reconstruction: " << *it << std::endl; + std::ostringstream message; + message << "Significant difference between cleaned and original arrangement; using original for topology reconstruction: " + << *it; + logger.Message(Logger::LOG_WARNING, "ARR", 13, message.str()); fallback_to_line_cleaning_algo_1 = true; apply_line_cleaning_algo_1(); } else { @@ -3754,7 +3791,7 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std if (settings.perform_cleanup && settings.line_cleaning_algo != 0) { remove_colinear_vertices(arr); double threshold; - clean_noisy_paths(debug_output, arr, segment_lookup, threshold); + clean_noisy_paths(debug_output, arr, segment_lookup, threshold, logger); remove_colinear_vertices(arr); // clean_noisy_bounds(debug_output, arr, segment_lookup, threshold); } @@ -3773,7 +3810,11 @@ void arrange_cgal_polygons(svgfill::arrange_polygon_settings settings, const std #ifndef SVGFILL_MAIN -bool svgfill::arrange_polygons(arrange_polygon_settings settings, const std::vector& polygons, std::vector& arranged) { +bool svgfill::arrange_polygons( + arrange_polygon_settings settings, + const std::vector& polygons, + std::vector& arranged, + Logger& logger) { std::vector cgal_polygons, cgal_polygons_out; std::transform(polygons.begin(), polygons.end(), std::back_inserter(cgal_polygons), [](auto& poly) { Polygon_2 result; @@ -3782,7 +3823,7 @@ bool svgfill::arrange_polygons(arrange_polygon_settings settings, const std::vec }); return result; }); - arrange_cgal_polygons(settings, cgal_polygons, cgal_polygons_out); + arrange_cgal_polygons(settings, cgal_polygons, cgal_polygons_out, logger); std::transform(cgal_polygons_out.begin(), cgal_polygons_out.end(), std::back_inserter(arranged), [](auto& poly) { svgfill::polygon_2 result; std::transform(poly.begin(), poly.end(), std::back_inserter(result.boundary), [](auto& pt) { @@ -3812,6 +3853,9 @@ Polygon_2 create_rectangle(T x_min, T y_min, T x_max, T y_max) { int main(int argc, char** argv) { std::vector input_polygons, output; + Logger logger; + logger.SetOutput(&std::cout, &std::cerr); + logger.Verbosity(Logger::LOG_PERF); if (argc == 2) { using json = nlohmann::json; @@ -3820,7 +3864,7 @@ int main(int argc, char** argv) { file >> jsonData; size_t i = 0; for (const auto& item : jsonData.items()) { - std::cout << "i " << i << std::endl; + logger.Message(Logger::LOG_NOTICE, "ARR", 14, "Processing arrangement " + std::to_string(i)); i++; input_polygons.clear(); const auto& polygonsData = item.value(); @@ -3832,7 +3876,7 @@ int main(int argc, char** argv) { input_polygons.back().push_back(CGAL::Point_2(x, y)); } } - arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output); + arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output, logger); break; } return 0; @@ -3845,7 +3889,7 @@ int main(int argc, char** argv) { input_polygons = { rect1, rect2, rect3, rect4, rect5 }; } - arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output); + arrange_cgal_polygons(arrange_polygon_settings{}, input_polygons, output, logger); return 0; } diff --git a/src/svgfill/src/svgfill.h b/src/svgfill/src/svgfill.h index 2588636a8d..5bc3fa39e9 100644 --- a/src/svgfill/src/svgfill.h +++ b/src/svgfill/src/svgfill.h @@ -40,6 +40,8 @@ #include #include +class Logger; + namespace svgfill { typedef std::array point_2; typedef std::array line_segment_2; @@ -133,7 +135,7 @@ namespace svgfill { double subdivision_factor = 16.; }; - SVGFILL_API bool arrange_polygons(arrange_polygon_settings settings, const std::vector& polygons, std::vector& arranged); + SVGFILL_API bool arrange_polygons(arrange_polygon_settings settings, const std::vector& polygons, std::vector& arranged, Logger& logger); } #endif