From d2e5be89e3460964fb3c38db066a21020de0f0db Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Thu, 29 Oct 2020 10:51:28 +0100 Subject: [PATCH] Fix of Slicer image not good #4992 Tighter parsing of PrusaSlicer's own G-code annotations to avoid clashes with comments inside user G-codes. Also the GCodeReader was extended to return string_views instead of copying a substring, and the GCodeProcessor was partially adapted to string_views. --- src/libslic3r/ExtrusionEntity.cpp | 2 +- src/libslic3r/ExtrusionEntity.hpp | 3 +- src/libslic3r/GCode/GCodeProcessor.cpp | 186 ++++++++++------------ src/libslic3r/GCode/GCodeProcessor.hpp | 19 +-- src/libslic3r/GCode/PressureEqualizer.cpp | 3 +- src/libslic3r/GCodeReader.hpp | 11 +- 6 files changed, 103 insertions(+), 121 deletions(-) diff --git a/src/libslic3r/ExtrusionEntity.cpp b/src/libslic3r/ExtrusionEntity.cpp index b2c5e1350..aa9f0f9cd 100644 --- a/src/libslic3r/ExtrusionEntity.cpp +++ b/src/libslic3r/ExtrusionEntity.cpp @@ -331,7 +331,7 @@ std::string ExtrusionEntity::role_to_string(ExtrusionRole role) return ""; } -ExtrusionRole ExtrusionEntity::string_to_role(const std::string& role) +ExtrusionRole ExtrusionEntity::string_to_role(const std::string_view role) { if (role == L("Perimeter")) return erPerimeter; diff --git a/src/libslic3r/ExtrusionEntity.hpp b/src/libslic3r/ExtrusionEntity.hpp index 0adb2019e..6b0153b2e 100644 --- a/src/libslic3r/ExtrusionEntity.hpp +++ b/src/libslic3r/ExtrusionEntity.hpp @@ -6,6 +6,7 @@ #include "Polyline.hpp" #include +#include namespace Slic3r { @@ -106,7 +107,7 @@ public: virtual double total_volume() const = 0; static std::string role_to_string(ExtrusionRole role); - static ExtrusionRole string_to_role(const std::string& role); + static ExtrusionRole string_to_role(const std::string_view role); }; typedef std::vector ExtrusionEntitiesPtr; diff --git a/src/libslic3r/GCode/GCodeProcessor.cpp b/src/libslic3r/GCode/GCodeProcessor.cpp index 209e76fa8..4a971960b 100644 --- a/src/libslic3r/GCode/GCodeProcessor.cpp +++ b/src/libslic3r/GCode/GCodeProcessor.cpp @@ -9,6 +9,7 @@ #include #include +#include #if ENABLE_GCODE_VIEWER #include @@ -748,9 +749,9 @@ void GCodeProcessor::process_file(const std::string& filename, std::function 1 && detect_producer(comment)) m_parser.quit_parsing_file(); } @@ -874,7 +875,7 @@ void GCodeProcessor::process_gcode_line(const GCodeReader::GCodeLine& line) // update start position m_start_position = m_end_position; - std::string cmd = line.cmd(); + const std::string_view cmd = line.cmd(); if (cmd.length() > 1) { // process command lines switch (::toupper(cmd[0])) @@ -932,122 +933,115 @@ void GCodeProcessor::process_gcode_line(const GCodeReader::GCodeLine& line) } } else { - std::string comment = line.comment(); - if (comment.length() > 1) - // process tags embedded into comments - process_tags(comment); + const std::string &comment = line.raw(); + if (comment.length() > 2 && comment.front() == ';') + // Process tags embedded into comments. Tag comments always start at the start of a line + // with a comment and continue with a tag without any whitespace separator. + process_tags(comment.substr(1)); } } -void GCodeProcessor::process_tags(const std::string& comment) +static inline bool starts_with(const std::string_view comment, const std::string_view tag) +{ + size_t tag_len = tag.size(); + return comment.size() >= tag_len && comment.substr(0, tag_len) == tag; +} + +// Returns true if the number was parsed correctly into out and the number spanned the whole input string. +template +static inline bool parse_number(const std::string_view str, T &out) +{ + auto str_end = str.data() + str.size(); + auto [end_ptr, error_code] = std::from_chars(str.data(), str_end, out); + return error_code == std::errc() && end_ptr == str_end; +} + +void GCodeProcessor::process_tags(const std::string_view comment) { // producers tags - if (m_producers_enabled) { - if (m_producer != EProducer::Unknown) { - if (process_producers_tags(comment)) - return; - } - } + if (m_producers_enabled && process_producers_tags(comment)) + return; // extrusion role tag - size_t pos = comment.find(Extrusion_Role_Tag); - if (pos != comment.npos) { - m_extrusion_role = ExtrusionEntity::string_to_role(comment.substr(pos + Extrusion_Role_Tag.length())); + if (starts_with(comment, Extrusion_Role_Tag)) { + m_extrusion_role = ExtrusionEntity::string_to_role(comment.substr(Extrusion_Role_Tag.length())); return; } - if (!m_producers_enabled || m_producer == EProducer::PrusaSlicer) { + if ((!m_producers_enabled || m_producer == EProducer::PrusaSlicer) && + starts_with(comment, Height_Tag)) { // height tag - pos = comment.find(Height_Tag); - if (pos != comment.npos) { - try { - m_height = std::stof(comment.substr(pos + Height_Tag.length())); - } - catch (...) { - BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Height (" << comment << ")."; - } - return; - } + if (! parse_number(comment.substr(Height_Tag.size()), m_height)) + BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Height (" << comment << ")."; + return; } #if ENABLE_GCODE_VIEWER_DATA_CHECKING // width tag - pos = comment.find(Width_Tag); - if (pos != comment.npos) { - try { - m_width_compare.last_tag_value = std::stof(comment.substr(pos + Width_Tag.length())); - } - catch (...) { + if (starts_with(comment, Width_Tag)) { + if (! parse_number(comment.substr(Width_Tag.size()), m_width_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Width (" << comment << ")."; - } return; } #endif // ENABLE_GCODE_VIEWER_DATA_CHECKING // color change tag - pos = comment.find(Color_Change_Tag); - if (pos != comment.npos) { - pos = comment.find_last_of(",T"); - try { - unsigned char extruder_id = (pos == comment.npos) ? 0 : static_cast(std::stoi(comment.substr(pos + 1))); - - m_extruder_colors[extruder_id] = static_cast(m_extruder_offsets.size()) + m_cp_color.counter; // color_change position in list of color for preview - ++m_cp_color.counter; - if (m_cp_color.counter == UCHAR_MAX) - m_cp_color.counter = 0; - - if (m_extruder_id == extruder_id) { - m_cp_color.current = m_extruder_colors[extruder_id]; - store_move_vertex(EMoveType::Color_change); + if (starts_with(comment, Color_Change_Tag)) { + unsigned char extruder_id = 0; + if (starts_with(comment.substr(Color_Change_Tag.size()), ",T")) { + int eid; + if (! parse_number(comment.substr(Color_Change_Tag.size() + 2), eid) || eid < 0 || eid > 255) { + BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Color_Change (" << comment << ")."; + return; } + extruder_id = static_cast(eid); + } - process_custom_gcode_time(CustomGCode::ColorChange); - } - catch (...) { - BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Color_Change (" << comment << ")."; + m_extruder_colors[extruder_id] = static_cast(m_extruder_offsets.size()) + m_cp_color.counter; // color_change position in list of color for preview + ++m_cp_color.counter; + if (m_cp_color.counter == UCHAR_MAX) + m_cp_color.counter = 0; + + if (m_extruder_id == extruder_id) { + m_cp_color.current = m_extruder_colors[extruder_id]; + store_move_vertex(EMoveType::Color_change); } + process_custom_gcode_time(CustomGCode::ColorChange); + return; } // pause print tag - pos = comment.find(Pause_Print_Tag); - if (pos != comment.npos) { + if (comment == Pause_Print_Tag) { store_move_vertex(EMoveType::Pause_Print); process_custom_gcode_time(CustomGCode::PausePrint); return; } // custom code tag - pos = comment.find(Custom_Code_Tag); - if (pos != comment.npos) { + if (comment == Custom_Code_Tag) { store_move_vertex(EMoveType::Custom_GCode); return; } #if ENABLE_GCODE_VIEWER_DATA_CHECKING // mm3_per_mm print tag - pos = comment.find(Mm3_Per_Mm_Tag); - if (pos != comment.npos) { - try { - m_mm3_per_mm_compare.last_tag_value = std::stof(comment.substr(pos + Mm3_Per_Mm_Tag.length())); - } - catch (...) { + if (starts_with(comment, Mm3_Per_Mm_Tag)) { + if (! parse_number(comment.substr(Mm3_Per_Mm_Tag.size()), m_mm3_per_mm_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Mm3_Per_Mm (" << comment << ")."; - } return; } #endif // ENABLE_GCODE_VIEWER_DATA_CHECKING // layer change tag - pos = comment.find(Layer_Change_Tag); - if (pos != comment.npos) { + if (comment == Layer_Change_Tag) { ++m_layer_id; return; } } -bool GCodeProcessor::process_producers_tags(const std::string& comment) +bool GCodeProcessor::process_producers_tags(const std::string_view comment) { switch (m_producer) { @@ -1060,18 +1054,18 @@ bool GCodeProcessor::process_producers_tags(const std::string& comment) } } -bool GCodeProcessor::process_prusaslicer_tags(const std::string& comment) +bool GCodeProcessor::process_prusaslicer_tags(const std::string_view comment) { return false; } -bool GCodeProcessor::process_cura_tags(const std::string& comment) +bool GCodeProcessor::process_cura_tags(const std::string_view comment) { // TYPE -> extrusion role std::string tag = "TYPE:"; size_t pos = comment.find(tag); if (pos != comment.npos) { - std::string type = comment.substr(pos + tag.length()); + const std::string_view type = comment.substr(pos + tag.length()); if (type == "SKIRT") m_extrusion_role = erSkirt; else if (type == "WALL-OUTER") @@ -1100,7 +1094,7 @@ bool GCodeProcessor::process_cura_tags(const std::string& comment) tag = "FLAVOR:"; pos = comment.find(tag); if (pos != comment.npos) { - std::string flavor = comment.substr(pos + tag.length()); + const std::string_view flavor = comment.substr(pos + tag.length()); if (flavor == "BFB") m_flavor = gcfMarlin; // << ??????????????????????? else if (flavor == "Mach3") @@ -1128,7 +1122,7 @@ bool GCodeProcessor::process_cura_tags(const std::string& comment) return false; } -bool GCodeProcessor::process_simplify3d_tags(const std::string& comment) +bool GCodeProcessor::process_simplify3d_tags(const std::string_view comment) { // extrusion roles @@ -1216,7 +1210,7 @@ bool GCodeProcessor::process_simplify3d_tags(const std::string& comment) std::string tag = " tool"; pos = comment.find(tag); if (pos == 0) { - std::string data = comment.substr(pos + tag.length()); + const std::string_view data = comment.substr(pos + tag.length()); std::string h_tag = "H"; size_t h_start = data.find(h_tag); size_t h_end = data.find_first_of(' ', h_start); @@ -1224,20 +1218,12 @@ bool GCodeProcessor::process_simplify3d_tags(const std::string& comment) size_t w_start = data.find(w_tag); size_t w_end = data.find_first_of(' ', w_start); if (h_start != data.npos) { - try { - m_height_compare.last_tag_value = std::stof(data.substr(h_start + 1, (h_end != data.npos) ? h_end - h_start - 1 : h_end)); - } - catch (...) { + if (! parse_number(data.substr(h_start + 1, (h_end != data.npos) ? h_end - h_start - 1 : h_end), m_height_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Height (" << comment << ")."; - } } if (w_start != data.npos) { - try { - m_width_compare.last_tag_value = std::stof(data.substr(w_start + 1, (w_end != data.npos) ? w_end - w_start - 1 : w_end)); - } - catch (...) { + if (! parse_number(data.substr(w_start + 1, (w_end != data.npos) ? w_end - w_start - 1 : w_end), m_width_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Width (" << comment << ")."; - } } return true; @@ -1247,13 +1233,13 @@ bool GCodeProcessor::process_simplify3d_tags(const std::string& comment) return false; } -bool GCodeProcessor::process_craftware_tags(const std::string& comment) +bool GCodeProcessor::process_craftware_tags(const std::string_view comment) { // segType -> extrusion role std::string tag = "segType:"; size_t pos = comment.find(tag); if (pos != comment.npos) { - std::string type = comment.substr(pos + tag.length()); + const std::string_view type = comment.substr(pos + tag.length()); if (type == "Skirt") m_extrusion_role = erSkirt; else if (type == "Perimeter") @@ -1287,13 +1273,13 @@ bool GCodeProcessor::process_craftware_tags(const std::string& comment) return false; } -bool GCodeProcessor::process_ideamaker_tags(const std::string& comment) +bool GCodeProcessor::process_ideamaker_tags(const std::string_view comment) { // TYPE -> extrusion role std::string tag = "TYPE:"; size_t pos = comment.find(tag); if (pos != comment.npos) { - std::string type = comment.substr(pos + tag.length()); + const std::string_view type = comment.substr(pos + tag.length()); if (type == "RAFT") m_extrusion_role = erSkirt; else if (type == "WALL-OUTER") @@ -1322,12 +1308,8 @@ bool GCodeProcessor::process_ideamaker_tags(const std::string& comment) tag = "WIDTH:"; pos = comment.find(tag); if (pos != comment.npos) { - try { - m_width_compare.last_tag_value = std::stof(comment.substr(pos + tag.length())); - } - catch (...) { + if (! parse_number(comment.substr(pos + tag.length()), m_width_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Width (" << comment << ")."; - } return true; } @@ -1335,12 +1317,8 @@ bool GCodeProcessor::process_ideamaker_tags(const std::string& comment) tag = "HEIGHT:"; pos = comment.find(tag); if (pos != comment.npos) { - try { - m_height_compare.last_tag_value = std::stof(comment.substr(pos + tag.length())); - } - catch (...) { + if (! parse_number(comment.substr(pos + tag.length()), m_height_compare.last_tag_value)) BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid value for Height (" << comment << ")."; - } return true; } #endif // ENABLE_GCODE_VIEWER_DATA_CHECKING @@ -1348,7 +1326,7 @@ bool GCodeProcessor::process_ideamaker_tags(const std::string& comment) return false; } -bool GCodeProcessor::detect_producer(const std::string& comment) +bool GCodeProcessor::detect_producer(const std::string_view comment) { for (const auto& [id, search_string] : Producers) { size_t pos = comment.find(search_string); @@ -2004,11 +1982,14 @@ void GCodeProcessor::process_T(const GCodeReader::GCodeLine& line) process_T(line.cmd()); } -void GCodeProcessor::process_T(const std::string& command) +void GCodeProcessor::process_T(const std::string_view command) { if (command.length() > 1) { - try { - unsigned char id = static_cast(std::stoi(command.substr(1))); + int eid; + if (! parse_number(command.substr(1), eid) || eid < 0 || eid > 255) { + BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid toolchange (" << command << ")."; + } else { + unsigned char id = static_cast(eid); if (m_extruder_id != id) { unsigned char extruders_count = static_cast(m_extruder_offsets.size()); if (id >= extruders_count) @@ -2030,9 +2011,6 @@ void GCodeProcessor::process_T(const std::string& command) store_move_vertex(EMoveType::Tool_change); } } - catch (...) { - BOOST_LOG_TRIVIAL(error) << "GCodeProcessor encountered an invalid toolchange (" << command << ")."; - } } } diff --git a/src/libslic3r/GCode/GCodeProcessor.hpp b/src/libslic3r/GCode/GCodeProcessor.hpp index 8f0c26cf5..5f9c4ae45 100644 --- a/src/libslic3r/GCode/GCodeProcessor.hpp +++ b/src/libslic3r/GCode/GCodeProcessor.hpp @@ -11,6 +11,7 @@ #include #include #include +#include namespace Slic3r { @@ -446,15 +447,15 @@ namespace Slic3r { void process_gcode_line(const GCodeReader::GCodeLine& line); // Process tags embedded into comments - void process_tags(const std::string& comment); - bool process_producers_tags(const std::string& comment); - bool process_prusaslicer_tags(const std::string& comment); - bool process_cura_tags(const std::string& comment); - bool process_simplify3d_tags(const std::string& comment); - bool process_craftware_tags(const std::string& comment); - bool process_ideamaker_tags(const std::string& comment); + void process_tags(const std::string_view comment); + bool process_producers_tags(const std::string_view comment); + bool process_prusaslicer_tags(const std::string_view comment); + bool process_cura_tags(const std::string_view comment); + bool process_simplify3d_tags(const std::string_view comment); + bool process_craftware_tags(const std::string_view comment); + bool process_ideamaker_tags(const std::string_view comment); - bool detect_producer(const std::string& comment); + bool detect_producer(const std::string_view comment); // Move void process_G0(const GCodeReader::GCodeLine& line); @@ -540,7 +541,7 @@ namespace Slic3r { // Processes T line (Select Tool) void process_T(const GCodeReader::GCodeLine& line); - void process_T(const std::string& command); + void process_T(const std::string_view command); void store_move_vertex(EMoveType type); diff --git a/src/libslic3r/GCode/PressureEqualizer.cpp b/src/libslic3r/GCode/PressureEqualizer.cpp index 33601e5e9..c3f084a24 100644 --- a/src/libslic3r/GCode/PressureEqualizer.cpp +++ b/src/libslic3r/GCode/PressureEqualizer.cpp @@ -165,9 +165,10 @@ static inline float parse_float(const char *&line) return result; }; -#define EXTRUSION_ROLE_TAG ";_EXTRUSION_ROLE:" bool PressureEqualizer::process_line(const char *line, const size_t len, GCodeLine &buf) { + static constexpr const char *EXTRUSION_ROLE_TAG = ";_EXTRUSION_ROLE:"; + if (strncmp(line, EXTRUSION_ROLE_TAG, strlen(EXTRUSION_ROLE_TAG)) == 0) { line += strlen(EXTRUSION_ROLE_TAG); int role = atoi(line); diff --git a/src/libslic3r/GCodeReader.hpp b/src/libslic3r/GCodeReader.hpp index 7e0793cd9..8d835185e 100644 --- a/src/libslic3r/GCodeReader.hpp +++ b/src/libslic3r/GCodeReader.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include "PrintConfig.hpp" namespace Slic3r { @@ -17,13 +18,13 @@ public: GCodeLine() { reset(); } void reset() { m_mask = 0; memset(m_axis, 0, sizeof(m_axis)); m_raw.clear(); } - const std::string& raw() const { return m_raw; } - const std::string cmd() const { + const std::string& raw() const { return m_raw; } + const std::string_view cmd() const { const char *cmd = GCodeReader::skip_whitespaces(m_raw.c_str()); - return std::string(cmd, GCodeReader::skip_word(cmd)); + return std::string_view(cmd, GCodeReader::skip_word(cmd) - cmd); } - const std::string comment() const - { size_t pos = m_raw.find(';'); return (pos == std::string::npos) ? "" : m_raw.substr(pos + 1); } + const std::string_view comment() const + { size_t pos = m_raw.find(';'); return (pos == std::string::npos) ? std::string_view() : std::string_view(m_raw).substr(pos + 1); } bool has(Axis axis) const { return (m_mask & (1 << int(axis))) != 0; } float value(Axis axis) const { return m_axis[axis]; }