From e1ff808f14f214dfcad2e4e779b43273b69e82f0 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Tue, 6 Aug 2019 11:29:26 +0200 Subject: [PATCH 1/4] Fixed parallelization of texture compression: Memory synchronization (memory barriers) are introduced using std::atomic variables. --- src/slic3r/GUI/GLTexture.cpp | 75 +++++++++++++++++------------------- src/slic3r/GUI/GLTexture.hpp | 18 +++++---- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/slic3r/GUI/GLTexture.cpp b/src/slic3r/GUI/GLTexture.cpp index 516f8b934..4fdf12489 100644 --- a/src/slic3r/GUI/GLTexture.cpp +++ b/src/slic3r/GUI/GLTexture.cpp @@ -27,49 +27,57 @@ namespace GUI { void GLTexture::Compressor::reset() { - if (m_is_compressing) - { - // force compression completion, if any + if (m_thread.joinable()) { m_abort_compressing = true; - // wait for compression completion, if any - while (m_is_compressing) {} - } - - m_levels.clear(); -} - -void GLTexture::Compressor::add_level(unsigned int w, unsigned int h, const std::vector& data) -{ - m_levels.emplace_back(w, h, data); + m_thread.join(); + m_levels.clear(); + m_num_levels_compressed = 0; + m_abort_compressing = false; + } + assert(m_levels.empty()); + assert(m_abort_compressing == false); + assert(m_num_levels_compressed == 0); } void GLTexture::Compressor::start_compressing() { - std::thread t(&GLTexture::Compressor::compress, this); - t.detach(); + // The worker thread should be stopped already. + assert(! m_thread.joinable()); + assert(! m_levels.empty()); + assert(m_abort_compressing == false); + assert(m_num_levels_compressed == 0); + if (! m_levels.empty()) { + std::thread thrd(&GLTexture::Compressor::compress, this); + m_thread = std::move(thrd); + } } bool GLTexture::Compressor::unsent_compressed_data_available() const { - for (const Level& level : m_levels) - { - if (!level.sent_to_gpu && level.compressed) + if (m_levels.empty()) + return false; + // Querying the atomic m_num_levels_compressed value synchronizes processor caches, so that the dat of m_levels modified by the worker thread are accessible to the calling thread. + unsigned int num_compressed = m_num_levels_compressed; + for (unsigned int i = 0; i < num_compressed; ++ i) + if (! m_levels[i].sent_to_gpu && ! m_levels[i].compressed_data.empty()) return true; - } - return false; } void GLTexture::Compressor::send_compressed_data_to_gpu() { // this method should be called inside the main thread of Slicer or a new OpenGL context (sharing resources) would be needed + if (m_levels.empty()) + return; glsafe(::glPixelStorei(GL_UNPACK_ALIGNMENT, 1)); glsafe(::glBindTexture(GL_TEXTURE_2D, m_texture.m_id)); - for (int i = 0; i < (int)m_levels.size(); ++i) + // Querying the atomic m_num_levels_compressed value synchronizes processor caches, so that the dat of m_levels modified by the worker thread are accessible to the calling thread. + int num_compressed = (int)m_num_levels_compressed; + for (int i = 0; i < num_compressed; ++ i) { Level& level = m_levels[i]; - if (!level.sent_to_gpu && level.compressed) + if (! level.sent_to_gpu && ! level.compressed_data.empty()) { glsafe(::glCompressedTexSubImage2D(GL_TEXTURE_2D, (GLint)i, 0, 0, (GLsizei)level.w, (GLsizei)level.h, GL_COMPRESSED_RGBA_S3TC_DXT5_EXT, (GLsizei)level.compressed_data.size(), (const GLvoid*)level.compressed_data.data())); glsafe(::glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, i)); @@ -77,29 +85,21 @@ void GLTexture::Compressor::send_compressed_data_to_gpu() level.sent_to_gpu = true; // we are done with the compressed data, we can discard it level.compressed_data.clear(); - level.compressed = false; } } glsafe(::glBindTexture(GL_TEXTURE_2D, 0)); -} -bool GLTexture::Compressor::all_compressed_data_sent_to_gpu() const -{ - for (const Level& level : m_levels) - { - if (!level.sent_to_gpu) - return false; - } - - return true; + if (num_compressed == (unsigned int)m_levels.size()) + // Finalize the worker thread, close it. + this->reset(); } void GLTexture::Compressor::compress() { // reference: https://github.com/Cyan4973/RygsDXTc - m_is_compressing = true; - m_abort_compressing = false; + assert(m_num_levels_compressed == 0); + assert(m_abort_compressing == false); for (Level& level : m_levels) { @@ -115,11 +115,8 @@ void GLTexture::Compressor::compress() // we are done with the source data, we can discard it level.src_data.clear(); - level.compressed = true; + ++ m_num_levels_compressed; } - - m_is_compressing = false; - m_abort_compressing = false; } GLTexture::Quad_UVs GLTexture::FullTextureUVs = { { 0.0f, 1.0f }, { 1.0f, 1.0f }, { 1.0f, 0.0f }, { 0.0f, 0.0f } }; diff --git a/src/slic3r/GUI/GLTexture.hpp b/src/slic3r/GUI/GLTexture.hpp index ec362944d..26829fa72 100644 --- a/src/slic3r/GUI/GLTexture.hpp +++ b/src/slic3r/GUI/GLTexture.hpp @@ -19,30 +19,34 @@ namespace GUI { unsigned int h; std::vector src_data; std::vector compressed_data; - bool compressed; bool sent_to_gpu; - Level(unsigned int w, unsigned int h, const std::vector& data) : w(w), h(h), src_data(data), compressed(false), sent_to_gpu(false) {} + Level(unsigned int w, unsigned int h, const std::vector& data) : w(w), h(h), src_data(data), sent_to_gpu(false) {} }; GLTexture& m_texture; std::vector m_levels; - bool m_is_compressing; - bool m_abort_compressing; + std::thread m_thread; + // Does the caller want the background thread to stop? + // This atomic also works as a memory barrier for synchronizing the cancel event with the worker thread. + std::atomic m_abort_compressing; + // How many levels were compressed since the start of the background processing thread? + // This atomic also works as a memory barrier for synchronizing results of the worker thread with the calling thread. + std::atomic m_num_levels_compressed; public: - explicit Compressor(GLTexture& texture) : m_texture(texture), m_is_compressing(false), m_abort_compressing(false) {} + explicit Compressor(GLTexture& texture) : m_texture(texture), m_abort_compressing(false), m_num_levels_compressed(0) {} ~Compressor() { reset(); } void reset(); - void add_level(unsigned int w, unsigned int h, const std::vector& data); + void add_level(unsigned int w, unsigned int h, const std::vector& data) { m_levels.emplace_back(w, h, data); } void start_compressing(); bool unsent_compressed_data_available() const; void send_compressed_data_to_gpu(); - bool all_compressed_data_sent_to_gpu() const; + bool all_compressed_data_sent_to_gpu() const { return m_levels.empty(); } private: void compress(); From 29d9c65ee2c3b8d53bb53c0c619e299648f17c3d Mon Sep 17 00:00:00 2001 From: bubnikv Date: Tue, 6 Aug 2019 11:40:33 +0200 Subject: [PATCH 2/4] Missing include (required by clang, not required by msvc) --- src/slic3r/GUI/GLTexture.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/slic3r/GUI/GLTexture.hpp b/src/slic3r/GUI/GLTexture.hpp index 26829fa72..2c8941632 100644 --- a/src/slic3r/GUI/GLTexture.hpp +++ b/src/slic3r/GUI/GLTexture.hpp @@ -3,6 +3,7 @@ #include #include +#include class wxImage; From 74e592ceaa22ee8793047ccaf26fdadbd46eff5d Mon Sep 17 00:00:00 2001 From: bubnikv Date: Tue, 6 Aug 2019 15:11:46 +0200 Subject: [PATCH 3/4] Improved handling of excessive extrusion width values (too small or too big). Fixes std: bad_alloc #2715 --- src/libslic3r/Flow.cpp | 20 +++++++++++++++----- src/libslic3r/Print.cpp | 33 +++++++++++++++++++++++++++++++-- src/slic3r/GUI/Tab.cpp | 16 +++++++++++++--- src/slic3r/GUI/Tab.hpp | 1 + 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/libslic3r/Flow.cpp b/src/libslic3r/Flow.cpp index e71b935db..6069677a1 100644 --- a/src/libslic3r/Flow.cpp +++ b/src/libslic3r/Flow.cpp @@ -76,10 +76,14 @@ float Flow::spacing() const return this->width + BRIDGE_EXTRA_SPACING; // rectangle with semicircles at the ends float min_flow_spacing = this->width - this->height * (1. - 0.25 * PI); - return this->width - PERIMETER_LINE_OVERLAP_FACTOR * (this->width - min_flow_spacing); + float res = this->width - PERIMETER_LINE_OVERLAP_FACTOR * (this->width - min_flow_spacing); #else - return float(this->bridge ? (this->width + BRIDGE_EXTRA_SPACING) : (this->width - this->height * (1. - 0.25 * PI))); + float res = float(this->bridge ? (this->width + BRIDGE_EXTRA_SPACING) : (this->width - this->height * (1. - 0.25 * PI))); #endif +// assert(res > 0.f); + if (res <= 0.f) + throw std::runtime_error("Flow::spacing() produced negative spacing. Did you set some extrusion width too small?"); + return res; } // This method returns the centerline spacing between an extrusion using this @@ -89,20 +93,26 @@ float Flow::spacing(const Flow &other) const { assert(this->height == other.height); assert(this->bridge == other.bridge); - return float(this->bridge ? + float res = float(this->bridge ? 0.5 * this->width + 0.5 * other.width + BRIDGE_EXTRA_SPACING : 0.5 * this->spacing() + 0.5 * other.spacing()); +// assert(res > 0.f); + if (res <= 0.f) + throw std::runtime_error("Flow::spacing() produced negative spacing. Did you set some extrusion width too small?"); + return res; } // This method returns extrusion volume per head move unit. double Flow::mm3_per_mm() const { - double res = this->bridge ? + float res = this->bridge ? // Area of a circle with dmr of this->width. (this->width * this->width) * 0.25 * PI : // Rectangle with semicircles at the ends. ~ h (w - 0.215 h) this->height * (this->width - this->height * (1. - 0.25 * PI)); - assert(res > 0.); + //assert(res > 0.); + if (res <= 0.) + throw std::runtime_error("Flow::mm3_per_mm() produced negative flow. Did you set some extrusion width too small?"); return res; } diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index e53f49810..1664acdde 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include //! macro used to mark string used at localization, @@ -1168,7 +1169,7 @@ std::string Print::validate() const bool has_custom_layering = false; std::vector> layer_height_profiles; for (const PrintObject *object : m_objects) { - has_custom_layering = ! object->model_object()->layer_config_ranges.empty() || ! object->model_object()->layer_height_profile.empty(); // #ys_FIXME_experiment + has_custom_layering = ! object->model_object()->layer_config_ranges.empty() || ! object->model_object()->layer_height_profile.empty(); if (has_custom_layering) { layer_height_profiles.assign(m_objects.size(), std::vector()); break; @@ -1247,6 +1248,18 @@ std::string Print::validate() const return L("One or more object were assigned an extruder that the printer does not have."); #endif + auto validate_extrusion_width = [min_nozzle_diameter, max_nozzle_diameter](const ConfigBase &config, const char *opt_key, double layer_height, std::string &err_msg) -> bool { + double extrusion_width_min = config.get_abs_value(opt_key, min_nozzle_diameter); + double extrusion_width_max = config.get_abs_value(opt_key, max_nozzle_diameter); + if (extrusion_width_min <= layer_height) { + err_msg = (boost::format(L("%1%=%2% mm is too low to be printable at a layer height %3% mm")) % opt_key % extrusion_width_min % layer_height).str(); + return false; + } else if (extrusion_width_max >= max_nozzle_diameter * 2.) { + err_msg = (boost::format(L("Excessive %1%=%2% mm to be printable with a nozzle diameter %3% mm")) % opt_key % extrusion_width_max % max_nozzle_diameter).str(); + return false; + } + return true; + }; for (PrintObject *object : m_objects) { if (object->config().raft_layers > 0 || object->config().support_material.value) { if ((object->config().support_material_extruder == 0 || object->config().support_material_interface_extruder == 0) && max_nozzle_diameter - min_nozzle_diameter > EPSILON) { @@ -1290,8 +1303,24 @@ std::string Print::validate() const return L("First layer height can't be greater than nozzle diameter"); // validate layer_height - if (object->config().layer_height.value > min_nozzle_diameter) + double layer_height = object->config().layer_height.value; + if (layer_height > min_nozzle_diameter) return L("Layer height can't be greater than nozzle diameter"); + + // Validate extrusion widths. + for (const char *opt_key : { "extrusion_width", "support_material_extrusion_width" }) { + std::string err_msg; + if (! validate_extrusion_width(object->config(), opt_key, layer_height, err_msg)) + return err_msg; + } + for (const char *opt_key : { "perimeter_extrusion_width", "external_perimeter_extrusion_width", "infill_extrusion_width", "solid_infill_extrusion_width", "top_infill_extrusion_width" }) { + for (size_t i = 0; i < object->region_volumes.size(); ++ i) + if (! object->region_volumes[i].empty()) { + std::string err_msg; + if (! validate_extrusion_width(this->get_region(i)->config(), opt_key, layer_height, err_msg)) + return err_msg; + } + } } } diff --git a/src/slic3r/GUI/Tab.cpp b/src/slic3r/GUI/Tab.cpp index 9d7fc20a3..ac89ba898 100644 --- a/src/slic3r/GUI/Tab.cpp +++ b/src/slic3r/GUI/Tab.cpp @@ -1758,6 +1758,17 @@ void TabFilament::reload_config() Tab::reload_config(); } +void TabFilament::update_volumetric_flow_preset_hints() +{ + wxString text; + try { + text = from_u8(PresetHints::maximum_volumetric_flow_description(*m_preset_bundle)); + } catch (std::exception &ex) { + text = _(L("Volumetric flow hints not available\n\n")) + from_u8(ex.what()); + } + m_volumetric_speed_description_line->SetText(text); +} + void TabFilament::update() { if (m_preset_bundle->printers.get_selected_preset().printer_technology() == ptSLA) @@ -1767,8 +1778,7 @@ void TabFilament::update() wxString text = from_u8(PresetHints::cooling_description(m_presets->get_edited_preset())); m_cooling_description_line->SetText(text); - text = from_u8(PresetHints::maximum_volumetric_flow_description(*m_preset_bundle)); - m_volumetric_speed_description_line->SetText(text); + this->update_volumetric_flow_preset_hints(); Layout(); bool cooling = m_config->opt_bool("cooling", 0); @@ -1790,7 +1800,7 @@ void TabFilament::update() void TabFilament::OnActivate() { - m_volumetric_speed_description_line->SetText(from_u8(PresetHints::maximum_volumetric_flow_description(*m_preset_bundle))); + this->update_volumetric_flow_preset_hints(); Tab::OnActivate(); } diff --git a/src/slic3r/GUI/Tab.hpp b/src/slic3r/GUI/Tab.hpp index 6ff76f5c4..423bc198f 100644 --- a/src/slic3r/GUI/Tab.hpp +++ b/src/slic3r/GUI/Tab.hpp @@ -340,6 +340,7 @@ class TabFilament : public Tab void add_filament_overrides_page(); void update_filament_overrides_page(); + void update_volumetric_flow_preset_hints(); std::map m_overrides_options; public: From 9905f8d349f8e4f0f4a38517adb5c9d6936ab305 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Tue, 6 Aug 2019 15:36:16 +0200 Subject: [PATCH 4/4] Fix of the previous commit: zero extrusion width parameter is always valid, it is replaced with an "auto" value. --- src/libslic3r/Print.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 1664acdde..9df122cee 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -1251,7 +1251,9 @@ std::string Print::validate() const auto validate_extrusion_width = [min_nozzle_diameter, max_nozzle_diameter](const ConfigBase &config, const char *opt_key, double layer_height, std::string &err_msg) -> bool { double extrusion_width_min = config.get_abs_value(opt_key, min_nozzle_diameter); double extrusion_width_max = config.get_abs_value(opt_key, max_nozzle_diameter); - if (extrusion_width_min <= layer_height) { + if (extrusion_width_min == 0) { + // Default "auto-generated" extrusion width is always valid. + } else if (extrusion_width_min <= layer_height) { err_msg = (boost::format(L("%1%=%2% mm is too low to be printable at a layer height %3% mm")) % opt_key % extrusion_width_min % layer_height).str(); return false; } else if (extrusion_width_max >= max_nozzle_diameter * 2.) {