mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-09-12 11:37:42 +00:00
Finish Fixes from Copilot Review (#39)
* fix: restore BuildVolume bounds when toggling belt mode set_belt_printer() mutated m_bboxf when enabling but never restored the original extents on disable or when switching infinite_y true->false, leaving stale max.y/max.z values that broke collision and object_state checks. Recompute m_bboxf from m_bed_shape + m_max_print_height at the top of each call, then apply belt-specific adjustments on top. Addresses Copilot review comment on PR #12998 (BuildVolume.cpp:196). * chore: drop [BELT-DEBUG] to_machine_coords log to trace Was emitting at warning level once per 0.2mm Z bucket during every belt print export, polluting default user logs. Trace level matches the rest of the belt diagnostics and is silent in production. Addresses Copilot review comment on PR #12998 (BeltGCodeWriter.cpp:86). * chore: drop [BELTRACE] make_perimeters/support logs to trace Eight warning-level traces around make_perimeters and generate_support_material were emitting on every call/exit during normal slicing, cluttering default logs. They're concurrency-debug breadcrumbs not user-facing diagnostics, so drop them to trace. Addresses Copilot review comment on PR #12998 (PrintObject.cpp:438). * perf: gate BeltSliceStrategy diagnostic bbox tracking behind compile flag apply_to_trafo() walked every model vertex twice (once for min_z, once for per-volume mesh/slicer bboxes) and emitted seven trace logs per call. The bboxes and logs are diagnostic only; min_z is the load-bearing output. Wrap the bbox accumulation, logging, and supporting headers in SLIC3R_BELT_DIAGNOSTIC_LOG so production builds do the bare min_z scan. Addresses Copilot review comment on PR #12998 (BeltSliceStrategy.cpp:95). * fix: apply part_cooling_fan_min_pwm to first-layer plane fan crossings apply_first_layer_plane_fan_eval emitted band-crossing M106 commands through GCodeWriter::set_fan() without the per-printer PWM floor that every other set_fan call in CoolingBuffer applies. On printers with a non-zero part_cooling_fan_min_pwm, fans could fail to spin up at low requested speeds near the belt surface. Addresses Copilot review comment on PR #12998 (CoolingBuffer.cpp:1227).
This commit is contained in:
@@ -76,7 +76,7 @@ Vec3d BeltGCodeWriter::to_machine_coords(const Vec3d &pos) const
|
|||||||
int z_bucket = static_cast<int>(std::floor(pos.z() * 5.0)); // every 0.2mm
|
int z_bucket = static_cast<int>(std::floor(pos.z() * 5.0)); // every 0.2mm
|
||||||
if (z_bucket != s_last_logged_z) {
|
if (z_bucket != s_last_logged_z) {
|
||||||
s_last_logged_z = z_bucket;
|
s_last_logged_z = z_bucket;
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELT-DEBUG] to_machine_coords"
|
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] to_machine_coords"
|
||||||
<< " slicer_in=(" << pos.x() << "," << pos.y() << "," << pos.z() << ")"
|
<< " slicer_in=(" << pos.x() << "," << pos.y() << "," << pos.z() << ")"
|
||||||
<< " after_back=(" << after_back.x() << "," << after_back.y() << "," << after_back.z() << ")"
|
<< " after_back=(" << after_back.x() << "," << after_back.y() << "," << after_back.z() << ")"
|
||||||
<< " after_remap=(" << after_remap.x() << "," << after_remap.y() << "," << after_remap.z() << ")"
|
<< " after_remap=(" << after_remap.x() << "," << after_remap.y() << "," << after_remap.z() << ")"
|
||||||
|
|||||||
@@ -2,9 +2,11 @@
|
|||||||
#include "Model.hpp"
|
#include "Model.hpp"
|
||||||
|
|
||||||
#include <boost/log/trivial.hpp>
|
#include <boost/log/trivial.hpp>
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
#include <iomanip>
|
#include <iomanip>
|
||||||
#include <sstream>
|
#include <sstream>
|
||||||
#include <thread>
|
#include <thread>
|
||||||
|
#endif
|
||||||
|
|
||||||
namespace Slic3r {
|
namespace Slic3r {
|
||||||
|
|
||||||
@@ -40,7 +42,8 @@ void BeltSliceStrategy::apply_to_trafo(Transform3d &trafo,
|
|||||||
// coordinates rather than object-space coordinates, so volumes translated
|
// coordinates rather than object-space coordinates, so volumes translated
|
||||||
// along the slicer's Z axis are silently excluded from the bound check.
|
// along the slicer's Z axis are silently excluded from the bound check.
|
||||||
if (has_remap || m_has_rotation) {
|
if (has_remap || m_has_rotation) {
|
||||||
// [BELT-DEBUG] Capture the incoming trafo for diagnostic logging.
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
|
// Capture the incoming trafo for diagnostic logging.
|
||||||
// This is the slicer-frame transform AFTER belt_xform but BEFORE z_shift.
|
// This is the slicer-frame transform AFTER belt_xform but BEFORE z_shift.
|
||||||
const Transform3d trafo_pre_shift = trafo;
|
const Transform3d trafo_pre_shift = trafo;
|
||||||
auto log_mat = [](const Matrix3d &m) {
|
auto log_mat = [](const Matrix3d &m) {
|
||||||
@@ -63,29 +66,41 @@ void BeltSliceStrategy::apply_to_trafo(Transform3d &trafo,
|
|||||||
<< " trafo.linear=" << log_mat(trafo_pre_shift.linear())
|
<< " trafo.linear=" << log_mat(trafo_pre_shift.linear())
|
||||||
<< " trafo.translation=" << log_vec3(trafo_pre_shift.translation())
|
<< " trafo.translation=" << log_vec3(trafo_pre_shift.translation())
|
||||||
<< " volumes=" << model_volumes.size();
|
<< " volumes=" << model_volumes.size();
|
||||||
|
#endif
|
||||||
|
|
||||||
double min_z = std::numeric_limits<double>::max();
|
double min_z = std::numeric_limits<double>::max();
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
int vol_idx = 0;
|
int vol_idx = 0;
|
||||||
|
#endif
|
||||||
for (const ModelVolume *mv : model_volumes) {
|
for (const ModelVolume *mv : model_volumes) {
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
if (!mv->is_model_part()) { ++vol_idx; continue; }
|
if (!mv->is_model_part()) { ++vol_idx; continue; }
|
||||||
|
#else
|
||||||
|
if (!mv->is_model_part()) continue;
|
||||||
|
#endif
|
||||||
Transform3d vol_trafo = trafo * mv->get_matrix();
|
Transform3d vol_trafo = trafo * mv->get_matrix();
|
||||||
// [BELT-DEBUG] Per-volume bbox in mesh-frame and post-trafo slicer-frame.
|
|
||||||
const auto &its = mv->mesh().its;
|
const auto &its = mv->mesh().its;
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
|
// Per-volume bbox in mesh-frame and post-trafo slicer-frame.
|
||||||
Vec3d mesh_min(std::numeric_limits<double>::max(), std::numeric_limits<double>::max(), std::numeric_limits<double>::max());
|
Vec3d mesh_min(std::numeric_limits<double>::max(), std::numeric_limits<double>::max(), std::numeric_limits<double>::max());
|
||||||
Vec3d mesh_max(std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest());
|
Vec3d mesh_max(std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest());
|
||||||
Vec3d slicer_min(std::numeric_limits<double>::max(), std::numeric_limits<double>::max(), std::numeric_limits<double>::max());
|
Vec3d slicer_min(std::numeric_limits<double>::max(), std::numeric_limits<double>::max(), std::numeric_limits<double>::max());
|
||||||
Vec3d slicer_max(std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest());
|
Vec3d slicer_max(std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest());
|
||||||
double vol_min_z = std::numeric_limits<double>::max();
|
double vol_min_z = std::numeric_limits<double>::max();
|
||||||
|
#endif
|
||||||
for (const stl_vertex &v : its.vertices) {
|
for (const stl_vertex &v : its.vertices) {
|
||||||
Vec3d vm = v.cast<double>();
|
Vec3d vm = v.cast<double>();
|
||||||
|
Vec3d pt = vol_trafo * vm;
|
||||||
|
min_z = std::min(min_z, pt.z());
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
mesh_min = mesh_min.cwiseMin(vm);
|
mesh_min = mesh_min.cwiseMin(vm);
|
||||||
mesh_max = mesh_max.cwiseMax(vm);
|
mesh_max = mesh_max.cwiseMax(vm);
|
||||||
Vec3d pt = vol_trafo * vm;
|
|
||||||
slicer_min = slicer_min.cwiseMin(pt);
|
slicer_min = slicer_min.cwiseMin(pt);
|
||||||
slicer_max = slicer_max.cwiseMax(pt);
|
slicer_max = slicer_max.cwiseMax(pt);
|
||||||
vol_min_z = std::min(vol_min_z, pt.z());
|
vol_min_z = std::min(vol_min_z, pt.z());
|
||||||
min_z = std::min(min_z, pt.z());
|
#endif
|
||||||
}
|
}
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] vol[" << vol_idx
|
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] vol[" << vol_idx
|
||||||
<< "] id=" << mv->id().id << " name='" << mv->name << "'"
|
<< "] id=" << mv->id().id << " name='" << mv->name << "'"
|
||||||
<< " mesh_bbox_min=" << log_vec3(mesh_min) << " mesh_bbox_max=" << log_vec3(mesh_max)
|
<< " mesh_bbox_min=" << log_vec3(mesh_min) << " mesh_bbox_max=" << log_vec3(mesh_max)
|
||||||
@@ -93,10 +108,13 @@ void BeltSliceStrategy::apply_to_trafo(Transform3d &trafo,
|
|||||||
<< " slicer_bbox_min=" << log_vec3(slicer_min) << " slicer_bbox_max=" << log_vec3(slicer_max)
|
<< " slicer_bbox_min=" << log_vec3(slicer_min) << " slicer_bbox_max=" << log_vec3(slicer_max)
|
||||||
<< " vol_min_z=" << vol_min_z;
|
<< " vol_min_z=" << vol_min_z;
|
||||||
++vol_idx;
|
++vol_idx;
|
||||||
|
#endif
|
||||||
}
|
}
|
||||||
double belt_z_shift_val = (min_z < 0. && min_z != std::numeric_limits<double>::max()) ? -min_z : 0.;
|
double belt_z_shift_val = (min_z < 0. && min_z != std::numeric_limits<double>::max()) ? -min_z : 0.;
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] combined min_z=" << min_z
|
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] combined min_z=" << min_z
|
||||||
<< " z_shift_val=" << belt_z_shift_val;
|
<< " z_shift_val=" << belt_z_shift_val;
|
||||||
|
#endif
|
||||||
if (belt_z_shift_val > 0.) {
|
if (belt_z_shift_val > 0.) {
|
||||||
Transform3d z_shift = Transform3d::Identity();
|
Transform3d z_shift = Transform3d::Identity();
|
||||||
z_shift.matrix()(2, 3) = belt_z_shift_val;
|
z_shift.matrix()(2, 3) = belt_z_shift_val;
|
||||||
@@ -104,13 +122,17 @@ void BeltSliceStrategy::apply_to_trafo(Transform3d &trafo,
|
|||||||
}
|
}
|
||||||
if (out_belt_min_z) {
|
if (out_belt_min_z) {
|
||||||
double new_val = (min_z != std::numeric_limits<double>::max()) ? min_z : 0.;
|
double new_val = (min_z != std::numeric_limits<double>::max()) ? min_z : 0.;
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] write m_belt_min_z tid=" << std::this_thread::get_id()
|
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] write m_belt_min_z tid=" << std::this_thread::get_id()
|
||||||
<< " target=" << out_belt_min_z << " old=" << *out_belt_min_z << " new=" << new_val;
|
<< " target=" << out_belt_min_z << " old=" << *out_belt_min_z << " new=" << new_val;
|
||||||
|
#endif
|
||||||
*out_belt_min_z = new_val;
|
*out_belt_min_z = new_val;
|
||||||
}
|
}
|
||||||
|
#ifdef SLIC3R_BELT_DIAGNOSTIC_LOG
|
||||||
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] apply_to_trafo exit"
|
BOOST_LOG_TRIVIAL(trace) << "[BELT-DEBUG] apply_to_trafo exit"
|
||||||
<< " final_trafo.linear=" << log_mat(trafo.linear())
|
<< " final_trafo.linear=" << log_mat(trafo.linear())
|
||||||
<< " final_trafo.translation=" << log_vec3(trafo.translation());
|
<< " final_trafo.translation=" << log_vec3(trafo.translation());
|
||||||
|
#endif
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -182,6 +182,12 @@ void BuildVolume::set_belt_printer(bool enabled, double angle_deg, bool infinite
|
|||||||
m_belt_angle = angle_deg;
|
m_belt_angle = angle_deg;
|
||||||
m_belt_infinite_y = infinite_y;
|
m_belt_infinite_y = infinite_y;
|
||||||
|
|
||||||
|
// Restart from the unmodified bbox each call. Without this, toggling
|
||||||
|
// belt mode off (or switching infinite_y true→false) would leave the
|
||||||
|
// extents inflated and break collision / object_state checks.
|
||||||
|
BoundingBoxf bboxf = get_extents(m_bed_shape);
|
||||||
|
m_bboxf = BoundingBoxf3{ to_3d(bboxf.min, 0.), to_3d(bboxf.max, m_max_print_height) };
|
||||||
|
|
||||||
if (enabled) {
|
if (enabled) {
|
||||||
if (infinite_y) {
|
if (infinite_y) {
|
||||||
// Extend the Y bound to a very large value for infinite belt.
|
// Extend the Y bound to a very large value for infinite belt.
|
||||||
|
|||||||
@@ -1091,6 +1091,10 @@ std::string CoolingBuffer::apply_first_layer_plane_fan_eval(
|
|||||||
std::string out;
|
std::string out;
|
||||||
out.reserve(gcode.size() + 256);
|
out.reserve(gcode.size() + 256);
|
||||||
|
|
||||||
|
// Match the PWM floor applied at every other set_fan call in this file so
|
||||||
|
// band-crossing M106 emissions start the fan reliably at low speeds.
|
||||||
|
const unsigned int part_cooling_fan_min_pwm = static_cast<unsigned int>(std::max(0, m_config.part_cooling_fan_min_pwm.value));
|
||||||
|
|
||||||
// Track position in slicing-frame mm. Seed from m_current_pos which the
|
// Track position in slicing-frame mm. Seed from m_current_pos which the
|
||||||
// CoolingBuffer keeps up-to-date across layers.
|
// CoolingBuffer keeps up-to-date across layers.
|
||||||
Vec3d cur_pos_mm(m_current_pos[0], m_current_pos[1], m_current_pos[2]);
|
Vec3d cur_pos_mm(m_current_pos[0], m_current_pos[1], m_current_pos[2]);
|
||||||
@@ -1221,7 +1225,7 @@ std::string CoolingBuffer::apply_first_layer_plane_fan_eval(
|
|||||||
target_fan = pre_band_main_fan;
|
target_fan = pre_band_main_fan;
|
||||||
}
|
}
|
||||||
if (target_fan != current_main_fan) {
|
if (target_fan != current_main_fan) {
|
||||||
out += GCodeWriter::set_fan(m_config.gcode_flavor, target_fan);
|
out += GCodeWriter::set_fan(m_config.gcode_flavor, target_fan, part_cooling_fan_min_pwm);
|
||||||
current_main_fan = target_fan;
|
current_main_fan = target_fan;
|
||||||
m_fan_speed = target_fan;
|
m_fan_speed = target_fan;
|
||||||
m_current_fan_speed = target_fan;
|
m_current_fan_speed = target_fan;
|
||||||
|
|||||||
@@ -427,15 +427,15 @@ std::vector<std::set<int>> PrintObject::detect_extruder_geometric_unprintables()
|
|||||||
// 3) Generates perimeters, gap fills and fill regions (fill regions of type stInternal).
|
// 3) Generates perimeters, gap fills and fill regions (fill regions of type stInternal).
|
||||||
void PrintObject::make_perimeters()
|
void PrintObject::make_perimeters()
|
||||||
{
|
{
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] make_perimeters request tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] make_perimeters request tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
// prerequisites
|
// prerequisites
|
||||||
this->slice();
|
this->slice();
|
||||||
|
|
||||||
if (! this->set_started(posPerimeters)) {
|
if (! this->set_started(posPerimeters)) {
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] make_perimeters SKIP tid=" << std::this_thread::get_id() << " obj=" << this << " (already started/done)";
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] make_perimeters SKIP tid=" << std::this_thread::get_id() << " obj=" << this << " (already started/done)";
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] make_perimeters ENTER tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] make_perimeters ENTER tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
|
|
||||||
m_print->set_status(15, L("Generating walls"));
|
m_print->set_status(15, L("Generating walls"));
|
||||||
BOOST_LOG_TRIVIAL(info) << "Generating walls..." << log_memory_info();
|
BOOST_LOG_TRIVIAL(info) << "Generating walls..." << log_memory_info();
|
||||||
@@ -533,7 +533,7 @@ void PrintObject::make_perimeters()
|
|||||||
m_print->throw_if_canceled();
|
m_print->throw_if_canceled();
|
||||||
BOOST_LOG_TRIVIAL(debug) << "Generating perimeters in parallel - end";
|
BOOST_LOG_TRIVIAL(debug) << "Generating perimeters in parallel - end";
|
||||||
|
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] make_perimeters EXIT tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] make_perimeters EXIT tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
this->set_done(posPerimeters);
|
this->set_done(posPerimeters);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -829,9 +829,9 @@ void PrintObject::detect_overhangs_for_lift()
|
|||||||
|
|
||||||
void PrintObject::generate_support_material()
|
void PrintObject::generate_support_material()
|
||||||
{
|
{
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] generate_support_material request tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] generate_support_material request tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
if (this->set_started(posSupportMaterial)) {
|
if (this->set_started(posSupportMaterial)) {
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] generate_support_material ENTER tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] generate_support_material ENTER tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
this->clear_support_layers();
|
this->clear_support_layers();
|
||||||
|
|
||||||
if(!has_support() && !m_print->get_no_check_flag()) {
|
if(!has_support() && !m_print->get_no_check_flag()) {
|
||||||
@@ -872,10 +872,10 @@ void PrintObject::generate_support_material()
|
|||||||
this->_generate_support_material();
|
this->_generate_support_material();
|
||||||
m_print->throw_if_canceled();
|
m_print->throw_if_canceled();
|
||||||
}
|
}
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] generate_support_material EXIT tid=" << std::this_thread::get_id() << " obj=" << this;
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] generate_support_material EXIT tid=" << std::this_thread::get_id() << " obj=" << this;
|
||||||
this->set_done(posSupportMaterial);
|
this->set_done(posSupportMaterial);
|
||||||
} else {
|
} else {
|
||||||
BOOST_LOG_TRIVIAL(warning) << "[BELTRACE] generate_support_material SKIP tid=" << std::this_thread::get_id() << " obj=" << this << " (already started/done)";
|
BOOST_LOG_TRIVIAL(trace) << "[BELTRACE] generate_support_material SKIP tid=" << std::this_thread::get_id() << " obj=" << this << " (already started/done)";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user