diff --git a/sandboxes/orca_inset_plugin_any.py b/sandboxes/orca_inset_plugin_any.py index ebcbc8563a..23daf8271d 100644 --- a/sandboxes/orca_inset_plugin_any.py +++ b/sandboxes/orca_inset_plugin_any.py @@ -19,8 +19,8 @@ geometry. At Step.posSlice the split slice loop runs make_perimeters() right aft the hook, so the change cascades into perimeters, infill and the final G-code -- the toolpath preview shrinks. -Unlike the old axis-aligned demo, ExPolygon.offset() is a correct inward offset -for any contour (it is Clipper under the hood), and it naturally handles holes. +ExPolygon.offset() is a correct inward offset for any contour (it is Clipper +under the hood), and it naturally handles holes. A surface may split into several islands or vanish when shrunk; both are handled. No numpy required: the whole edit is expressed with the host geometry classes. diff --git a/src/libslic3r/Print.hpp b/src/libslic3r/Print.hpp index 9830878a1c..9481498cc6 100644 --- a/src/libslic3r/Print.hpp +++ b/src/libslic3r/Print.hpp @@ -103,7 +103,7 @@ enum class SlicingPipelineStepPlugin { posSlice, posPerimeters, posEstimateCurledExtrusions, posPrepareInfill, posInfill, posIroning, posContouring, posSupportMaterial, posDetectOverhangsForLift, posSimplifyPath, psWipeTower, psSkirtBrim, // Fires from the GUI G-code export/post-process seam (PostProcessor.cpp), NOT from Print::process(). - // At this step the plugin edits the exported G-code file in place; see the binding for the full contract. + // At this step the plugin edits the exported G-code file in place; see SlicingPipelinePluginCapability for the full contract. psGCodePostProcess }; diff --git a/src/slic3r/plugin/PluginBindingUtils.hpp b/src/slic3r/plugin/PluginBindingUtils.hpp index 3f17b5668a..52050ed561 100644 --- a/src/slic3r/plugin/PluginBindingUtils.hpp +++ b/src/slic3r/plugin/PluginBindingUtils.hpp @@ -39,8 +39,8 @@ pybind11::array make_readonly_rows(pybind11::handle base, const T* data, pybind1 namespace py = pybind11; if (rows == 0 || data == nullptr) { py::array_t empty(std::vector{ 0, (py::ssize_t) N }); - // Keep behavior-preserving: the pre-refactor helper returned read-only - // arrays on every path, so mark the fresh empty array read-only too. + // Mark the fresh empty array read-only so every return path of this + // helper yields a read-only view. empty.attr("setflags")(py::arg("write") = false); return std::move(empty); } diff --git a/src/slic3r/plugin/PluginHostSlicing.cpp b/src/slic3r/plugin/PluginHostSlicing.cpp index 01739420f0..1db8120eb7 100644 --- a/src/slic3r/plugin/PluginHostSlicing.cpp +++ b/src/slic3r/plugin/PluginHostSlicing.cpp @@ -109,7 +109,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) // ------------------------------------------------------------------ // Slicing print-graph data model — raw bindings of the classes the C++ // pipeline itself uses, same nodelete/reference style as the Model and - // Preset graphs above. + // Preset graphs in PluginHostApi.cpp. // // LIFETIME (C++ semantics, the one rule of this API): every object handed // out below is a non-owning reference into the live slicing graph owned by @@ -306,8 +306,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) .def("set_type", [](SurfaceCollection& c, SurfaceType t) { c.set_type(t); }, py::arg("surface_type")) .def("set", [](SurfaceCollection& c, const std::vector& src, SurfaceType t) { c.set(src, t); }, py::arg("expolygons"), py::arg("surface_type"), - "Replace all surfaces from a list of ExPolygon, all tagged `surface_type`. " - "This is the faithful replacement for the retired set_slices().") + "Replace all surfaces from a list of ExPolygon, all tagged `surface_type`.") .def("set", [](SurfaceCollection& c, const std::vector& src) { c.set(src); }, py::arg("surfaces"), "Replace all surfaces from a list of Surface (types preserved per surface).") .def("append", [](SurfaceCollection& c, const std::vector& src, SurfaceType t) { c.append(src, t); }, @@ -315,9 +314,8 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) .def("filter_by_type", [](py::object self, SurfaceType t) { SurfaceCollection& c = self.cast(); py::list out; - // SurfacesPtr (SurfaceCollection::filter_by_type's return type) is - // std::vector (see Surface.hpp); the brief's note describing it - // as std::vector does not match the header, so this iterates by const + // SurfaceCollection::filter_by_type returns SurfacesPtr, which is + // std::vector (see Surface.hpp), so iterate by const // pointer (py::cast accepts `const itype*` directly, see cast.h cast(const itype*)). for (const Surface* s : c.filter_by_type(t)) out.append(py::cast(s, py::return_value_policy::reference_internal, self)); @@ -333,7 +331,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) }, "Surfaces as [Surface] references into the live collection. Invalidated by " "set()/append()/clear() on this collection (C++ vector semantics)."); - // --- Extrusion tree (read-only in v1). Registered polymorphically: when a returned + // --- Extrusion tree (read-only). Registered polymorphically: when a returned // ExtrusionEntity*'s dynamic type IS one of the classes registered below, pybind // hands the plugin that concrete type, so plugins walk the same tree shape C++ does. // When the dynamic type is NOT registered (e.g. ExtrusionLoopSloped, produced with @@ -419,9 +417,9 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) .def_readonly("fill_surfaces", &LayerRegion::fill_surfaces, "Surfaces prepared for infill (SurfaceCollection). Edit in place or via fill_surfaces.set(...).") .def_readonly("perimeters", &LayerRegion::perimeters, - "Perimeter toolpaths (ExtrusionEntityCollection, read-only in v1).") + "Perimeter toolpaths (ExtrusionEntityCollection, read-only).") .def_readonly("fills", &LayerRegion::fills, - "Infill toolpaths (ExtrusionEntityCollection, read-only in v1).") + "Infill toolpaths (ExtrusionEntityCollection, read-only).") .def("layer", [](LayerRegion& r) -> py::object { Layer* l = r.layer(); if (l == nullptr) @@ -487,7 +485,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host) out.append(py::cast(static_cast(sl), py::return_value_policy::reference_internal, self)); return out; - }, "Support layers as [Layer] (support-specific fields are not exposed in v1).") + }, "Support layers as [Layer] (support-specific fields are not exposed).") .def("model_object", [](PrintObject& o) -> py::object { // The Print's model SNAPSHOT (worker-thread stable), reusing the // orca.host.ModelObject bindings — mesh access for slicing plugins. diff --git a/src/slic3r/plugin/PluginManager.hpp b/src/slic3r/plugin/PluginManager.hpp index 45d116e99f..6d45b5b577 100644 --- a/src/slic3r/plugin/PluginManager.hpp +++ b/src/slic3r/plugin/PluginManager.hpp @@ -102,7 +102,7 @@ void execute_capabilities_from_refs(const ConfigOptionStrings& capabilities, { PluginManager& plugin_mgr = PluginManager::instance(); - // Log prefix derived from the capability type so each capability family (Post-processing, + // Log prefix derived from the capability type so each capability family (Printer connection, // Slicing Pipeline, ...) tags its dispatch diagnostics with its own display name. const std::string tag = plugin_capability_type_display_name(type); diff --git a/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapability.cpp b/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapability.cpp index 2828c0496e..19e32118aa 100644 --- a/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapability.cpp +++ b/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapability.cpp @@ -10,7 +10,7 @@ namespace Slic3r { bool SlicingPipelineContext::cancelled() const { return print && print->canceled(); } void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py::enum_& pluginTypes) { - (void) pluginTypes; // matches gcode/script/printerAgent; Step is a fresh enum below. + (void) pluginTypes; // unused: this capability defines its own Step enum (below) rather than extending the shared PluginCapabilityType enum. auto slicing = module.def_submodule("slicing", "Slicing pipeline API (research/experimental)."); py::enum_(slicing, "Step") @@ -18,7 +18,7 @@ void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py:: .value("posPerimeters", SlicingPipelineStepPlugin::posPerimeters) .value("posEstimateCurledExtrusions", SlicingPipelineStepPlugin::posEstimateCurledExtrusions) .value("posPrepareInfill", SlicingPipelineStepPlugin::posPrepareInfill) // after prepare_infill, before make_fills: editing fill_surfaces here CASCADES - .value("posInfill", SlicingPipelineStepPlugin::posInfill) // after make_fills: editing fill_surfaces here does NOT regenerate fills (v1) + .value("posInfill", SlicingPipelineStepPlugin::posInfill) // after make_fills: editing fill_surfaces here does NOT regenerate the fills .value("posIroning", SlicingPipelineStepPlugin::posIroning) .value("posContouring", SlicingPipelineStepPlugin::posContouring) .value("posSupportMaterial", SlicingPipelineStepPlugin::posSupportMaterial) @@ -70,8 +70,8 @@ void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py:: if (ctx.object == nullptr) return py::none(); // The hook signature hands objects out as const; they are genuinely mutable - // (owned by the Print) — the same const_cast the old view mutators used, - // done once here at the graph entry point. + // (owned by the Print), so the const_cast is safe — done once here at the + // graph entry point so Python steps receive a mutable PrintObject. return py::cast(const_cast(ctx.object), py::return_value_policy::reference); }, "orca.host.PrintObject for object-scoped steps, or None for print-wide steps. " "Valid only during the execute(ctx) call.") diff --git a/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapabilityTrampoline.hpp b/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapabilityTrampoline.hpp index 5dc14c74b9..350f8be506 100644 --- a/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapabilityTrampoline.hpp +++ b/src/slic3r/plugin/pluginTypes/slicingPipeline/SlicingPipelinePluginCapabilityTrampoline.hpp @@ -14,10 +14,10 @@ public: [&]{ // At Step.psGCodePostProcess the plugin edits the exported G-code file, which lives // outside data_dir() (a temp/output folder), so writing to it would otherwise be - // blocked by the audit sandbox. Grant that folder as a scoped allowed root, mirroring - // the former G-code post-processing trampoline. The setup callback runs AFTER the - // audit context is constructed, so the scoped root is not cleared by its constructor. - // Empty at every other step, so no extra access is granted to the geometry hooks. + // blocked by the audit sandbox. Grant that folder as a scoped allowed root. The setup + // callback runs AFTER the audit context is constructed, so the scoped root is not + // cleared by its constructor. Empty at every other step, so no extra access is + // granted to the geometry hooks. if (!ctx.gcode_path.empty()) ::Slic3r::PluginAuditManager::instance().add_scoped_allowed_root( std::filesystem::path(ctx.gcode_path).parent_path()); diff --git a/tests/fff_print/test_slicing_pipeline_hook.cpp b/tests/fff_print/test_slicing_pipeline_hook.cpp index 9e035afea4..ceb17108d1 100644 --- a/tests/fff_print/test_slicing_pipeline_hook.cpp +++ b/tests/fff_print/test_slicing_pipeline_hook.cpp @@ -115,7 +115,7 @@ TEST_CASE("Inactive hook: process output is byte-identical (no-op hook == unset) CHECK(strip_nondeterministic_gcode_lines(run(true, true)) == baseline); // active no-op hook fires everywhere, mutates nothing } -// Fix 4(a): gating negative path. With the option EMPTY the plugin is inactive, so a +// Gating negative path. With the option EMPTY the plugin is inactive, so a // registered hook must NOT fire even once across a full slice (m_pipeline_plugin_active // stays false in Print::apply). Distinct from the byte-identical test above: this asserts // the gate directly by counting invocations rather than comparing output. @@ -132,7 +132,7 @@ TEST_CASE("Empty option: registered hook is gated off and never fires", "[slicin CHECK(calls == 0); } -// Fix 4(b): duplicate-skip gating. Two ModelObjects that share one mesh_ptr are detected as +// Duplicate-skip gating. Two ModelObjects that share one mesh_ptr are detected as // identical by Print::process()'s is_print_object_the_same(); the second becomes a shared // (duplicate) object and is NOT re-sliced, so the Slice hook must fire exactly once even // though there are two print objects. The clone shares mesh_ptr and copies the volume @@ -220,11 +220,11 @@ TEST_CASE("Changing slicing_pipeline_plugin invalidates posSlice", "[slicing_pip #include -// §3.6 (Twistify design): Twistify's effect is a similarity transform (rotate + uniform -// scale) applied to slices at Step.posSlice. This C++ analogue rotates every region's slices a -// fixed 45 deg about the object's base-footprint center -- the same seam and cascade that -// Twistify.py drives through the slices.set() + Layer::make_slices() path. Two end-to-end invariants after -// process() confirm the approach: +// A similarity transform (rotate + uniform scale) applied to slices at Step.posSlice, matching +// what the Twistify sample (sandboxes/orca_twistify_plugin_example_any.py) does. This C++ analogue +// rotates every region's slices a fixed 45 deg about the object's base-footprint center -- the same +// seam and cascade the sample drives through the slices.set() + Layer::make_slices() path. Two +// end-to-end invariants after process() confirm the approach: // (1) a pure rotation is a similarity with scale 1, so total fill area is preserved, and // (2) the mutation genuinely cascaded into make_perimeters' fill_surfaces -- a 20mm square // rotated 45 deg becomes a diamond whose bbox is ~sqrt(2)x wider (it did not stay @@ -300,13 +300,13 @@ TEST_CASE("Rotating slices at the Slice boundary cascades (area preserved, bbox CHECK(rot.height < 1.5 * base.height); } -// §3.6 (Twistify design): Twistify skips exact-identity layers entirely, but every transformed -// layer invokes the slices.set() write-back + make_perimeters re-run. This proves that write path -// is lossless for already-normalized (CCW contour / CW hole) input -- an active hook that -// re-sets every region's slices to their CURRENT geometry (the identity similarity transform) -// produces output byte-identical to an active hook that mutates nothing. Both runs are active -// (same config dump); the only difference is whether the write path ran, so equality isolates it. -TEST_CASE("Identity round-trip through set_slices is byte-identical", "[slicing_pipeline]") { +// The Twistify sample skips exact-identity layers entirely, but every transformed layer invokes +// the slices.set() write-back + make_perimeters re-run. This proves that write path is lossless +// for already-normalized (CCW contour / CW hole) input -- an active hook that re-sets every +// region's slices to their CURRENT geometry (the identity similarity transform) produces output +// byte-identical to an active hook that mutates nothing. Both runs are active (same config dump); +// the only difference is whether the write path ran, so equality isolates it. +TEST_CASE("Identity round-trip through slices.set() is byte-identical", "[slicing_pipeline]") { auto run = [](bool roundtrip) { Slic3r::Print print; Slic3r::Model model; auto config = Slic3r::DynamicPrintConfig::full_print_config(); @@ -390,7 +390,7 @@ TEST_CASE("raw_slices captures post-hook geometry so a perimeter re-run keeps th } // A plugin can mutate fill_surfaces at the new PrepareInfill seam and have make_fills consume -// them, whereas the pre-existing Infill seam fires after the fills are already built (v1 limit). +// them, whereas the pre-existing Infill seam fires after the fills are already built. // All three runs register a hook (active path) so the comparison isolates only the mutation. TEST_CASE("fill_surfaces mutation cascades at PrepareInfill but not at Infill", "[slicing_pipeline]") { auto fill_paths = [](bool shrink, Slic3r::SlicingPipelineStepPlugin at) { @@ -423,7 +423,7 @@ TEST_CASE("fill_surfaces mutation cascades at PrepareInfill but not at Infill", const size_t base = fill_paths(false, S::posPrepareInfill); // active hook, no mutation CHECK(base > 0); CHECK(fill_paths(true, S::posPrepareInfill) < base); // mutation before make_fills cascades - CHECK(fill_paths(true, S::posInfill) == base); // mutation after make_fills is a no-op (v1) + CHECK(fill_paths(true, S::posInfill) == base); // mutation after make_fills is a no-op } // lslices (the layer's merged islands) are built once in slice() and never rebuilt by diff --git a/tests/slic3rutils/test_slicing_pipeline_bindings.cpp b/tests/slic3rutils/test_slicing_pipeline_bindings.cpp index 35f49241b6..893e00ab04 100644 --- a/tests/slic3rutils/test_slicing_pipeline_bindings.cpp +++ b/tests/slic3rutils/test_slicing_pipeline_bindings.cpp @@ -75,7 +75,7 @@ TEST_CASE("make_writable_rows builds a writable (N,2) int64 view that aliases th TEST_CASE("orca.slicing module: Step enum, context, and a Python capability can execute", "[slicing_pipeline]") { ensure_python_initialized(); - import_orca_module(); // forces PythonPluginBridge::instance() (see test_plugin_host_api.cpp:32-40) + import_orca_module(); // forces PythonPluginBridge::instance() (see import_orca_module in python_test_support.hpp) py::gil_scoped_acquire gil; py::module_ orca = py::module_::import("orca"); REQUIRE(py::hasattr(orca, "slicing")); @@ -301,7 +301,7 @@ TEST_CASE("orca.host Surface/SurfaceCollection: construct, writable members, set surf.attr("thickness") = py::float_(0.3); CHECK_THAT(surf.attr("thickness").cast(), WithinRel(0.3, 1e-9)); - // SurfaceCollection.set(expolys, type) — the faithful replacement for set_slices' body. + // SurfaceCollection.set(expolys, type): replace all surfaces from a list of ExPolygon tagged with one SurfaceType. Slic3r::SurfaceCollection coll; py::object cv = py::cast(&coll, py::return_value_policy::reference); py::list expolys; expolys.append(ex);