Przeglądaj źródła

Pick the nearest eligible filament colour instead of the first one in tray order (#2804) (#2823)

Martin Grolmus 3 tygodni temu
rodzic
commit
4f7a02b393

Plik diff jest za duży
+ 0 - 0
CHANGELOG.md


+ 69 - 31
backend/app/services/print_scheduler.py

@@ -2740,6 +2740,31 @@ class PrintScheduler:
             return ""
         return color.replace("#", "").lower()[:6]
 
+    def _color_distance(self, color1: str | None, color2: str | None) -> float | None:
+        """Euclidean RGB distance, or None when either colour is unusable.
+
+        Ranks the candidates ``_colors_are_similar`` admits (#2804). Eligibility
+        stays the per-channel box that shipped — this only decides which of
+        several eligible spools is closest, so nothing becomes usable or
+        unusable because of it.
+
+        Alpha is dropped by ``_normalize_color_for_compare``, deliberately: the
+        alpha a slicer writes for a transparent filament is not a colour the
+        user chose, and counting it would stop a transparent filament matching
+        itself.
+        """
+        hex1 = self._normalize_color_for_compare(color1)
+        hex2 = self._normalize_color_for_compare(color2)
+        if not hex1 or not hex2 or len(hex1) < 6 or len(hex2) < 6:
+            return None
+        try:
+            dr = int(hex1[0:2], 16) - int(hex2[0:2], 16)
+            dg = int(hex1[2:4], 16) - int(hex2[2:4], 16)
+            db = int(hex1[4:6], 16) - int(hex2[4:6], 16)
+        except ValueError:
+            return None
+        return (dr * dr + dg * dg + db * db) ** 0.5
+
     def _colors_are_similar(self, color1: str | None, color2: str | None, threshold: int = 40) -> bool:
         """Check if two colors are visually similar within a threshold."""
         hex1 = self._normalize_color_for_compare(color1)
@@ -2943,6 +2968,7 @@ class PrintScheduler:
             idx_match = None
             exact_match = None
             similar_match = None
+            similar_distance = float("inf")
             type_only_match = None
 
             # Get available trays (not already used)
@@ -3018,8 +3044,10 @@ class PrintScheduler:
                             if not exact_match:
                                 exact_match = f
                         elif self._colors_are_similar(f_color, req_color):
-                            if not similar_match:
+                            distance = self._color_distance(f_color, req_color)
+                            if distance is not None and distance < similar_distance:
                                 similar_match = f
+                                similar_distance = distance
                         elif not type_only_match:
                             type_only_match = f
 
@@ -3036,8 +3064,15 @@ class PrintScheduler:
                         if not exact_match:
                             exact_match = f
                     elif self._colors_are_similar(f_color, req_color):
-                        if not similar_match:
+                        # Nearest wins, not first-in-tray-order. `available` is
+                        # already in the caller's order (slot order, or the
+                        # prefer-lowest sort), and `<` keeps the earliest of
+                        # equally close spools — so that order survives as the
+                        # tie-break (#2804).
+                        distance = self._color_distance(f_color, req_color)
+                        if distance is not None and distance < similar_distance:
                             similar_match = f
+                            similar_distance = distance
                     elif not type_only_match:
                         type_only_match = f
 
@@ -3047,35 +3082,38 @@ class PrintScheduler:
                 comparisons.append({"slot_id": req.get("slot_id", 0), "global_tray_id": match["global_tray_id"]})
             else:
                 comparisons.append({"slot_id": req.get("slot_id", 0), "global_tray_id": -1})
-            if prefer_lowest:
-                # Pair with the "available (sorted)" log above so the reporter
-                # bundle shows BOTH what the matcher saw AND which match bucket
-                # won — fast triage when "Prefer Lowest Filament" picks the
-                # wrong slot (#1766).
-                if match:
-                    bucket = (
-                        "idx"
-                        if idx_match is not None
-                        else "exact_color"
-                        if exact_match is not None
-                        else "similar_color"
-                        if similar_match is not None
-                        else "type_only"
-                    )
-                    logger.info(
-                        "[prefer-lowest] picked gtid=%s via %s for req slot=%s",
-                        match["global_tray_id"],
-                        bucket,
-                        req.get("slot_id"),
-                    )
-                else:
-                    logger.info(
-                        "[prefer-lowest] NO MATCH for req slot=%s (type=%r color=%r tii=%r)",
-                        req.get("slot_id"),
-                        req_type,
-                        req_color,
-                        req_tray_info_idx,
-                    )
+            # Which bucket won, always — not only under Prefer Lowest (#2804).
+            # "Why did it pick that spool" is the question every wrong-filament
+            # report starts with, and a `similar_color` win is now a ranked
+            # choice among several eligible spools rather than whichever tray
+            # came first, so it is worth being able to see after the fact.
+            # Pairs with the "available (sorted)" log above when Prefer Lowest
+            # is on (#1766).
+            if match:
+                bucket = (
+                    "idx"
+                    if idx_match is not None
+                    else "exact_color"
+                    if exact_match is not None
+                    else "similar_color"
+                    if similar_match is not None
+                    else "type_only"
+                )
+                logger.info(
+                    "[ams-match] picked gtid=%s via %s for req slot=%s%s",
+                    match["global_tray_id"],
+                    bucket,
+                    req.get("slot_id"),
+                    f" (colour distance {similar_distance:.1f})" if bucket == "similar_color" else "",
+                )
+            else:
+                logger.info(
+                    "[ams-match] NO MATCH for req slot=%s (type=%r color=%r tii=%r)",
+                    req.get("slot_id"),
+                    req_type,
+                    req_color,
+                    req_tray_info_idx,
+                )
 
         # Build mapping array
         if not comparisons:

+ 107 - 0
backend/tests/unit/test_nearest_colour_match_2804.py

@@ -0,0 +1,107 @@
+"""Regression tests for slot-order-dependent colour matching (#2804).
+
+When no spool matches a required colour exactly, the matcher falls back to
+spools inside a per-channel tolerance. It took the first of those in tray order,
+so the winner depended on which slot a spool sat in rather than on which colour
+was closest.
+
+The worked example throughout is the maintainer's: a required ``#3A7BD5`` with a
+purple ``#6253AD`` in tray 1 (40/40/40 off — admitted by the box, distance ~69)
+and a near-identical ``#3B7AD2`` in tray 3 (distance ~3). Both qualify; the
+purple used to win on position alone.
+"""
+
+import pytest
+
+from backend.app.services.print_scheduler import PrintScheduler
+
+REQUIRED_COLOR = "#3A7BD5"
+NEAR = "3B7AD2FF"  # distance ~3
+FAR_BUT_ADMITTED = "6253ADFF"  # 40/40/40 off — inside the box, distance ~69
+
+
+@pytest.fixture
+def scheduler():
+    return PrintScheduler.__new__(PrintScheduler)
+
+
+def tray(global_tray_id, color, type_="PETG"):
+    return {
+        "global_tray_id": global_tray_id,
+        "type": type_,
+        "color": color,
+        "tray_info_idx": "",
+        "extruder_id": 0,
+        "remain": 100,
+    }
+
+
+def req(color=REQUIRED_COLOR, slot_id=1, type_="PETG", tray_info_idx=""):
+    return {"slot_id": slot_id, "type": type_, "color": color, "tray_info_idx": tray_info_idx}
+
+
+class TestColorDistance:
+    def test_identical_colours_are_zero_apart(self, scheduler):
+        assert scheduler._color_distance("#3A7BD5", "3A7BD5FF") == 0
+
+    def test_alpha_is_ignored_so_a_transparent_filament_matches_itself(self, scheduler):
+        """The alpha a slicer writes is not a colour the user chose."""
+        assert scheduler._color_distance("#76D9F4", "76D9F400") == 0
+
+    def test_distance_is_euclidean_not_per_channel(self, scheduler):
+        # 40 off on each channel is sqrt(3 * 40^2) ~= 69.28, not 40.
+        assert scheduler._color_distance("#000000", "#282828") == pytest.approx(69.28, abs=0.01)
+
+    def test_unusable_input_is_none_rather_than_a_number(self, scheduler):
+        assert scheduler._color_distance(None, "#3A7BD5") is None
+        assert scheduler._color_distance("", "#3A7BD5") is None
+        assert scheduler._color_distance("#abc", "#3A7BD5") is None
+        assert scheduler._color_distance("#zzzzzz", "#3A7BD5") is None
+
+
+class TestNearestSimilarWins:
+    def test_closest_admitted_colour_wins_regardless_of_slot_order(self, scheduler):
+        loaded = [tray(1, FAR_BUT_ADMITTED), tray(3, NEAR)]
+        assert scheduler._match_filaments_to_slots([req()], loaded) == [3]
+
+    def test_result_does_not_depend_on_tray_order(self, scheduler):
+        """The bug in one line: reversing the AMS used to reverse the answer."""
+        forward = scheduler._match_filaments_to_slots([req()], [tray(1, FAR_BUT_ADMITTED), tray(3, NEAR)])
+        reversed_ = scheduler._match_filaments_to_slots([req()], [tray(3, NEAR), tray(1, FAR_BUT_ADMITTED)])
+        assert forward == reversed_ == [3]
+
+    def test_an_exact_match_still_outranks_a_near_one(self, scheduler):
+        loaded = [tray(1, NEAR), tray(2, "3A7BD5FF")]
+        assert scheduler._match_filaments_to_slots([req()], loaded) == [2]
+
+    def test_eligibility_is_unchanged_so_a_far_colour_is_still_type_only(self, scheduler):
+        """Ranking must not admit spools the tolerance excluded: 41 off on one
+        channel fails the box and can only win as a type-only fallback."""
+        loaded = [tray(5, "3A7BFEFF")]  # blue channel 41 away
+        assert scheduler._match_filaments_to_slots([req()], loaded) == [5]
+
+    def test_ties_keep_the_caller_order_so_prefer_lowest_still_decides(self, scheduler):
+        """Two spools equally close: the incoming order wins, which is the
+        prefer-lowest sort when that preference is on."""
+        loaded = [tray(2, "3A7BD0FF"), tray(7, "3A7BDAFF")]  # both 5 away
+        assert scheduler._match_filaments_to_slots([req()], loaded) == [2]
+        assert scheduler._match_filaments_to_slots([req()], list(reversed(loaded))) == [7]
+
+    def test_nearest_applies_within_a_shared_tray_info_idx_too(self, scheduler):
+        """The tray_info_idx subset walks its own colour comparison."""
+        loaded = [
+            tray(1, FAR_BUT_ADMITTED) | {"tray_info_idx": "GFG99"},
+            tray(3, NEAR) | {"tray_info_idx": "GFG99"},
+        ]
+        assert scheduler._match_filaments_to_slots([req(tray_info_idx="GFG99")], loaded) == [3]
+
+    def test_type_is_still_a_hard_filter(self, scheduler):
+        """A perfect colour in the wrong material never wins."""
+        loaded = [tray(1, "3A7BD5FF", type_="ASA"), tray(3, NEAR)]
+        assert scheduler._match_filaments_to_slots([req()], loaded) == [3]
+
+    def test_each_slot_consumes_its_tray(self, scheduler):
+        """Two slots wanting the same colour take different spools, nearest first."""
+        loaded = [tray(1, FAR_BUT_ADMITTED), tray(3, NEAR)]
+        mapping = scheduler._match_filaments_to_slots([req(slot_id=1), req(slot_id=2)], loaded)
+        assert mapping == [3, 1]

+ 77 - 0
frontend/src/__tests__/utils/nearestColourMatch.test.ts

@@ -0,0 +1,77 @@
+/**
+ * Regression tests for slot-order-dependent colour matching (#2804), frontend side.
+ *
+ * Three matchers in the interface fell back to "first spool within tolerance"
+ * when no exact colour was loaded, so the winner depended on AMS slot order.
+ * They now share `findNearestSimilar` with the scheduler's equivalent, so the
+ * dialog cannot promise a spool the backend would not have picked.
+ *
+ * The worked example is the maintainer's: required `#3A7BD5`, a purple
+ * `#6253AD` in tray 1 (40/40/40 off — admitted, distance ~69) and a
+ * near-identical `#3B7AD2` in tray 3 (distance ~3).
+ */
+
+import { describe, expect, it } from 'vitest';
+import { colorDistance, colorsAreSimilar, findNearestSimilar } from '../../utils/amsHelpers';
+
+const REQUIRED = '#3A7BD5';
+const NEAR = '3B7AD2FF';
+const FAR_BUT_ADMITTED = '6253ADFF';
+
+const tray = (globalTrayId: number, color: string) => ({ globalTrayId, color });
+const pick = (candidates: { globalTrayId: number; color: string }[], required = REQUIRED) =>
+  findNearestSimilar(candidates, required, (c) => c.color)?.globalTrayId;
+
+describe('colorDistance', () => {
+  it('is zero for the same colour', () => {
+    expect(colorDistance('#3A7BD5', '3A7BD5FF')).toBe(0);
+  });
+
+  it('ignores alpha so a transparent filament matches itself', () => {
+    expect(colorDistance('#76D9F4', '76D9F400')).toBe(0);
+  });
+
+  it('is euclidean rather than per-channel', () => {
+    expect(colorDistance('#000000', '#282828')).toBeCloseTo(69.28, 1);
+  });
+
+  it('returns null for unusable input rather than a number', () => {
+    expect(colorDistance(undefined, REQUIRED)).toBeNull();
+    expect(colorDistance('', REQUIRED)).toBeNull();
+    expect(colorDistance('#abc', REQUIRED)).toBeNull();
+  });
+});
+
+describe('findNearestSimilar', () => {
+  it('picks the closest admitted colour, not the first one', () => {
+    expect(pick([tray(1, FAR_BUT_ADMITTED), tray(3, NEAR)])).toBe(3);
+  });
+
+  it('gives the same answer whichever order the trays arrive in', () => {
+    expect(pick([tray(1, FAR_BUT_ADMITTED), tray(3, NEAR)])).toBe(3);
+    expect(pick([tray(3, NEAR), tray(1, FAR_BUT_ADMITTED)])).toBe(3);
+  });
+
+  it('keeps the incoming order on a tie, so prefer-lowest still decides', () => {
+    // Both 5 away — whichever the caller put first wins.
+    expect(pick([tray(2, '3A7BD0FF'), tray(7, '3A7BDAFF')])).toBe(2);
+    expect(pick([tray(7, '3A7BDAFF'), tray(2, '3A7BD0FF')])).toBe(7);
+  });
+
+  it('admits exactly what the tolerance admitted before', () => {
+    // 41 off on one channel is outside the box and must not be ranked in.
+    expect(colorsAreSimilar('3A7BFEFF', REQUIRED)).toBe(false);
+    expect(pick([tray(5, '3A7BFEFF')])).toBeUndefined();
+    // 40 off on all three is inside it, as it was.
+    expect(colorsAreSimilar(FAR_BUT_ADMITTED, REQUIRED)).toBe(true);
+    expect(pick([tray(5, FAR_BUT_ADMITTED)])).toBe(5);
+  });
+
+  it('returns undefined when nothing qualifies', () => {
+    expect(pick([tray(1, 'FF0000FF')])).toBeUndefined();
+  });
+
+  it('skips candidates with no usable colour instead of throwing', () => {
+    expect(pick([{ globalTrayId: 1, color: '' }, tray(3, NEAR)])).toBe(3);
+  });
+});

+ 9 - 8
frontend/src/hooks/useFilamentMapping.ts

@@ -4,6 +4,7 @@ import {
   normalizeColor,
   normalizeColorForCompare,
   colorsAreSimilar,
+  findNearestSimilar,
   formatSlotLabel,
   getGlobalTrayId,
   preferLowestSortKey,
@@ -333,10 +334,10 @@ export function buildFilamentComparison(
             normalizeColorForCompare(f.color) === normalizeColorForCompare(req.color)
         );
         if (!exactMatch) {
-          similarMatch = idxMatches.find(
-            (f) =>
-              f.type?.toUpperCase() === req.type?.toUpperCase() &&
-              colorsAreSimilar(f.color, req.color)
+          similarMatch = findNearestSimilar(
+            idxMatches.filter((f) => f.type?.toUpperCase() === req.type?.toUpperCase()),
+            req.color,
+            (f) => f.color,
           );
         }
         if (!exactMatch && !similarMatch) {
@@ -355,10 +356,10 @@ export function buildFilamentComparison(
           normalizeColorForCompare(f.color) === normalizeColorForCompare(req.color)
       );
       if (!exactMatch) {
-        similarMatch = available.find(
-          (f) =>
-            f.type?.toUpperCase() === req.type?.toUpperCase() &&
-            colorsAreSimilar(f.color, req.color)
+        similarMatch = findNearestSimilar(
+          available.filter((f) => f.type?.toUpperCase() === req.type?.toUpperCase()),
+          req.color,
+          (f) => f.color,
         );
       }
       if (!exactMatch && !similarMatch) {

+ 9 - 8
frontend/src/hooks/useMultiPrinterFilamentMapping.ts

@@ -11,6 +11,7 @@ import {
 import {
   normalizeColorForCompare,
   colorsAreSimilar,
+  findNearestSimilar,
   preferLowestSortKey,
   compareSortKeys,
   effectivePreferLowest,
@@ -156,10 +157,10 @@ function computeMatchDetails(
     );
     const similarMatch = exactMatch
       ? undefined
-      : candidates.find(
-          (f) =>
-            f.type?.toUpperCase() === req.type?.toUpperCase() &&
-            colorsAreSimilar(f.color, req.color)
+      : findNearestSimilar(
+          candidates.filter((f) => f.type?.toUpperCase() === req.type?.toUpperCase()),
+          req.color,
+          (f) => f.color,
         );
     const typeOnlyMatch =
       exactMatch || similarMatch
@@ -245,10 +246,10 @@ function computeMappingWithOverrides(
     );
     const similarMatch = exactMatch
       ? undefined
-      : candidates.find(
-          (f) =>
-            f.type?.toUpperCase() === req.type?.toUpperCase() &&
-            colorsAreSimilar(f.color, req.color)
+      : findNearestSimilar(
+          candidates.filter((f) => f.type?.toUpperCase() === req.type?.toUpperCase()),
+          req.color,
+          (f) => f.color,
         );
     const typeOnlyMatch =
       exactMatch || similarMatch

+ 70 - 5
frontend/src/utils/amsHelpers.ts

@@ -116,6 +116,70 @@ export function colorsAreSimilar(
   );
 }
 
+/**
+ * Euclidean RGB distance between two hex colours, or null if either is unusable.
+ *
+ * Used to rank the candidates `colorsAreSimilar` admits. Eligibility stays the
+ * per-channel box that shipped; this only decides which of several eligible
+ * spools is closest, so no spool becomes usable or unusable because of it.
+ *
+ * Alpha is dropped by `normalizeColorForCompare`, deliberately: the alpha a
+ * slicer writes for a transparent filament is not a colour the user chose, and
+ * counting it would stop a transparent filament matching itself.
+ */
+export function colorDistance(
+  color1: string | undefined,
+  color2: string | undefined,
+): number | null {
+  const hex1 = normalizeColorForCompare(color1);
+  const hex2 = normalizeColorForCompare(color2);
+  if (!hex1 || !hex2 || hex1.length < 6 || hex2.length < 6) return null;
+
+  const dr = parseInt(hex1.substring(0, 2), 16) - parseInt(hex2.substring(0, 2), 16);
+  const dg = parseInt(hex1.substring(2, 4), 16) - parseInt(hex2.substring(2, 4), 16);
+  const db = parseInt(hex1.substring(4, 6), 16) - parseInt(hex2.substring(4, 6), 16);
+  if (Number.isNaN(dr) || Number.isNaN(dg) || Number.isNaN(db)) return null;
+
+  return Math.sqrt(dr * dr + dg * dg + db * db);
+}
+
+/**
+ * The closest colour match among `candidates`, or undefined if none is similar
+ * enough to qualify.
+ *
+ * Callers pass candidates in the order they already established — slot order,
+ * or the "prefer lowest remaining" sort. Ties keep the earliest of them, so
+ * that order survives as the tie-break and Prefer Lowest still decides between
+ * two equally close spools, which is the case it was actually for.
+ *
+ * This exists so the four matchers that pick a spool (`autoMatchFilament`,
+ * `computeAmsMapping`, `computeMappingWithOverrides`, `computeMatchDetails`)
+ * share one ranking rule instead of four copies of "first one within
+ * tolerance", which made the winner depend on AMS slot order.
+ */
+export function findNearestSimilar<T>(
+  candidates: T[],
+  requiredColor: string | undefined,
+  getColor: (candidate: T) => string | undefined,
+): T | undefined {
+  let best: T | undefined;
+  let bestDistance = Infinity;
+
+  for (const candidate of candidates) {
+    const color = getColor(candidate);
+    if (!colorsAreSimilar(color, requiredColor)) continue;
+    const distance = colorDistance(color, requiredColor);
+    if (distance === null) continue;
+    // Strict <: an equally close candidate never displaces an earlier one.
+    if (distance < bestDistance) {
+      best = candidate;
+      bestDistance = distance;
+    }
+  }
+
+  return best;
+}
+
 /**
  * Format slot label for display in the UI.
  * @param amsId - AMS unit ID (0-3 for regular AMS, 128+ for AMS-HT)
@@ -319,11 +383,12 @@ export function autoMatchFilament(
   );
   const similarMatch = exactMatch
     ? undefined
-    : nozzleFilaments.find(
-        (f) =>
-          !usedTrayIds.has(f.globalTrayId) &&
-          filamentTypesCompatible(f.type, req.type) &&
-          colorsAreSimilar(f.color, req.color)
+    : findNearestSimilar(
+        nozzleFilaments.filter(
+          (f) => !usedTrayIds.has(f.globalTrayId) && filamentTypesCompatible(f.type, req.type),
+        ),
+        req.color,
+        (f) => f.color,
       );
   const typeOnlyMatch =
     exactMatch || similarMatch

Niektóre pliki nie zostały wyświetlone z powodu dużej ilości zmienionych plików