Browse Source

Stop the G-code preview from sizing the box that sizes it (issue #2887)

Opening a 3D Preview from Archives left an empty white pane with the
legend and the layer slider drawn over it, and the page's scrollbar
shrank for as long as it stayed open -- about 190px of page height a
second, with no limit.

The viewer appends its canvas into the very element it measures with
clientWidth/clientHeight and watches with a ResizeObserver. setSize
writes each new size onto the canvas as inline style, and three.js
leaves the canvas display:inline, so the line box adds descender space
on top of the height just set. Where that element takes its height from
its contents, the canvas sizes the box that sizes the canvas and gains a
fixed 33px every round -- the reporter measured container = canvas + 33
on every sample.

The page gave it no height to take instead. The viewer pane is flex-1
min-h-0, which divides nothing unless the column above it is a definite
height, and h-full is a percentage resolved against a main area whose
own height comes from a min-height -- a floor, not a size. So it fell
through to the content, and the content was the canvas.

Nothing was ever drawn because of the same loop, not a second fault:
every observer callback reallocated and cleared the frame buffer, and an
antialiased render of what had grown to roughly 18 megapixels never
finished before the next one arrived. The data path was fine throughout,
which the legend and the 1..57 layer slider both prove -- they are built
from the parsed toolpath.

The canvas is now positioned out of flow, so it cannot contribute to the
height of the element that measures it on any page, and that element
takes a definite height from the pane around it rather than a
percentage. display:block goes on too, for the case where something
overrides the positioning. The page is sized from the viewport the way
the File Manager page already was.

Either change alone stops the growth, but the structural one alone would
trade it for a collapsed pane: an out-of-flow canvas contributes nothing
to content height, so with no definite height above it the pane becomes
clientHeight || 1. They belong together.

The same viewer in the File Manager dialog was never affected -- a
dialog gives it a fixed height, so neither fault could arise there.

jsdom does no layout, so the loop cannot be reproduced in a test. The
structure that forbids it can: the new cases assert the canvas is out of
flow, that the measured element is definite-height rather than h-full,
and that the pane stays positioned so inset-0 resolves against it.
Reverting either change fails exactly those.
maziggy 2 weeks ago
parent
commit
3916db822c

+ 1 - 0
CHANGELOG.md

@@ -8,6 +8,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The K value is on the AMS slot itself, not only in the popover (#2532, requested and contributed by @gyrene2083)** — Reading back a slot's pressure-advance value meant hovering it: the K factor lived in the filament popover alone, so checking whether a calibration had actually taken across four slots was four hovers, and comparing two of them side by side was not possible at all. Every slot card now carries the value under the material name, the way Bambu Studio shows it per slot — on regular AMS units, on AMS-HT, and on the external spool of a dual-nozzle machine. Only a value the printer actually reported is shown: a loaded but never-calibrated slot stays blank rather than inheriting the 0.020 that fills the popover's own field, and a slot the firmware reports as exactly 0 counts as uncalibrated the same way the stored K-profiles do. The label is shortened to **K** with the full localized name on hover, because "K Factor", "K-Faktor" and "Facteur K" ate the value itself — the whole point of the line — on cards under about 350px, and the figure is set in tabular numerals so it measures the same in Safari as in Chromium. Where one slot of a unit is calibrated and its neighbours are not, the neighbours hold the same row open so the fill bars stay level across the card.
 
 ### Fixed
+- **The full-page G-code preview grew without limit and never drew anything (#2887, reported by @ojimpo)** — Opening a 3D Preview from Archives left an empty white pane with the legend and layer slider floating over it, while the page's scrollbar shrank for as long as it stayed open — about 190px of page height per second, without stopping. Two faults compounded. The viewer appends its canvas into the very element it measures and watches for resizes, and three.js writes each new size onto the canvas as inline style, leaving it `display: inline` so the line box adds descender space on top; on a page where that element takes its height from its contents, the canvas was sizing the box that sizes the canvas, gaining a fixed 33px every round. And the page never gave it a height to take instead: the viewer pane is `flex-1 min-h-0`, which divides nothing unless the column above it is a definite height, and `h-full` is a percentage that resolves against a main area whose own height comes from a `min-height` — a floor, not a size — so it fell through to the content. Nothing was ever drawn because each observer callback reallocated and cleared the frame buffer before an antialiased render of what had grown to roughly 18 megapixels could finish. The canvas is now positioned out of flow, so it cannot contribute to the height of the element that measures it on any page, present or future, and that element takes a definite height from the pane around it rather than a percentage; the page itself is sized from the viewport the same way the File Manager page already was. The same viewer inside the File Manager dialog was never affected — a dialog gives it a fixed height, so neither fault could arise there. Nothing was wrong with the data path at any point: the legend and the layer slider were built from the parsed toolpath throughout, so the fetch, the parse and the layer split had all succeeded.
 - **A print could not start on the nozzle sitting in the H2C's rack (#2885, reported by @apizz)** — A job sliced for a 0.2mm nozzle failed in the queue with "install the matching nozzle before printing", even though a 0.2mm nozzle was in the tool-changer rack and the printer would have fetched it. Only picking the nozzle up by hand on the printer's own screen first let the print run, and because the item failed rather than waited, the rest of the queue went with it. The reporter noticed the giveaway: going *to* 0.4mm always worked, going *to* 0.2mm never did. The nozzle-diameter guard that catches a genuinely wrong slice before upload was measuring the wrong thing — it compared the sliced diameter against the two mounted hotends only, so on a machine whose hotends both read 0.4mm nothing but 0.4mm could ever pass, and the rack picker that would have fetched the right one runs further down the same dispatch and never got the chance. The rack now counts as reachable: a diameter parked in any dock satisfies the guard the same way a mounted one does. This was never only about 0.2mm — a 0.6mm slice was blocked identically. A diameter that is in neither a hotend nor a dock still stops the print before it uploads, and the message now lists both sets so it is clear what the machine actually has. Also fixed alongside it: an empty carriage keeps reporting the diameter of the nozzle it last held, so a hotend that had parked its nozzle back in the rack was counted as a mounted 0.4mm that was not there — presence is now read from the nozzle's own temperature rating and serial number, and a hotend is only discarded when both agree it is empty. Printers that report no rack at all are unaffected.
 - **An AMS slot could name the wrong white** — A hex is not one colour in Bambu's range: `#FFFFFF` is Jade White in PLA Basic, Ivory White in PLA Matte and plain White in six other materials, and `#000000` is Charcoal in PLA Matte where it is Black everywhere else. The slot popover looked the colour up by hex alone, and the lookup table can only keep one name per hex — so an ivory Matte spool was titled "Jade White" even while the profile line beside it correctly read Matte Ivory. The colour map now also carries the names that collapsing loses, keyed by material, and every slot resolves its colour with the material the printer reports for it (`tray_sub_brands`). Slots with a spool assigned from Inventory are titled with that spool's own colour name, which is the roll the user actually put in. Only a name the same brand's own range lost is carried — one manufacturer's name must not displace another's — so the added map is 11 entries against the 608 in the shipped catalog.
 - **A queued job switched on printers that could never have printed it** — When no printer of the target model was available, the queue powered one on via its smart plug, but chose it on model alone: it walked the farm in printer-ID order, woke the first machine with an Auto On plug, and only then discovered the loaded filament was the wrong colour. A job for a colour loaded at the far end of the farm therefore woke every earlier printer in turn and left each one running until its own auto-power-off timer expired. The colours were known the whole time — a printer keeps its last reported AMS and external-spool trays after the power goes — so the wake step now asks the same three questions the matcher asks a live printer (required types, forced colours, preferred colours) and passes over a printer whose last known filament cannot satisfy the job. A printer Bambuddy has never heard from is still woken: no reading is not the same as no filament. Reconnecting a printer also no longer discards that reading, so a power-on attempt that times out stops erasing what the next attempt needs.

+ 115 - 0
frontend/src/__tests__/components/GcodeToolpathViewerSizing.test.tsx

@@ -0,0 +1,115 @@
+/**
+ * How the toolpath viewer is sized (#2887).
+ *
+ * The viewer appends its canvas into the same element it measures with
+ * `clientWidth`/`clientHeight` and watches with a ResizeObserver. If that
+ * element can take its height from its contents, the canvas ends up sizing the
+ * box that sizes the canvas: every resize grew the container, which fired the
+ * observer, which resized again. On `/gcode-viewer` the page climbed about
+ * 190px a second and never rendered a frame, because a fresh ~18-megapixel
+ * buffer was allocated and cleared before the previous one could finish.
+ *
+ * jsdom does no layout, so the loop itself cannot be reproduced here. What can
+ * be pinned down is the structure that makes it impossible: the canvas is out
+ * of flow, so it contributes nothing to its parent's height, and the measured
+ * element takes a definite height from the pane above it rather than a
+ * percentage that resolves to `auto`.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render } from '@testing-library/react';
+
+// Everything three.js does here is arithmetic except creating a GL context,
+// which jsdom has no answer for -- so only WebGLRenderer is replaced. Keeping
+// Scene/PerspectiveCamera/GridHelper real means the component runs its actual
+// setup path rather than one written for the test.
+vi.mock('three', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('three')>();
+  class FakeWebGLRenderer {
+    domElement = document.createElement('canvas');
+    setSize(width: number, height: number) {
+      // Mirror three.js: the size is written onto the canvas as inline style,
+      // which is what fed back into the container's height.
+      this.domElement.style.width = `${width}px`;
+      this.domElement.style.height = `${height}px`;
+    }
+    setPixelRatio() {}
+    render() {}
+    dispose() {}
+  }
+  return { ...actual, WebGLRenderer: FakeWebGLRenderer };
+});
+
+vi.mock('three/examples/jsm/controls/OrbitControls.js', () => ({
+  OrbitControls: class {
+    enableDamping = false;
+    dampingFactor = 0;
+    update() {}
+    dispose() {}
+  },
+}));
+
+import { GcodeToolpathViewer } from '../../components/GcodeToolpathViewer';
+
+const canvas = () => document.querySelector('canvas') as HTMLCanvasElement;
+
+describe('GcodeToolpathViewer sizing', () => {
+  beforeEach(() => {
+    // Hold the component in its loading state so the canvas stays mounted;
+    // an error would swap the whole tree for a message.
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(() => new Promise(() => {})),
+    );
+    vi.stubGlobal(
+      'ResizeObserver',
+      class {
+        observe() {}
+        unobserve() {}
+        disconnect() {}
+      },
+    );
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  it('puts the canvas out of flow so it cannot size its own container', () => {
+    render(<GcodeToolpathViewer gcodeUrl="/api/v1/archives/224/gcode" />);
+
+    const element = canvas();
+    expect(element).toBeTruthy();
+    expect(element.style.position).toBe('absolute');
+    // An in-flow canvas is `display: inline` by default, and the line box then
+    // adds descender space on top of the height just set -- the ~33px per round
+    // that drove the growth.
+    expect(element.style.display).toBe('block');
+    expect(element.style.inset).toBe('0');
+  });
+
+  it('measures an element with a definite height, not a percentage one', () => {
+    render(<GcodeToolpathViewer gcodeUrl="/api/v1/archives/224/gcode" />);
+
+    const measured = canvas().parentElement as HTMLElement;
+    // `h-full` is `height: 100%`, which resolves to `auto` unless every ancestor
+    // has a definite height -- on the full-page route none does. `inset-0`
+    // against the positioned pane is definite whatever the page does.
+    expect(measured.className).toContain('absolute');
+    expect(measured.className).toContain('inset-0');
+    expect(measured.className).not.toContain('h-full');
+  });
+
+  it('keeps the measured element inside a positioned pane', () => {
+    const { container } = render(
+      <GcodeToolpathViewer gcodeUrl="/api/v1/archives/224/gcode" className="flex-1 min-h-0" />,
+    );
+
+    // `inset-0` only means anything against a positioned ancestor; without the
+    // `relative` pane the canvas would escape to the viewport.
+    const pane = container.firstElementChild as HTMLElement;
+    expect(pane.className).toContain('relative');
+    expect(pane.className).toContain('flex-1');
+    expect(pane.contains(canvas())).toBe(true);
+  });
+});

+ 12 - 0
frontend/src/__tests__/pages/GCodeViewerPage.test.tsx

@@ -90,6 +90,18 @@ describe('GCodeViewerPage', () => {
     expect(screen.getByText(/No file was given/i)).toBeInTheDocument();
   });
 
+  it('gives the viewer pane a height to fill (#2887)', () => {
+    // The viewer is `flex-1 min-h-0`, which divides nothing unless this column
+    // has a definite height. `h-full` did not provide one -- it resolves against
+    // `<main>`, whose height comes from `flex-1` under a `min-h-screen` root, so
+    // the percentage fell through to content and the canvas grew the page it was
+    // measured against, without limit.
+    visit('?archive=82');
+    const column = screen.getByTestId('toolpath-viewer').parentElement as HTMLElement;
+    expect(column.className).not.toContain('h-full');
+    expect(column.className).toContain('h-[calc(100vh-64px)]');
+  });
+
   it('offers a way back to where the file came from', () => {
     visit('?archive=82');
     expect(screen.getByRole('button', { name: /Back to Print Archives/i })).toBeInTheDocument();

+ 25 - 1
frontend/src/components/GcodeToolpathViewer.tsx

@@ -256,6 +256,19 @@ export function GcodeToolpathViewer({
     const renderer = new THREE.WebGLRenderer({ antialias: true });
     renderer.setSize(width, height);
     renderer.setPixelRatio(Math.min(window.devicePixelRatio, 2));
+    // Take the canvas out of flow before it is ever in the document (#2887).
+    // `setSize` writes the size onto the canvas as inline width/height, and the
+    // canvas lives inside the very element we measure and observe — so on a page
+    // where that element's height comes from its content, each resize grew the
+    // container, which fired the observer, which resized again. three.js leaves
+    // the canvas `display: inline`, so the line box added its descender space
+    // (~33px) every round and the page climbed without limit. Out of flow it
+    // cannot contribute to the container's height at all; `display: block` is
+    // belt and braces for the same descender, and matters if this is ever
+    // rendered somewhere the absolute positioning is overridden.
+    renderer.domElement.style.display = 'block';
+    renderer.domElement.style.position = 'absolute';
+    renderer.domElement.style.inset = '0';
     container.appendChild(renderer.domElement);
 
     const controls = new OrbitControls(camera, renderer.domElement);
@@ -437,7 +450,18 @@ export function GcodeToolpathViewer({
 
   return (
     <div className={`relative ${className}`}>
-      <div ref={containerRef} className="w-full h-full" />
+      {/*
+        Absolute, not `w-full h-full` (#2887). The canvas is appended here and
+        this element is what the ResizeObserver watches, so its height must come
+        from the pane above it and never from what it contains. `h-full` is a
+        percentage, which resolves to `auto` unless every ancestor has a definite
+        height — on the full-page route none does, so the height fell through to
+        the content and the canvas ended up sizing the box that sizes the canvas.
+        `inset-0` against the `relative` parent is a definite height whatever the
+        page does, which also keeps this working if a future caller forgets to
+        give the pane a height of its own.
+      */}
+      <div ref={containerRef} className="absolute inset-0" />
 
       {loading && (
         <div className="absolute inset-0 flex items-center justify-center gap-2 bg-bambu-dark/60 text-sm text-bambu-gray">

+ 8 - 1
frontend/src/pages/GCodeViewerPage.tsx

@@ -116,8 +116,15 @@ export function GCodeViewerPage() {
     ? t('gcodeViewer.backToArchives', 'Back to Archives')
     : t('gcodeViewer.backToFiles', 'Back to File Manager');
 
+  // The viewer pane is `flex-1 min-h-0`, which only means anything if this
+  // column has a height to divide. `h-full` did not give it one: it resolves
+  // against `<main>`, whose own height comes from `flex-1` under a
+  // `min-h-screen` root -- a minimum, so the height property stays `auto` and
+  // the percentage falls through to content (#2887). Same viewport-minus-header
+  // pattern the File Manager page uses, with `min-h` keeping the toolbar
+  // reachable on short screens where a hard height would clip it.
   return (
-    <div className="flex flex-col h-full">
+    <div className="flex flex-col min-h-[calc(100vh-64px)] lg:h-[calc(100vh-64px)]">
       <div className="flex-shrink-0 px-4 py-2 border-b border-bambu-dark-tertiary flex flex-wrap items-center gap-x-4 gap-y-2">
         <button
           type="button"

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-CVHHvKY8.js


File diff suppressed because it is too large
+ 1 - 0
static/assets/index-DynWy-72.css


File diff suppressed because it is too large
+ 0 - 1
static/assets/index-kSJGQrMr.css


+ 2 - 2
static/index.html

@@ -26,8 +26,8 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-BumpPKBp.js"></script>
-    <link rel="stylesheet" crossorigin href="/assets/index-kSJGQrMr.css">
+    <script type="module" crossorigin src="/assets/index-CVHHvKY8.js"></script>
+    <link rel="stylesheet" crossorigin href="/assets/index-DynWy-72.css">
   </head>
   <body>
     <div id="root"></div>

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