Browse Source

fix(slicer): stop a 3MF from switching off supports its process preset turned on (#2820)

--load-settings is authoritative, so since #1881 four support fields
travel the other way -- enable_support, the two filament slots, and
support_type -- lifted out of the source 3MF and written over the picked
process preset. Bambu's shipped presets all set enable_support: 0
because supports are a per-print decision, and without the carry a
project exported with PVA in the interface slot sliced single-material.

But the carry ran in both directions, and the off direction is the one
nobody asked for. Nearly every published model ships with supports off,
so slicing one against a custom preset that deliberately enabled them
stripped them back out. The reporter's preset sets enable_support 1,
support_type normal(auto), support_style snug; the slice came back
disabled and tree(auto). Only the style survived -- it is not one of the
four carried, and they had re-entered it in the slice dialog.

The source can now switch supports on, never off. Nothing is lost:
every shipped preset has them off, so a preset that has them on is a
deliberate choice by whoever wrote it, and a file that wants supports
still gets them with its slot assignments. A file that never declares
enable_support is treated as off -- no intent to act on.

The truthiness rule ("1", true, 1, and the forks that write neither) now
lives in one place as supports_enabled_in_config(), shared with
extract_support_filament_slots_from_3mf, which had it inline.

Also log the carry with the fields it took. The slice dialog shows the
picked preset's values, so a carried field silently disagrees with what
was on screen and this step logged nothing at all -- the report chased an
unrelated sanitiser line about the source file's own settings, which was
the only thing in the log that mentioned any of these keys.
maziggy 3 weeks ago
parent
commit
36d996e453

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 22 - 3
backend/app/api/routes/library.py

@@ -81,6 +81,7 @@ from backend.app.utils.threemf_tools import (
     extract_nozzle_mapping_from_3mf,
     extract_nozzle_mapping_from_3mf,
     extract_project_filaments_from_3mf,
     extract_project_filaments_from_3mf,
     select_plate_gcode_name,
     select_plate_gcode_name,
+    supports_enabled_in_config,
 )
 )
 
 
 logger = logging.getLogger(__name__)
 logger = logging.getLogger(__name__)
@@ -3622,6 +3623,16 @@ _SOURCE_PROCESS_SUPPORT_KEYS_TO_PRESERVE = (
 def _patch_process_support_settings(process_json: str, source_3mf_bytes: bytes) -> str:
 def _patch_process_support_settings(process_json: str, source_3mf_bytes: bytes) -> str:
     """Overlay the source 3MF's support configuration onto the process JSON.
     """Overlay the source 3MF's support configuration onto the process JSON.
 
 
+    The carry is deliberately one-way: a source can switch supports *on*,
+    never off (#2820). The original #1881 rule was "source wins in both
+    directions", which quietly stripped supports from every custom process
+    preset that enabled them — a MakerWorld download nearly always ships
+    `enable_support: 0`, so the reporter's own preset (supports on, normal
+    (auto)) came back out of the slicer disabled and set to tree(auto).
+    Nothing is lost by not carrying the off direction: a process preset
+    with supports *on* is by definition a deliberate user preset, since
+    Bambu's shipped ones all ship them off.
+
     Only fires on 3MF sources — STL / STEP don't carry `project_settings.
     Only fires on 3MF sources — STL / STEP don't carry `project_settings.
     config`. Silently no-ops when the source doesn't have the config, has
     config`. Silently no-ops when the source doesn't have the config, has
     a malformed one, or when the process JSON isn't parseable — the slice
     a malformed one, or when the process JSON isn't parseable — the slice
@@ -3639,6 +3650,8 @@ def _patch_process_support_settings(process_json: str, source_3mf_bytes: bytes)
         return process_json
         return process_json
     if not isinstance(src_cfg, dict):
     if not isinstance(src_cfg, dict):
         return process_json
         return process_json
+    if not supports_enabled_in_config(src_cfg):
+        return process_json
 
 
     try:
     try:
         process_cfg = json.loads(process_json)
         process_cfg = json.loads(process_json)
@@ -3647,9 +3660,15 @@ def _patch_process_support_settings(process_json: str, source_3mf_bytes: bytes)
     if not isinstance(process_cfg, dict):
     if not isinstance(process_cfg, dict):
         return process_json
         return process_json
 
 
-    for key in _SOURCE_PROCESS_SUPPORT_KEYS_TO_PRESERVE:
-        if key in src_cfg:
-            process_cfg[key] = src_cfg[key]
+    carried = {key: src_cfg[key] for key in _SOURCE_PROCESS_SUPPORT_KEYS_TO_PRESERVE if key in src_cfg}
+    process_cfg.update(carried)
+    # Logged because this is the one layer of the process JSON the user
+    # can't see coming: the slice modal shows the picked preset's values,
+    # so a carried key silently disagrees with what was on screen.
+    logger.info(
+        "Carried support settings from the source 3MF onto the process preset: %s",
+        dict(sorted(carried.items())),
+    )
 
 
     return json.dumps(process_cfg)
     return json.dumps(process_cfg)
 
 

+ 19 - 6
backend/app/utils/threemf_tools.py

@@ -1099,6 +1099,24 @@ def expand_to_project_slots(zf: zipfile.ZipFile, used: list[dict]) -> list[dict]
     return out
     return out
 
 
 
 
+# BambuStudio serialises bool config options as string "1"/"0" in
+# project_settings.config, but forks / older versions occasionally write real
+# booleans or ints — accept anything that isn't unambiguously falsy. A missing
+# key counts as off: a 3MF that never declares `enable_support` gives us no
+# support intent to act on.
+_SUPPORTS_DISABLED_VALUES = (False, 0, "0", "false", "False", "", None)
+
+
+def supports_enabled_in_config(cfg: dict[str, object]) -> bool:
+    """Whether a 3MF's ``project_settings.config`` has supports switched on.
+
+    Shared by the callers that read support intent out of a source file so
+    they agree on what "on" means: the slot extractor below and the slice
+    route's process-preset support carry-over (#1881 / #2820).
+    """
+    return cfg.get("enable_support") not in _SUPPORTS_DISABLED_VALUES
+
+
 def extract_support_filament_slots_from_3mf(zf: zipfile.ZipFile) -> set[int]:
 def extract_support_filament_slots_from_3mf(zf: zipfile.ZipFile) -> set[int]:
     """Slots referenced by the process settings for support material.
     """Slots referenced by the process settings for support material.
 
 
@@ -1124,12 +1142,7 @@ def extract_support_filament_slots_from_3mf(zf: zipfile.ZipFile) -> set[int]:
         return set()
         return set()
     if not isinstance(cfg, dict):
     if not isinstance(cfg, dict):
         return set()
         return set()
-    # BambuStudio serialises bool config options as string "1"/"0" in
-    # project_settings.config, but forks / older versions occasionally
-    # write real booleans or ints — accept anything that isn't
-    # unambiguously falsy.
-    enable = cfg.get("enable_support")
-    if enable in (False, 0, "0", "false", "False", "", None):
+    if not supports_enabled_in_config(cfg):
         return set()
         return set()
     out: set[int] = set()
     out: set[int] = set()
     for key in ("support_filament", "support_interface_filament"):
     for key in ("support_filament", "support_interface_filament"):

+ 68 - 10
backend/tests/unit/test_slice_process_support_patch.py

@@ -13,10 +13,16 @@ but never used.
 The patch reads support-related fields from the source's
 The patch reads support-related fields from the source's
 project_settings.config and overlays them onto the process preset JSON,
 project_settings.config and overlays them onto the process preset JSON,
 so the source's per-project support intent survives `--load-settings`.
 so the source's per-project support intent survives `--load-settings`.
+
+The carry is one-way (#2820): a source can switch supports on, never off.
+The original rule was symmetric, which meant any 3MF that shipped with
+supports disabled -- i.e. nearly every MakerWorld download -- stripped
+them back out of a custom process preset that deliberately enabled them.
 """
 """
 
 
 import io
 import io
 import json
 import json
+import logging
 import zipfile
 import zipfile
 
 
 from backend.app.api.routes.library import _patch_process_support_settings
 from backend.app.api.routes.library import _patch_process_support_settings
@@ -65,24 +71,76 @@ class TestPatchProcessSupportSettings:
         assert result["layer_height"] == "0.20"
         assert result["layer_height"] == "0.20"
         assert result["name"] == "0.20mm Standard @BBL H2D"
         assert result["name"] == "0.20mm Standard @BBL H2D"
 
 
-    def test_source_supports_off_beats_preset_supports_on(self):
-        # Symmetric: a source with supports explicitly disabled must win
-        # over a process preset that happens to have supports on. Rare in
-        # practice (Bambu's presets ship off) but the semantic is "source
-        # wins" regardless of direction — a user who exported without
-        # supports doesn't want a preset accidentally re-enabling them.
+    def test_preset_supports_on_survives_a_source_with_supports_off(self):
+        # #2820: the reporter's own process preset turns supports on with
+        # normal(auto); the MakerWorld source they sliced ships them off
+        # with tree(auto), like nearly every published 3MF. Carrying the
+        # off direction handed them a supportless tree(auto) slice, so the
+        # source is now only allowed to switch supports *on*.
         source = _make_3mf(
         source = _make_3mf(
             {
             {
                 "enable_support": "0",
                 "enable_support": "0",
                 "support_filament": "0",
                 "support_filament": "0",
                 "support_interface_filament": "0",
                 "support_interface_filament": "0",
+                "support_type": "tree(auto)",
+            }
+        )
+        preset = json.dumps(
+            {
+                "name": "Pokeball Fast - Buddy",
+                "enable_support": "1",
+                "support_filament": "2",
+                "support_interface_filament": "2",
+                "support_type": "normal(auto)",
+                "support_style": "snug",
             }
             }
         )
         )
-        preset = json.dumps({"enable_support": "1", "support_filament": "2", "support_interface_filament": "2"})
         result = json.loads(_patch_process_support_settings(preset, source))
         result = json.loads(_patch_process_support_settings(preset, source))
-        assert result["enable_support"] == "0"
-        assert result["support_filament"] == "0"
-        assert result["support_interface_filament"] == "0"
+        assert result["enable_support"] == "1"
+        assert result["support_filament"] == "2"
+        assert result["support_interface_filament"] == "2"
+        assert result["support_type"] == "normal(auto)"
+        assert result["support_style"] == "snug"
+
+    def test_source_without_enable_support_carries_nothing(self):
+        # A source that never declares enable_support gives us no support
+        # intent to act on, so its slot assignments stay out of the preset
+        # — same "supports off" branch, reached via the missing key.
+        source = _make_3mf({"support_filament": "3", "support_interface_filament": "3"})
+        preset = json.dumps({"support_filament": "0", "support_interface_filament": "0"})
+        result = json.loads(_patch_process_support_settings(preset, source))
+        assert result == {"support_filament": "0", "support_interface_filament": "0"}
+
+    def test_non_string_enable_support_still_counts_as_on(self):
+        # Forks and older BambuStudio builds write real booleans / ints
+        # instead of "1" — those must still carry (shared truthiness rule
+        # with extract_support_filament_slots_from_3mf).
+        for enabled in (True, 1, "1", "true"):
+            source = _make_3mf({"enable_support": enabled, "support_interface_filament": "2"})
+            preset = json.dumps({"enable_support": "0", "support_interface_filament": "0"})
+            result = json.loads(_patch_process_support_settings(preset, source))
+            assert result["enable_support"] == enabled, f"failed for {enabled!r}"
+            assert result["support_interface_filament"] == "2"
+
+    def test_carry_is_logged_with_the_keys_it_took(self, caplog):
+        # The slice modal shows the picked preset's values, so a carried
+        # key silently disagrees with what the user saw. #2820's reporter
+        # spent the bug report chasing an unrelated sanitiser line because
+        # this step logged nothing at all.
+        source = _make_3mf({"enable_support": "1", "support_interface_filament": "2"})
+        preset = json.dumps({"enable_support": "0", "support_interface_filament": "0"})
+        with caplog.at_level(logging.INFO, logger="backend.app.api.routes.library"):
+            _patch_process_support_settings(preset, source)
+        assert "Carried support settings" in caplog.text
+        assert "enable_support" in caplog.text
+        assert "support_interface_filament" in caplog.text
+
+    def test_no_log_when_the_source_has_supports_off(self, caplog):
+        source = _make_3mf({"enable_support": "0", "support_type": "tree(auto)"})
+        preset = json.dumps({"enable_support": "1"})
+        with caplog.at_level(logging.INFO, logger="backend.app.api.routes.library"):
+            _patch_process_support_settings(preset, source)
+        assert "Carried support settings" not in caplog.text
 
 
     def test_only_patches_keys_present_in_source(self):
     def test_only_patches_keys_present_in_source(self):
         # Source with a partial support config (e.g. legacy 3MFs from an
         # Source with a partial support config (e.g. legacy 3MFs from an

Some files were not shown because too many files changed in this diff