WIP PrintRegion refactoring: Another round of bug fixing, negative volumes

seem to work.

7 files changed, 121 insertions, 101 deletions

diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp
index f5fefd812..58959f94b 100644
--- a/src/libslic3r/Model.cpp
+++ b/src/libslic3r/Model.cpp
@@ -2034,45 +2034,56 @@ bool model_object_list_extended(const Model &model_old, const Model &model_new)
     return true;
 }
 
-bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const ModelVolumeType type)
+template<typename TypeFilterFn>
+bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, TypeFilterFn type_filter)
 {
     size_t i_old, i_new;
     for (i_old = 0, i_new = 0; i_old < model_object_old.volumes.size() && i_new < model_object_new.volumes.size();) {
         const ModelVolume &mv_old = *model_object_old.volumes[i_old];
         const ModelVolume &mv_new = *model_object_new.volumes[i_new];
-        if (mv_old.type() != type) {
+        if (! type_filter(mv_old.type())) {
             ++ i_old;
             continue;
         }
-        if (mv_new.type() != type) {
+        if (! type_filter(mv_new.type())) {
             ++ i_new;
             continue;
         }
-        if (mv_old.id() != mv_new.id())
+        if (mv_old.type() != mv_new.type() || mv_old.id() != mv_new.id())
             return true;
         //FIXME test for the content of the mesh!
-
-        if (!mv_old.get_matrix().isApprox(mv_new.get_matrix()))
+        if (! mv_old.get_matrix().isApprox(mv_new.get_matrix()))
             return true;
-
         ++ i_old;
         ++ i_new;
     }
     for (; i_old < model_object_old.volumes.size(); ++ i_old) {
         const ModelVolume &mv_old = *model_object_old.volumes[i_old];
-        if (mv_old.type() == type)
+        if (type_filter(mv_old.type()))
             // ModelVolume was deleted.
             return true;
     }
     for (; i_new < model_object_new.volumes.size(); ++ i_new) {
         const ModelVolume &mv_new = *model_object_new.volumes[i_new];
-        if (mv_new.type() == type)
+        if (type_filter(mv_new.type()))
             // ModelVolume was added.
             return true;
     }
     return false;
 }
 
+bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const ModelVolumeType type)
+{
+    return model_volume_list_changed(model_object_old, model_object_new, [type](const ModelVolumeType t) { return t == type; });
+}
+
+bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const std::initializer_list<ModelVolumeType> &types)
+{
+    return model_volume_list_changed(model_object_old, model_object_new, [&types](const ModelVolumeType t) {
+        return std::find(types.begin(), types.end(), t) != types.end();
+    });
+}
+
 bool model_custom_supports_data_changed(const ModelObject& mo, const ModelObject& mo_new) {
     assert(! model_volume_list_changed(mo, mo_new, ModelVolumeType::MODEL_PART));
     assert(mo.volumes.size() == mo_new.volumes.size());
diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp
index 3cbf684a1..ea0043749 100644
--- a/src/libslic3r/Model.hpp
+++ b/src/libslic3r/Model.hpp
@@ -1075,29 +1075,30 @@ private:
 
 // Test whether the two models contain the same number of ModelObjects with the same set of IDs
 // ordered in the same order. In that case it is not necessary to kill the background processing.
-extern bool model_object_list_equal(const Model &model_old, const Model &model_new);
+bool model_object_list_equal(const Model &model_old, const Model &model_new);
 
 // Test whether the new model is just an extension of the old model (new objects were added
 // to the end of the original list. In that case it is not necessary to kill the background processing.
-extern bool model_object_list_extended(const Model &model_old, const Model &model_new);
+bool model_object_list_extended(const Model &model_old, const Model &model_new);
 
 // Test whether the new ModelObject contains a different set of volumes (or sorted in a different order)
 // than the old ModelObject.
-extern bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const ModelVolumeType type);
+bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const ModelVolumeType type);
+bool model_volume_list_changed(const ModelObject &model_object_old, const ModelObject &model_object_new, const std::initializer_list<ModelVolumeType> &types);
 
 // Test whether the now ModelObject has newer custom supports data than the old one.
 // The function assumes that volumes list is synchronized.
-extern bool model_custom_supports_data_changed(const ModelObject& mo, const ModelObject& mo_new);
+bool model_custom_supports_data_changed(const ModelObject& mo, const ModelObject& mo_new);
 
 // Test whether the now ModelObject has newer custom seam data than the old one.
 // The function assumes that volumes list is synchronized.
-extern bool model_custom_seam_data_changed(const ModelObject& mo, const ModelObject& mo_new);
+bool model_custom_seam_data_changed(const ModelObject& mo, const ModelObject& mo_new);
 
 // If the model has multi-part objects, then it is currently not supported by the SLA mode.
 // Either the model cannot be loaded, or a SLA printer has to be activated.
-extern bool model_has_multi_part_objects(const Model &model);
+bool model_has_multi_part_objects(const Model &model);
 // If the model has advanced features, then it cannot be processed in simple mode.
-extern bool model_has_advanced_features(const Model &model);
+bool model_has_advanced_features(const Model &model);
 
 #ifndef NDEBUG
 // Verify whether the IDs of Model / ModelObject / ModelVolume / ModelInstance / ModelMaterial are valid and unique.
diff --git a/src/libslic3r/Print.hpp b/src/libslic3r/Print.hpp
index ac5d2e5a9..05d417084 100644
--- a/src/libslic3r/Print.hpp
+++ b/src/libslic3r/Print.hpp
@@ -226,6 +226,7 @@ public:
     void clear() {
         all_regions.clear();
         layer_ranges.clear();
+        cached_volume_ids.clear();
     }
 
 private:
diff --git a/src/libslic3r/PrintApply.cpp b/src/libslic3r/PrintApply.cpp
index 602c835c4..3c1a03af6 100644
--- a/src/libslic3r/PrintApply.cpp
+++ b/src/libslic3r/PrintApply.cpp
@@ -295,7 +295,7 @@ public:
             std::abs(it->layer_height_range.first - range.first) > EPSILON ||
             std::abs(it->layer_height_range.second - range.second) > EPSILON )
             return nullptr; // desired range doesn't found
-        return (it == m_ranges.end()) ? nullptr : it->config;
+        return it == m_ranges.end() ? nullptr : it->config;
     }
 
     std::vector<LayerRange>::const_iterator begin() const { return m_ranges.cbegin(); }
@@ -442,7 +442,7 @@ private:
     std::multiset<PrintObjectStatus> m_db;
 };
 
-static inline bool model_volume_needs_bbox(const ModelVolume &mv)
+static inline bool model_volume_solid_or_modifier(const ModelVolume &mv)
 {
     ModelVolumeType type = mv.type();
     return type == ModelVolumeType::MODEL_PART || type == ModelVolumeType::NEGATIVE_VOLUME || type == ModelVolumeType::PARAMETER_MODIFIER;
@@ -590,7 +590,7 @@ void print_objects_regions_invalidate_keep_some_volumes(PrintObjectRegions &prin
 
 const BoundingBoxf3* find_volume_extents(const PrintObjectRegions::LayerRangeRegions &layer_range, const ModelVolume &volume)
 {
-    auto it = lower_bound_by_predicate(layer_range.volumes.begin(), layer_range.volumes.end(), [&volume](const PrintObjectRegions::VolumeExtents &v){ return v.volume_id < volume.id(); });
+    auto it = lower_bound_by_predicate(layer_range.volumes.begin(), layer_range.volumes.end(), [&volume](const PrintObjectRegions::VolumeExtents &l){ return l.volume_id < volume.id(); });
     return it != layer_range.volumes.end() && it->volume_id == volume.id() ? &it->bbox : nullptr;
 }
 
@@ -619,27 +619,28 @@ bool verify_update_print_object_regions(
 
     // Verify and / or update PrintRegions produced by ModelVolumes, layer range modifiers, modifier volumes.
     for (PrintObjectRegions::LayerRangeRegions &layer_range : print_object_regions.layer_ranges)
-        for (PrintObjectRegions::VolumeRegion &region : layer_range.volume_regions) {
-            auto it_model_volume = lower_bound_by_predicate(model_volumes.begin(), model_volumes.end(), [&region](const ModelVolume *v){ return v->id() < region.model_volume->id(); });
-            assert(it_model_volume != model_volumes.end() && (*it_model_volume)->id() == region.model_volume->id());
-            PrintRegionConfig cfg = region.parent == -1 ?
-                region_config_from_model_volume(default_region_config, layer_range.config, **it_model_volume, num_extruders) :
-                region_config_from_model_volume(layer_range.volume_regions[region.parent].region->config(), nullptr, **it_model_volume, num_extruders);
-            if (cfg != region.region->config()) {
-                // Region configuration changed.
-                if (print_region_ref_cnt(*region.region) == 0) {
-                    // Region is referenced for the first time. Just change its parameters.
-                    // Stop the background process before assigning new configuration to the regions.
-                    t_config_option_keys diff = region.region->config().diff(cfg);
-                    callback_invalidate(region.region->config(), cfg, diff);
-                    region.region->config_apply_only(cfg, diff, false);
-                } else {
-                    // Region is referenced multiple times, thus the region is being split. We need to reslice.
-                    return false;
+        for (PrintObjectRegions::VolumeRegion &region : layer_range.volume_regions)
+            if (region.model_volume->is_model_part() || region.model_volume->is_modifier()) {
+                auto it_model_volume = lower_bound_by_predicate(model_volumes.begin(), model_volumes.end(), [&region](const ModelVolume *l){ return l->id() < region.model_volume->id(); });
+                assert(it_model_volume != model_volumes.end() && (*it_model_volume)->id() == region.model_volume->id());
+                PrintRegionConfig cfg = region.parent == -1 ?
+                    region_config_from_model_volume(default_region_config, layer_range.config, **it_model_volume, num_extruders) :
+                    region_config_from_model_volume(layer_range.volume_regions[region.parent].region->config(), nullptr, **it_model_volume, num_extruders);
+                if (cfg != region.region->config()) {
+                    // Region configuration changed.
+                    if (print_region_ref_cnt(*region.region) == 0) {
+                        // Region is referenced for the first time. Just change its parameters.
+                        // Stop the background process before assigning new configuration to the regions.
+                        t_config_option_keys diff = region.region->config().diff(cfg);
+                        callback_invalidate(region.region->config(), cfg, diff);
+                        region.region->config_apply_only(cfg, diff, false);
+                    } else {
+                        // Region is referenced multiple times, thus the region is being split. We need to reslice.
+                        return false;
+                    }
                 }
+                print_region_ref_inc(*region.region);
             }
-            print_region_ref_inc(*region.region);
-        }
 
     // Verify and / or update PrintRegions produced by color painting. 
     for (const PrintObjectRegions::LayerRangeRegions &layer_range : print_object_regions.layer_ranges) {
@@ -707,9 +708,9 @@ void update_volume_bboxes(
         std::vector<PrintObjectRegions::VolumeExtents> volumes_old(std::move(layer_range.volumes));
         layer_range.volumes.reserve(model_volumes.size());
         for (const ModelVolume *model_volume : model_volumes)
-            if (model_volume_needs_bbox(*model_volume)) {
+            if (model_volume_solid_or_modifier(*model_volume)) {
                 if (std::binary_search(cached_volume_ids.begin(), cached_volume_ids.end(), model_volume->id())) {
-                    auto it = lower_bound_by_predicate(volumes_old.begin(), volumes_old.end(), [model_volume](PrintObjectRegions::VolumeExtents &v) { return v.volume_id == model_volume->id(); });
+                    auto it = lower_bound_by_predicate(volumes_old.begin(), volumes_old.end(), [model_volume](PrintObjectRegions::VolumeExtents &l) { return l.volume_id < model_volume->id(); });
                     if (it != volumes_old.end() && it->volume_id == model_volume->id())
                         layer_range.volumes.emplace_back(*it);
                 } else
@@ -737,11 +738,11 @@ void update_volume_bboxes(
             ranges.emplace_back(r);
         }
         for (const ModelVolume *model_volume : model_volumes)
-            if (model_volume_needs_bbox(*model_volume)) {
+            if (model_volume_solid_or_modifier(*model_volume)) {
                 if (std::binary_search(cached_volume_ids.begin(), cached_volume_ids.end(), model_volume->id())) {
                     for (PrintObjectRegions::LayerRangeRegions &layer_range : layer_ranges) {
                         const auto &vold = volumes_old[&layer_range - layer_ranges.data()];
-                        auto it = lower_bound_by_predicate(vold.begin(), vold.end(), [model_volume](const PrintObjectRegions::VolumeExtents &v) { return v.volume_id == model_volume->id(); });
+                        auto it = lower_bound_by_predicate(vold.begin(), vold.end(), [model_volume](const PrintObjectRegions::VolumeExtents &l) { return l.volume_id < model_volume->id(); });
                         if (it != vold.end() && it->volume_id == model_volume->id())
                             layer_range.volumes.emplace_back(*it);
                     }
@@ -776,20 +777,22 @@ static PrintObjectRegions* generate_print_object_regions(
     auto &all_regions          = out->all_regions;
     auto &layer_ranges_regions = out->layer_ranges;
 
+    all_regions.clear();
+
     bool reuse_old = print_object_regions_old && !print_object_regions_old->layer_ranges.empty();
 
     if (reuse_old) {
-#ifndef NDEBUG
+        // Reuse old bounding boxes of some ModelVolumes and their ranges.
         // Verify that the old ranges match the new ranges.
         assert(model_layer_ranges.size() == layer_ranges_regions.size());
         for (const auto &range : model_layer_ranges) {
-            const PrintObjectRegions::LayerRangeRegions &r = layer_ranges_regions[&range - &*model_layer_ranges.begin()];
+            PrintObjectRegions::LayerRangeRegions &r = layer_ranges_regions[&range - &*model_layer_ranges.begin()];
             assert(range.layer_height_range == r.layer_height_range);
-            assert(range.config == r.config);
-            assert(r.volume_regions.empty());
-            assert(r.painted_regions.empty());
+            // If model::assign_copy() is called, layer_ranges_regions is copied thus the pointers to configs are lost.
+            r.config = range.config;
+            r.volume_regions.clear();
+            r.painted_regions.clear();
         }
-#endif // NDEBUG
     } else {
         out->trafo_bboxes = trafo;
         layer_ranges_regions.reserve(model_layer_ranges.size());
@@ -816,29 +819,34 @@ static PrintObjectRegions* generate_print_object_regions(
     // Chain the regions in the order they are stored in the volumes list.
     for (int volume_id = 0; volume_id < int(model_volumes.size()); ++ volume_id) {
         const ModelVolume &volume = *model_volumes[volume_id];
-        if (volume.type() == ModelVolumeType::MODEL_PART || volume.type() == ModelVolumeType::PARAMETER_MODIFIER) {
+        if (model_volume_solid_or_modifier(volume)) {
             for (PrintObjectRegions::LayerRangeRegions &layer_range : layer_ranges_regions)
                 if (const BoundingBoxf3 *bbox = find_volume_extents(layer_range, volume); bbox) {
-                    if (volume.is_model_part())
+                    if (volume.is_model_part()) {
+                        // Add a model volume, assign an existing region or generate a new one.
                         layer_range.volume_regions.push_back({
                             &volume, -1,
                             get_create_region(region_config_from_model_volume(default_region_config, layer_range.config, volume, num_extruders)),
                             bbox
                         });
-                    else {
+                    } else if (volume.is_negative_volume()) {
+                        // Add a negative (subtractor) volume. Such volume has neither region nor parent volume assigned.
+                        layer_range.volume_regions.push_back({ &volume, -1, nullptr, bbox });
+                    } else {
                         assert(volume.is_modifier());
                         // Modifiers may be chained one over the other. Check for overlap, merge DynamicPrintConfigs.
-                        for (int parent_region_id = int(layer_range.volume_regions.size()) - 1; parent_region_id >= 0; -- parent_region_id) {
-                            const PrintObjectRegions::VolumeRegion &parent_region = layer_range.volume_regions[parent_region_id];
-                            const BoundingBoxf3                    *parent_bbox   = find_volume_extents(layer_range, *parent_region.model_volume);
-                            assert(parent_bbox != nullptr);
-                            if (parent_bbox->overlap(*bbox))
-                                layer_range.volume_regions.push_back( {
-                                    &volume, parent_region_id,
-                                    get_create_region(region_config_from_model_volume(parent_region.region->config(), nullptr, volume, num_extruders)),
-                                    bbox
-                                });
-                        }
+                        for (int parent_region_id = int(layer_range.volume_regions.size()) - 1; parent_region_id >= 0; -- parent_region_id)
+                            if (const PrintObjectRegions::VolumeRegion &parent_region = layer_range.volume_regions[parent_region_id];
+                                parent_region.model_volume->is_model_part() || parent_region.model_volume->is_modifier()) {
+                                    const BoundingBoxf3 *parent_bbox = find_volume_extents(layer_range, *parent_region.model_volume);
+                                    assert(parent_bbox != nullptr);
+                                    if (parent_bbox->overlap(*bbox))
+                                        layer_range.volume_regions.push_back({
+                                            &volume, parent_region_id,
+                                            get_create_region(region_config_from_model_volume(parent_region.region->config(), nullptr, volume, num_extruders)),
+                                            bbox
+                                        });
+                            }
                     }
                 }
             }
@@ -847,13 +855,14 @@ static PrintObjectRegions* generate_print_object_regions(
     // Finally add painting regions.
     for (PrintObjectRegions::LayerRangeRegions &layer_range : layer_ranges_regions)
         for (unsigned int painted_extruder_id : painting_extruders)
-            for (int parent_region_id = 0; parent_region_id < int(layer_range.volume_regions.size()); ++ parent_region_id) {
-                const PrintObjectRegions::VolumeRegion &parent_region = layer_range.volume_regions[parent_region_id];
-                PrintRegionConfig cfg = parent_region.region->config();
-                cfg.perimeter_extruder.value = painted_extruder_id;
-                cfg.infill_extruder.value    = painted_extruder_id;
-                layer_range.painted_regions.push_back({ painted_extruder_id, parent_region_id, get_create_region(std::move(cfg))});
-            }
+            for (int parent_region_id = 0; parent_region_id < int(layer_range.volume_regions.size()); ++ parent_region_id)
+                if (const PrintObjectRegions::VolumeRegion &parent_region = layer_range.volume_regions[parent_region_id];
+                    parent_region.model_volume->is_model_part() || parent_region.model_volume->is_modifier()) {
+                    PrintRegionConfig cfg = parent_region.region->config();
+                    cfg.perimeter_extruder.value = painted_extruder_id;
+                    cfg.infill_extruder.value    = painted_extruder_id;
+                    layer_range.painted_regions.push_back({ painted_extruder_id, parent_region_id, get_create_region(std::move(cfg))});
+                }
 
     return out.release();
 }
@@ -1023,6 +1032,7 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
     PrintObjectStatusDB print_object_status_db(m_objects);
 
     // 3) Synchronize ModelObjects & PrintObjects.
+    const std::initializer_list<ModelVolumeType> solid_or_modifier_types { ModelVolumeType::MODEL_PART, ModelVolumeType::NEGATIVE_VOLUME, ModelVolumeType::PARAMETER_MODIFIER };
     for (size_t idx_model_object = 0; idx_model_object < model.objects.size(); ++ idx_model_object) {
         ModelObject       &model_object        = *m_model.objects[idx_model_object];
         ModelObjectStatus &model_object_status = const_cast<ModelObjectStatus&>(model_object_status_db.reuse(model_object));
@@ -1034,27 +1044,24 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
         assert(model_object_status.status == ModelObjectStatus::Old || model_object_status.status == ModelObjectStatus::Moved);
         // Check whether a model part volume was added or removed, their transformations or order changed.
         // Only volume IDs, volume types, transformation matrices and their order are checked, configuration and other parameters are NOT checked.
-        bool model_parts_differ         = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::MODEL_PART);
-        bool modifiers_differ           = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::PARAMETER_MODIFIER);
+        bool solid_or_modifier_differ   = model_volume_list_changed(model_object, model_object_new, solid_or_modifier_types);
         bool supports_differ            = model_volume_list_changed(model_object, model_object_new, ModelVolumeType::SUPPORT_BLOCKER) ||
                                           model_volume_list_changed(model_object, model_object_new, ModelVolumeType::SUPPORT_ENFORCER);
         bool layer_height_ranges_differ = ! layer_height_ranges_equal(model_object.layer_config_ranges, model_object_new.layer_config_ranges, model_object_new.layer_height_profile.empty());
+        bool model_origin_translation_differ = model_object.origin_translation != model_object_new.origin_translation;
         auto print_objects_range        = print_object_status_db.get_range(model_object);
         assert(print_objects_range.begin() != print_objects_range.end());
         // All PrintObjects in print_objects_range shall point to the same prints_objects_regions
         model_object_status.print_object_regions = print_objects_range.begin()->print_object->m_shared_regions;
         model_object_status.print_object_regions->ref_cnt_inc();
-        if (model_parts_differ || modifiers_differ || 
-            model_object.origin_translation != model_object_new.origin_translation   ||
-            ! model_object.layer_height_profile.timestamp_matches(model_object_new.layer_height_profile) ||
-            layer_height_ranges_differ) {
+        if (solid_or_modifier_differ || model_origin_translation_differ || layer_height_ranges_differ ||
+            ! model_object.layer_height_profile.timestamp_matches(model_object_new.layer_height_profile)) {
             // The very first step (the slicing step) is invalidated. One may freely remove all associated PrintObjects.
-            model_object_status.print_object_regions_status =
-                model_object.origin_translation == model_object_new.origin_translation && ! layer_height_ranges_differ ?
-                    // Drop print_objects_regions.
-                    ModelObjectStatus::PrintObjectRegionsStatus::Invalid :
-                    // Reuse bounding boxes of print_objects_regions for ModelVolumes with unmodified transformation.
-                    ModelObjectStatus::PrintObjectRegionsStatus::PartiallyValid;
+            model_object_status.print_object_regions_status = model_origin_translation_differ || layer_height_ranges_differ ?
+                // Drop print_objects_regions.
+                ModelObjectStatus::PrintObjectRegionsStatus::Invalid :
+                // Reuse bounding boxes of print_objects_regions for ModelVolumes with unmodified transformation.
+                ModelObjectStatus::PrintObjectRegionsStatus::PartiallyValid;
             for (const PrintObjectStatus &print_object_status : print_objects_range) {
                 update_apply_status(print_object_status.print_object->invalidate_all_steps());
                 const_cast<PrintObjectStatus&>(print_object_status).status = PrintObjectStatus::Deleted;
@@ -1085,7 +1092,7 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
                 update_apply_status(this->invalidate_step(psGCodeExport));
             }
         }
-        if (! model_parts_differ && ! modifiers_differ) {
+        if (! solid_or_modifier_differ) {
             // Synchronize Object's config.
             bool object_config_changed = ! model_object.config.timestamp_matches(model_object_new.config);
 			if (object_config_changed)
diff --git a/src/libslic3r/PrintObjectSlice.cpp b/src/libslic3r/PrintObjectSlice.cpp
index 5960853d2..a64cedd03 100644
--- a/src/libslic3r/PrintObjectSlice.cpp
+++ b/src/libslic3r/PrintObjectSlice.cpp
@@ -306,21 +306,20 @@ static std::vector<std::vector<ExPolygons>> slices_to_regions(
                 float z              = zs_complex[range.begin()].second;
                 auto  it_layer_range = lower_bound_by_predicate(print_object_regions.layer_ranges.begin(), print_object_regions.layer_ranges.end(), 
                     [z](const PrintObjectRegions::LayerRangeRegions &lr){ return lr.layer_height_range.second < z; });
-                assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first >= z && z <= it_layer_range->layer_height_range.second);
+                assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first <= z && z <= it_layer_range->layer_height_range.second);
                 if (z == it_layer_range->layer_height_range.second)
                     if (auto it_next = it_layer_range; ++ it_next != print_object_regions.layer_ranges.end() && it_next->layer_height_range.first == z)
                         it_layer_range = it_next;
-                assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first >= z && z < it_layer_range->layer_height_range.second);
+                assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first <= z && z <= it_layer_range->layer_height_range.second);
                 // Per volume_regions slices at this Z height.
                 struct RegionSlice { 
                     ExPolygons  expolygons;
                     // Identifier of this region in PrintObjectRegions::all_regions
                     int         region_id;
                     ObjectID    volume_id;
-                    bool empty() const { return region_id < 0 || expolygons.empty(); }
                     bool operator<(const RegionSlice &rhs) const {
-                        bool this_empty = this->empty();
-                        bool rhs_empty  = rhs.empty();
+                        bool this_empty = this->region_id < 0 || this->expolygons.empty();
+                        bool rhs_empty  = rhs.region_id < 0 || rhs.expolygons.empty();
                         // Sort the empty items to the end of the list.
                         // Sort by region_id & volume_id lexicographically.
                         return ! this_empty && (rhs_empty || (this->region_id < rhs.region_id || (this->region_id == rhs.region_id && volume_id < volume_id)));
@@ -331,7 +330,7 @@ static std::vector<std::vector<ExPolygons>> slices_to_regions(
                     auto [z_idx, z] = zs_complex[zs_complex_idx];
                     for (; it_layer_range->layer_height_range.second <= z; ++ it_layer_range)
                         assert(it_layer_range != print_object_regions.layer_ranges.end());
-                    assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first >= z && z < it_layer_range->layer_height_range.second);
+                    assert(it_layer_range != print_object_regions.layer_ranges.end() && it_layer_range->layer_height_range.first <= z && z < it_layer_range->layer_height_range.second);
                     const PrintObjectRegions::LayerRangeRegions &layer_range = *it_layer_range;
                     {
                         std::vector<VolumeSlices*> &layer_range_regions_to_slices = layer_ranges_regions_to_slices[it_layer_range - print_object_regions.layer_ranges.begin()];
@@ -344,26 +343,27 @@ static std::vector<std::vector<ExPolygons>> slices_to_regions(
                         }
                     }
                     for (int idx_region = 0; idx_region < int(layer_range.volume_regions.size()); ++ idx_region)
-                        if (! temp_slices[idx_region].empty()) {
+                        if (! temp_slices[idx_region].expolygons.empty()) {
                             const PrintObjectRegions::VolumeRegion &region = layer_range.volume_regions[idx_region];
                             if (region.model_volume->is_modifier()) {
                                 assert(region.parent > -1);
                                 bool next_region_same_modifier = idx_region + 1 < int(temp_slices.size()) && layer_range.volume_regions[idx_region + 1].model_volume == region.model_volume;
-                                if (next_region_same_modifier)
-                                    temp_slices[idx_region + 1] = std::move(temp_slices[idx_region]);
                                 RegionSlice &parent_slice = temp_slices[region.parent];
                                 RegionSlice &this_slice   = temp_slices[idx_region];
-                                if (parent_slice.empty())
-                                    this_slice.expolygons.clear();
-                                else {
-                                    RegionSlice &source_slice = temp_slices[idx_region + int(next_region_same_modifier)];
-                                    this_slice  .expolygons = intersection_ex(parent_slice.expolygons, source_slice.expolygons);
-                                    parent_slice.expolygons = diff_ex        (parent_slice.expolygons, source_slice.expolygons);
+                                ExPolygons   source       = std::move(this_slice.expolygons);
+                                if (parent_slice.expolygons.empty()) {
+                                    this_slice  .expolygons.clear();
+                                } else {
+                                    this_slice  .expolygons = intersection_ex(parent_slice.expolygons, source);
+                                    parent_slice.expolygons = diff_ex        (parent_slice.expolygons, source);
                                 }
+                                if (next_region_same_modifier)
+                                    // To be used in the following iteration.
+                                    temp_slices[idx_region + 1].expolygons = std::move(source);
                             } else if ((region.model_volume->is_model_part() && clip_multipart_objects) || region.model_volume->is_negative_volume()) {
                                 // Clip every non-zero region preceding it.
                                 for (int idx_region2 = 0; idx_region2 < idx_region; ++ idx_region2)
-                                    if (! temp_slices[idx_region2].empty()) {
+                                    if (! temp_slices[idx_region2].expolygons.empty()) {
                                         if (const PrintObjectRegions::VolumeRegion &region2 = layer_range.volume_regions[idx_region2];
                                             ! region2.model_volume->is_negative_volume() && overlap_in_xy(*region.bbox, *region2.bbox))
                                             temp_slices[idx_region2].expolygons = diff_ex(temp_slices[idx_region2].expolygons, temp_slices[idx_region].expolygons);
@@ -373,7 +373,7 @@ static std::vector<std::vector<ExPolygons>> slices_to_regions(
                     // Sort by region_id, push empty slices to the end.
                     std::sort(temp_slices.begin(), temp_slices.end());
                     // Remove the empty slices.
-                    temp_slices.erase(std::find_if(temp_slices.begin(), temp_slices.end(), [](const auto &slice) { return slice.empty(); }), temp_slices.end());
+                    temp_slices.erase(std::find_if(temp_slices.begin(), temp_slices.end(), [](const auto &slice) { return slice.region_id == -1 || slice.expolygons.empty(); }), temp_slices.end());
                     // Merge slices and store them to the output.
                     for (int i = 0; i < int(temp_slices.size());) {
                         // Find a range of temp_slices with the same region_id.
diff --git a/src/slic3r/GUI/GUI_Factories.cpp b/src/slic3r/GUI/GUI_Factories.cpp
index a67589dd7..f0fda5fa8 100644
--- a/src/slic3r/GUI/GUI_Factories.cpp
+++ b/src/slic3r/GUI/GUI_Factories.cpp
@@ -151,7 +151,7 @@ wxBitmap SettingsFactory::get_category_bitmap(const std::string& category_name)
 //-------------------------------------
 
 // Note: id accords to type of the sub-object (adding volume), so sequence of the menu items is important
-std::vector<std::pair<std::string, std::string>> MenuFactory::ADD_VOLUME_MENU_ITEMS = {
+const std::vector<std::pair<std::string, std::string>> MenuFactory::ADD_VOLUME_MENU_ITEMS {
 //       menu_item Name              menu_item bitmap name
         {L("Add part"),              "add_part" },           // ~ModelVolumeType::MODEL_PART
         {L("Add negative volume"),   "add_negative" },       // ~ModelVolumeType::NEGATIVE_VOLUME
diff --git a/src/slic3r/GUI/GUI_Factories.hpp b/src/slic3r/GUI/GUI_Factories.hpp
index ffa17933c..edf1cb789 100644
--- a/src/slic3r/GUI/GUI_Factories.hpp
+++ b/src/slic3r/GUI/GUI_Factories.hpp
@@ -33,7 +33,7 @@ struct SettingsFactory
 class MenuFactory
 {
 public:
-    static std::vector<std::pair<std::string, std::string>> ADD_VOLUME_MENU_ITEMS;
+    static const std::vector<std::pair<std::string, std::string>> ADD_VOLUME_MENU_ITEMS;
     static std::vector<wxBitmap>    get_volume_bitmaps();
 
     MenuFactory();