From 3dd9b8c718c22e4ee7a000341b6d6c72b23df206 Mon Sep 17 00:00:00 2001
From: Vojtech Bubnik <bubnikv@gmail.com>
Date: Thu, 3 Dec 2020 12:50:18 +0100
Subject: [PATCH] Further improvement error reporting with buggy custom G-code
sections #1516 1) The macro-processor sanitizes the source code line for
invalid UTF-8 characters. Ideally these invalid characters would be
replaced with ?, they are just dropped as of now. Better than showing an
empty string when converting to wxString though. 2) G-code export collects
full error message for 1st occurence of an error for each custom G-code
block. 3) The composite error message now displays the errors collected in
2). 4) The error window is now scaled bigger and the Slicer logo is smaller
if the text is to be rendered with monospaced font, as the monospaced
text will not be word wrapped.
---
src/libslic3r/GCode.cpp | 12 +++++++++---
src/libslic3r/GCode.hpp | 3 ++-
src/libslic3r/PlaceholderParser.cpp | 5 ++++-
src/slic3r/GUI/BackgroundSlicingProcess.cpp | 8 ++++++--
src/slic3r/GUI/BackgroundSlicingProcess.hpp | 3 ++-
src/slic3r/GUI/MsgDialog.cpp | 5 +++--
src/slic3r/GUI/Plater.cpp | 10 +++++-----
7 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp
index 743c3f535..5c582f0f6 100644
--- a/src/libslic3r/GCode.cpp
+++ b/src/libslic3r/GCode.cpp
@@ -748,14 +748,16 @@ void GCode::do_export(Print* print, const char* path, GCodeProcessor::Result* re
if (! m_placeholder_parser_failed_templates.empty()) {
// G-code export proceeded, but some of the PlaceholderParser substitutions failed.
+ //FIXME localize!
std::string msg = std::string("G-code export to ") + path + " failed due to invalid custom G-code sections:\n\n";
- for (const std::string &name : m_placeholder_parser_failed_templates)
- msg += std::string("\t") + name + "\n";
+ for (const auto &name_and_error : m_placeholder_parser_failed_templates)
+ msg += name_and_error.first + "\n" + name_and_error.second + "\n";
msg += "\nPlease inspect the file ";
msg += path_tmp + " for error messages enclosed between\n";
msg += " !!!!! Failed to process the custom G-code template ...\n";
msg += "and\n";
msg += " !!!!! End of an error report for the custom G-code template ...\n";
+ msg += "for all macro processing errors.";
throw Slic3r::PlaceholderParserError(msg);
}
@@ -1434,7 +1436,11 @@ std::string GCode::placeholder_parser_process(const std::string &name, const std
return m_placeholder_parser.process(templ, current_extruder_id, config_override);
} catch (std::runtime_error &err) {
// Collect the names of failed template substitutions for error reporting.
- m_placeholder_parser_failed_templates.insert(name);
+ auto it = m_placeholder_parser_failed_templates.find(name);
+ if (it == m_placeholder_parser_failed_templates.end())
+ // Only if there was no error reported for this template, store the first error message into the map to be reported.
+ // We don't want to collect error message for each and every occurence of a single custom G-code section.
+ m_placeholder_parser_failed_templates.insert(it, std::make_pair(name, std::string(err.what())));
// Insert the macro error message into the G-code.
return
std::string("\n!!!!! Failed to process the custom G-code template ") + name + "\n" +
diff --git a/src/libslic3r/GCode.hpp b/src/libslic3r/GCode.hpp
index 377b27381..8a8154976 100644
--- a/src/libslic3r/GCode.hpp
+++ b/src/libslic3r/GCode.hpp
@@ -19,6 +19,7 @@
#include "GCode/ThumbnailData.hpp"
#include <memory>
+#include <map>
#include <string>
#ifdef HAS_PRESSURE_EQUALIZER
@@ -323,7 +324,7 @@ private:
GCodeWriter m_writer;
PlaceholderParser m_placeholder_parser;
// Collection of templates, on which the placeholder substitution failed.
- std::set<std::string> m_placeholder_parser_failed_templates;
+ std::map<std::string, std::string> m_placeholder_parser_failed_templates;
OozePrevention m_ooze_prevention;
Wipe m_wipe;
AvoidCrossingPerimeters m_avoid_crossing_perimeters;
diff --git a/src/libslic3r/PlaceholderParser.cpp b/src/libslic3r/PlaceholderParser.cpp
index 9bbc2cd8d..8d4a88b14 100644
--- a/src/libslic3r/PlaceholderParser.cpp
+++ b/src/libslic3r/PlaceholderParser.cpp
@@ -6,6 +6,7 @@
#include <iomanip>
#include <sstream>
#include <map>
+#include <boost/nowide/convert.hpp>
#ifdef _MSC_VER
#include <stdlib.h> // provides **_environ
#else
@@ -870,7 +871,9 @@ namespace client
}
}
msg += '\n';
- msg += error_line;
+ // This hack removes all non-UTF8 characters from the source line, so that the upstream wxWidgets conversions
+ // from UTF8 to UTF16 don't bail out.
+ msg += boost::nowide::narrow(boost::nowide::widen(error_line));
msg += '\n';
for (size_t i = 0; i < error_pos; ++ i)
msg += ' ';
diff --git a/src/slic3r/GUI/BackgroundSlicingProcess.cpp b/src/slic3r/GUI/BackgroundSlicingProcess.cpp
index 1d309299a..0e51ca77c 100644
--- a/src/slic3r/GUI/BackgroundSlicingProcess.cpp
+++ b/src/slic3r/GUI/BackgroundSlicingProcess.cpp
@@ -68,9 +68,10 @@ bool SlicingProcessCompletedEvent::invalidate_plater() const
return false;
}
-std::string SlicingProcessCompletedEvent::format_error_message() const
+std::pair<std::string, bool> SlicingProcessCompletedEvent::format_error_message() const
{
std::string error;
+ bool monospace = false;
try {
this->rethrow_exception();
} catch (const std::bad_alloc& ex) {
@@ -78,12 +79,15 @@ std::string SlicingProcessCompletedEvent::format_error_message() const
"If you are sure you have enough RAM on your system, this may also be a bug and we would "
"be glad if you reported it."))) % SLIC3R_APP_NAME).str());
error = std::string(errmsg.ToUTF8()) + "\n\n" + std::string(ex.what());
+ } catch (PlaceholderParserError &ex) {
+ error = ex.what();
+ monospace = true;
} catch (std::exception &ex) {
error = ex.what();
} catch (...) {
error = "Unknown C++ exception.";
}
- return error;
+ return std::make_pair(std::move(error), monospace);
}
BackgroundSlicingProcess::BackgroundSlicingProcess()
diff --git a/src/slic3r/GUI/BackgroundSlicingProcess.hpp b/src/slic3r/GUI/BackgroundSlicingProcess.hpp
index d4fc3ddcc..b3f8a0a6b 100644
--- a/src/slic3r/GUI/BackgroundSlicingProcess.hpp
+++ b/src/slic3r/GUI/BackgroundSlicingProcess.hpp
@@ -57,7 +57,8 @@ public:
// Only valid if error()
void rethrow_exception() const { assert(this->error()); assert(m_exception); std::rethrow_exception(m_exception); }
// Produce a human readable message to be displayed by a notification or a message box.
- std::string format_error_message() const;
+ // 2nd parameter defines whether the output should be displayed with a monospace font.
+ std::pair<std::string, bool> format_error_message() const;
private:
StatusType m_status;
diff --git a/src/slic3r/GUI/MsgDialog.cpp b/src/slic3r/GUI/MsgDialog.cpp
index 5ca8aa49d..043224375 100644
--- a/src/slic3r/GUI/MsgDialog.cpp
+++ b/src/slic3r/GUI/MsgDialog.cpp
@@ -75,7 +75,7 @@ ErrorDialog::ErrorDialog(wxWindow *parent, const wxString &msg, bool monospaced_
// Text shown as HTML, so that mouse selection and Ctrl-V to copy will work.
wxHtmlWindow* html = new wxHtmlWindow(this, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxHW_SCROLLBAR_AUTO);
{
- html->SetMinSize(wxSize(40 * wxGetApp().em_unit(), -1));
+ html->SetMinSize(wxSize(40 * wxGetApp().em_unit(), monospaced_font ? 30 * wxGetApp().em_unit() : -1));
wxFont font = wxSystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT);
wxFont monospace = wxSystemSettings::GetFont(wxSYS_ANSI_FIXED_FONT);
wxColour text_clr = wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOWTEXT);
@@ -100,7 +100,8 @@ ErrorDialog::ErrorDialog(wxWindow *parent, const wxString &msg, bool monospaced_
btn_ok->SetFocus();
btn_sizer->Add(btn_ok, 0, wxRIGHT, HORIZ_SPACING);
- logo->SetBitmap(create_scaled_bitmap("PrusaSlicer_192px_grayscale.png", this, 192));
+ // Use a small bitmap with monospaced font, as the error text will not be wrapped.
+ logo->SetBitmap(create_scaled_bitmap("PrusaSlicer_192px_grayscale.png", this, monospaced_font ? 48 : 192));
SetMaxSize(wxSize(-1, CONTENT_MAX_HEIGHT*wxGetApp().em_unit()));
Fit();
diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp
index 178644af4..ba54702e2 100644
--- a/src/slic3r/GUI/Plater.cpp
+++ b/src/slic3r/GUI/Plater.cpp
@@ -3643,17 +3643,17 @@ void Plater::priv::on_process_completed(SlicingProcessCompletedEvent &evt)
// This bool stops showing export finished notification even when process_completed_with_error is false
bool has_error = false;
if (evt.error()) {
- std::string message = evt.format_error_message();
+ std::pair<std::string, bool> message = evt.format_error_message();
if (evt.critical_error()) {
if (q->m_tracking_popup_menu)
// We don't want to pop-up a message box when tracking a pop-up menu.
// We postpone the error message instead.
- q->m_tracking_popup_menu_error_message = message;
+ q->m_tracking_popup_menu_error_message = message.first;
else
- show_error(q, message);
+ show_error(q, message.first, message.second);
} else
- notification_manager->push_slicing_error_notification(message, *q->get_current_canvas3D());
- this->statusbar()->set_status_text(from_u8(message));
+ notification_manager->push_slicing_error_notification(message.first, *q->get_current_canvas3D());
+ this->statusbar()->set_status_text(from_u8(message.first));
if (evt.invalidate_plater())
{
const wxString invalid_str = _L("Invalid data");