docs(plugin): make comments self-contained; drop design-doc and dead-code references

Review the slicing-pipeline plugin comments for context a reader of the source
alone cannot follow, and rewrite them to stand on their own:

- drop pointers to uncommitted design/plan material ("§3.6 (Twistify design)",
  "the brief's note", "Fix 4(a)/4(b)")
- fix dangling references to code this branch removed: the retired set_slices()
  and view mutators, the former G-code post-processing capability/trampoline,
  the "Post-processing" capability family, the pre-refactor array helper
- drop "v1"/"in v1" phase labels, keeping the behavior they described
- correct stale cross-references: Twistify.py -> the real sample path;
  test_plugin_host_api.cpp:32-40 -> import_orca_module in python_test_support.hpp;
  "the binding"/"graphs above" -> the named source

Comment/string-only; no code behavior change.
This commit is contained in:
SoftFever
2026-07-11 03:30:05 +08:00
parent 19352215da
commit a04ce5f81e
9 changed files with 40 additions and 42 deletions

View File

@@ -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 hook, so the change cascades into perimeters, infill and the final G-code
-- the toolpath preview shrinks. -- the toolpath preview shrinks.
Unlike the old axis-aligned demo, ExPolygon.offset() is a correct inward offset ExPolygon.offset() is a correct inward offset for any contour (it is Clipper
for any contour (it is Clipper under the hood), and it naturally handles holes. under the hood), and it naturally handles holes.
A surface may split into several islands or vanish when shrunk; both are handled. 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. No numpy required: the whole edit is expressed with the host geometry classes.

View File

@@ -103,7 +103,7 @@ enum class SlicingPipelineStepPlugin {
posSlice, posPerimeters, posEstimateCurledExtrusions, posPrepareInfill, posInfill, posIroning, posContouring, posSlice, posPerimeters, posEstimateCurledExtrusions, posPrepareInfill, posInfill, posIroning, posContouring,
posSupportMaterial, posDetectOverhangsForLift, posSimplifyPath, psWipeTower, psSkirtBrim, posSupportMaterial, posDetectOverhangsForLift, posSimplifyPath, psWipeTower, psSkirtBrim,
// Fires from the GUI G-code export/post-process seam (PostProcessor.cpp), NOT from Print::process(). // 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 psGCodePostProcess
}; };

View File

@@ -39,8 +39,8 @@ pybind11::array make_readonly_rows(pybind11::handle base, const T* data, pybind1
namespace py = pybind11; namespace py = pybind11;
if (rows == 0 || data == nullptr) { if (rows == 0 || data == nullptr) {
py::array_t<T> empty(std::vector<py::ssize_t>{ 0, (py::ssize_t) N }); py::array_t<T> empty(std::vector<py::ssize_t>{ 0, (py::ssize_t) N });
// Keep behavior-preserving: the pre-refactor helper returned read-only // Mark the fresh empty array read-only so every return path of this
// arrays on every path, so mark the fresh empty array read-only too. // helper yields a read-only view.
empty.attr("setflags")(py::arg("write") = false); empty.attr("setflags")(py::arg("write") = false);
return std::move(empty); return std::move(empty);
} }

View File

@@ -109,7 +109,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host)
// ------------------------------------------------------------------ // ------------------------------------------------------------------
// Slicing print-graph data model — raw bindings of the classes the C++ // Slicing print-graph data model — raw bindings of the classes the C++
// pipeline itself uses, same nodelete/reference style as the Model and // 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 // 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 // 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_type", [](SurfaceCollection& c, SurfaceType t) { c.set_type(t); }, py::arg("surface_type"))
.def("set", [](SurfaceCollection& c, const std::vector<ExPolygon>& src, SurfaceType t) { c.set(src, t); }, .def("set", [](SurfaceCollection& c, const std::vector<ExPolygon>& src, SurfaceType t) { c.set(src, t); },
py::arg("expolygons"), py::arg("surface_type"), py::arg("expolygons"), py::arg("surface_type"),
"Replace all surfaces from a list of ExPolygon, all tagged `surface_type`. " "Replace all surfaces from a list of ExPolygon, all tagged `surface_type`.")
"This is the faithful replacement for the retired set_slices().")
.def("set", [](SurfaceCollection& c, const std::vector<Surface>& src) { c.set(src); }, .def("set", [](SurfaceCollection& c, const std::vector<Surface>& src) { c.set(src); },
py::arg("surfaces"), "Replace all surfaces from a list of Surface (types preserved per surface).") py::arg("surfaces"), "Replace all surfaces from a list of Surface (types preserved per surface).")
.def("append", [](SurfaceCollection& c, const std::vector<ExPolygon>& src, SurfaceType t) { c.append(src, t); }, .def("append", [](SurfaceCollection& c, const std::vector<ExPolygon>& 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) { .def("filter_by_type", [](py::object self, SurfaceType t) {
SurfaceCollection& c = self.cast<SurfaceCollection&>(); SurfaceCollection& c = self.cast<SurfaceCollection&>();
py::list out; py::list out;
// SurfacesPtr (SurfaceCollection::filter_by_type's return type) is // SurfaceCollection::filter_by_type returns SurfacesPtr, which is
// std::vector<const Surface*> (see Surface.hpp); the brief's note describing it // std::vector<const Surface*> (see Surface.hpp), so iterate by const
// as std::vector<Surface*> does not match the header, so this iterates by const
// pointer (py::cast accepts `const itype*` directly, see cast.h cast(const itype*)). // pointer (py::cast accepts `const itype*` directly, see cast.h cast(const itype*)).
for (const Surface* s : c.filter_by_type(t)) for (const Surface* s : c.filter_by_type(t))
out.append(py::cast(s, py::return_value_policy::reference_internal, self)); 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 " }, "Surfaces as [Surface] references into the live collection. Invalidated by "
"set()/append()/clear() on this collection (C++ vector semantics)."); "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 // 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. // 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 // 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, .def_readonly("fill_surfaces", &LayerRegion::fill_surfaces,
"Surfaces prepared for infill (SurfaceCollection). Edit in place or via fill_surfaces.set(...).") "Surfaces prepared for infill (SurfaceCollection). Edit in place or via fill_surfaces.set(...).")
.def_readonly("perimeters", &LayerRegion::perimeters, .def_readonly("perimeters", &LayerRegion::perimeters,
"Perimeter toolpaths (ExtrusionEntityCollection, read-only in v1).") "Perimeter toolpaths (ExtrusionEntityCollection, read-only).")
.def_readonly("fills", &LayerRegion::fills, .def_readonly("fills", &LayerRegion::fills,
"Infill toolpaths (ExtrusionEntityCollection, read-only in v1).") "Infill toolpaths (ExtrusionEntityCollection, read-only).")
.def("layer", [](LayerRegion& r) -> py::object { .def("layer", [](LayerRegion& r) -> py::object {
Layer* l = r.layer(); Layer* l = r.layer();
if (l == nullptr) if (l == nullptr)
@@ -487,7 +485,7 @@ void PluginHostSlicing::RegisterBindings(py::module_& host)
out.append(py::cast(static_cast<Layer*>(sl), out.append(py::cast(static_cast<Layer*>(sl),
py::return_value_policy::reference_internal, self)); py::return_value_policy::reference_internal, self));
return out; 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 { .def("model_object", [](PrintObject& o) -> py::object {
// The Print's model SNAPSHOT (worker-thread stable), reusing the // The Print's model SNAPSHOT (worker-thread stable), reusing the
// orca.host.ModelObject bindings — mesh access for slicing plugins. // orca.host.ModelObject bindings — mesh access for slicing plugins.

View File

@@ -102,7 +102,7 @@ void execute_capabilities_from_refs(const ConfigOptionStrings& capabilities,
{ {
PluginManager& plugin_mgr = PluginManager::instance(); 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. // Slicing Pipeline, ...) tags its dispatch diagnostics with its own display name.
const std::string tag = plugin_capability_type_display_name(type); const std::string tag = plugin_capability_type_display_name(type);

View File

@@ -10,7 +10,7 @@ namespace Slic3r {
bool SlicingPipelineContext::cancelled() const { return print && print->canceled(); } bool SlicingPipelineContext::cancelled() const { return print && print->canceled(); }
void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py::enum_<PluginCapabilityType>& pluginTypes) { void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py::enum_<PluginCapabilityType>& 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)."); auto slicing = module.def_submodule("slicing", "Slicing pipeline API (research/experimental).");
py::enum_<SlicingPipelineStepPlugin>(slicing, "Step") py::enum_<SlicingPipelineStepPlugin>(slicing, "Step")
@@ -18,7 +18,7 @@ void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py::
.value("posPerimeters", SlicingPipelineStepPlugin::posPerimeters) .value("posPerimeters", SlicingPipelineStepPlugin::posPerimeters)
.value("posEstimateCurledExtrusions", SlicingPipelineStepPlugin::posEstimateCurledExtrusions) .value("posEstimateCurledExtrusions", SlicingPipelineStepPlugin::posEstimateCurledExtrusions)
.value("posPrepareInfill", SlicingPipelineStepPlugin::posPrepareInfill) // after prepare_infill, before make_fills: editing fill_surfaces here CASCADES .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("posIroning", SlicingPipelineStepPlugin::posIroning)
.value("posContouring", SlicingPipelineStepPlugin::posContouring) .value("posContouring", SlicingPipelineStepPlugin::posContouring)
.value("posSupportMaterial", SlicingPipelineStepPlugin::posSupportMaterial) .value("posSupportMaterial", SlicingPipelineStepPlugin::posSupportMaterial)
@@ -70,8 +70,8 @@ void SlicingPipelinePluginCapability::RegisterBindings(py::module_& module, py::
if (ctx.object == nullptr) if (ctx.object == nullptr)
return py::none(); return py::none();
// The hook signature hands objects out as const; they are genuinely mutable // 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, // (owned by the Print), so the const_cast is safe — done once here at the
// done once here at the graph entry point. // graph entry point so Python steps receive a mutable PrintObject.
return py::cast(const_cast<PrintObject*>(ctx.object), py::return_value_policy::reference); return py::cast(const_cast<PrintObject*>(ctx.object), py::return_value_policy::reference);
}, "orca.host.PrintObject for object-scoped steps, or None for print-wide steps. " }, "orca.host.PrintObject for object-scoped steps, or None for print-wide steps. "
"Valid only during the execute(ctx) call.") "Valid only during the execute(ctx) call.")

View File

@@ -14,10 +14,10 @@ public:
[&]{ [&]{
// At Step.psGCodePostProcess the plugin edits the exported G-code file, which lives // 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 // 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 // blocked by the audit sandbox. Grant that folder as a scoped allowed root. The setup
// the former G-code post-processing trampoline. The setup callback runs AFTER the // callback runs AFTER the audit context is constructed, so the scoped root is not
// audit context is constructed, so the scoped root is not cleared by its constructor. // cleared by its constructor. Empty at every other step, so no extra access is
// Empty at every other step, so no extra access is granted to the geometry hooks. // granted to the geometry hooks.
if (!ctx.gcode_path.empty()) if (!ctx.gcode_path.empty())
::Slic3r::PluginAuditManager::instance().add_scoped_allowed_root( ::Slic3r::PluginAuditManager::instance().add_scoped_allowed_root(
std::filesystem::path(ctx.gcode_path).parent_path()); std::filesystem::path(ctx.gcode_path).parent_path());

View File

@@ -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 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 // 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 // stays false in Print::apply). Distinct from the byte-identical test above: this asserts
// the gate directly by counting invocations rather than comparing output. // 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); 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 // 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 // (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 // 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 <catch2/matchers/catch_matchers_floating_point.hpp> #include <catch2/matchers/catch_matchers_floating_point.hpp>
// §3.6 (Twistify design): Twistify's effect is a similarity transform (rotate + uniform // A similarity transform (rotate + uniform scale) applied to slices at Step.posSlice, matching
// scale) applied to slices at Step.posSlice. This C++ analogue rotates every region's slices a // what the Twistify sample (sandboxes/orca_twistify_plugin_example_any.py) does. This C++ analogue
// fixed 45 deg about the object's base-footprint center -- the same seam and cascade that // rotates every region's slices a fixed 45 deg about the object's base-footprint center -- the same
// Twistify.py drives through the slices.set() + Layer::make_slices() path. Two end-to-end invariants after // seam and cascade the sample drives through the slices.set() + Layer::make_slices() path. Two
// process() confirm the approach: // 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 // (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 // (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 // 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); CHECK(rot.height < 1.5 * base.height);
} }
// §3.6 (Twistify design): Twistify skips exact-identity layers entirely, but every transformed // The Twistify sample skips exact-identity layers entirely, but every transformed layer invokes
// layer invokes the slices.set() write-back + make_perimeters re-run. This proves that write path // the slices.set() write-back + make_perimeters re-run. This proves that write path is lossless
// is lossless for already-normalized (CCW contour / CW hole) input -- an active hook that // for already-normalized (CCW contour / CW hole) input -- an active hook that re-sets every
// re-sets every region's slices to their CURRENT geometry (the identity similarity transform) // region's slices to their CURRENT geometry (the identity similarity transform) produces output
// produces output byte-identical to an active hook that mutates nothing. Both runs are active // byte-identical to an active hook that mutates nothing. Both runs are active (same config dump);
// (same config dump); the only difference is whether the write path ran, so equality isolates it. // 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]") { TEST_CASE("Identity round-trip through slices.set() is byte-identical", "[slicing_pipeline]") {
auto run = [](bool roundtrip) { auto run = [](bool roundtrip) {
Slic3r::Print print; Slic3r::Model model; Slic3r::Print print; Slic3r::Model model;
auto config = Slic3r::DynamicPrintConfig::full_print_config(); 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 // 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. // 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]") { TEST_CASE("fill_surfaces mutation cascades at PrepareInfill but not at Infill", "[slicing_pipeline]") {
auto fill_paths = [](bool shrink, Slic3r::SlicingPipelineStepPlugin at) { 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 const size_t base = fill_paths(false, S::posPrepareInfill); // active hook, no mutation
CHECK(base > 0); CHECK(base > 0);
CHECK(fill_paths(true, S::posPrepareInfill) < base); // mutation before make_fills cascades 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 // lslices (the layer's merged islands) are built once in slice() and never rebuilt by

View File

@@ -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]") { TEST_CASE("orca.slicing module: Step enum, context, and a Python capability can execute", "[slicing_pipeline]") {
ensure_python_initialized(); 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::gil_scoped_acquire gil;
py::module_ orca = py::module_::import("orca"); py::module_ orca = py::module_::import("orca");
REQUIRE(py::hasattr(orca, "slicing")); 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); surf.attr("thickness") = py::float_(0.3);
CHECK_THAT(surf.attr("thickness").cast<double>(), WithinRel(0.3, 1e-9)); CHECK_THAT(surf.attr("thickness").cast<double>(), 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; Slic3r::SurfaceCollection coll;
py::object cv = py::cast(&coll, py::return_value_policy::reference); py::object cv = py::cast(&coll, py::return_value_policy::reference);
py::list expolys; expolys.append(ex); py::list expolys; expolys.append(ex);