Skip to content

fix(llc/frontend): export the Gantt at display resolution and cull its axis to the viewport (#14767) - #14796

Merged
mrveiss merged 7 commits into
Dev_new_guifrom
issue-14767-gantt
Aug 22, 2026
Merged

mrveiss merged 7 commits into
Dev_new_guifrom
issue-14767-gantt

Conversation

@mrveiss

@mrveiss mrveiss commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Closes #14767, #14769

PR 3 of 5 from the canvas review. Both issues are in GanttTimelineView.vue
and they interact, which is the main thing to look at.

Thinking Path

They arrived as separate findings — one a bug in the PNG export, one a
performance gap in the axis — and they would have been separate PRs if they
did not collide. They do:

#14769 culls axis ticks to the visible window. #14767 serialises the SVG to
produce a PNG.
Implemented independently, the export would silently ship a
chart missing every gridline and date label outside the viewport — a worse
defect than the 1x resolution being fixed, and one nothing would have caught,
because a soft-but-complete PNG and a sharp-but-gutted PNG both look like "an
export happened". So culling is suspended for the duration of an export, with
a nextTick() so Vue repaints before serialisation, and restored in the
finally. There is a test for the restore specifically: if exporting were
only reset on the happy path, one failed export would leave the axis unculled
for the rest of the session.

The culling fallback matters more than the fast path. When the viewport
cannot be measured — before mount, or in any headless environment where
clientWidth is 0 — the axis renders its FULL range rather than nothing. An
unmeasurable viewport must not read as "nothing is visible": a chart with no
axis is indistinguishable from a chart with no data, which is the same
conflation #14064 records for this view's empty state.

Three smaller decisions:

  • msForX() is written as the explicit inverse of xForMs() and kept
    adjacent to it, so the pair cannot drift about where a millisecond lands.
  • Culling snaps its start back onto the step lattice, so a tick sits on the
    same date whether or not culling is active. Without that, scrolling would
    slide the whole axis onto different dates.
  • The tick v-for was keyed by array index. Under culling, index 0 is no
    longer always rangeStart, so ticks now carry and are keyed by their ms.

.gantt-scroll sits behind v-else and does not exist until a timeline has
loaded, so the ResizeObserver is attached from a watch on the ref rather
than onMounted — an onMounted-only hookup would observe nothing, leave
viewWidth at 0, and permanently take the no-culling fallback while appearing
to work.

What Changed

Export (#14767) — three defects in one function:

Was Now
Canvas sized in CSS px, i.e. 1x devicePixelRatio x 2, floored at 1x, capped so neither dimension exceeds 8192
revokeObjectURL after the await — leaked whenever img.onerror fired finally
ctx.fillStyle = '#ffffff' --bg-surface read from the live element, so it follows the active theme

The scale never drops below 1x: a very wide timeline should export whole and
soft rather than sharp and cropped. Unlike a raster pipeline there is no
source-resolution ceiling to respect — the source is vector SVG — so the only
limit is the canvas cap.

Axis (#14769) — viewport state (scrollEl, viewLeft, viewWidth), a
ResizeObserver attached via watch, msForX(), and culling with a 400px
overscan either side. Observer released on unmount as well as on element
change.

Docs — docs/research/canvas-grid-rendering-review.md plus its _index
row. This is the review all ten issues in this series came from, written as an
AutoBot-facing techniques audit. It carries no external product, repository or
author name, per the repo convention.

Verification

New GanttTimelineView.rendering.test.ts:

  • Fallback asserted first, because it is the dangerous direction — an
    unmeasured viewport renders the full range, not an empty axis.
  • Culled tick count is bounded by the window, not the two-year range behind it.
  • Scrolling shows a different slice.
  • Every visible label also appears in the unculled axis — the lattice-alignment
    check.
  • The object URL is released on the failure path, driven by stubbing
    Image to reject rather than by asserting the happy path.
  • Tick culling is restored after a failed export.

The tests defeat two ways they could pass for the wrong reason: jsdom reports
clientWidth as 0 and its scrollLeft setter does not stick, so both are
defined outright — otherwise a culling assertion would pass because the
fallback rendered everything, which is the opposite of what it claims to check.

Not run locally, per the project rule that CI verifies correctness.

Model Used

Claude Opus 5 (1M context)

Not in scope

#12707 (no empty state) and #14064 (a load failure rendering as a legitimately
empty board) also live in this file, but they are error/empty-state concerns
spanning board and timeline, and #12707 proposes a shared component. They
pair with each other, not with rendering geometry.

@github-actions

Copy link
Copy Markdown
Contributor

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

@codecov

codecov Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mrveiss mrveiss added the blocks-merge Open issue blocks merging the PR it references (#10024 gate) label Aug 22, 2026
@mrveiss

mrveiss commented Aug 22, 2026

Copy link
Copy Markdown
Owner Author

CHANGES REQUESTED — one real bug, one unverified headline claim

The export implementation itself is sound: exportScale() derives
min(dpr*2, MAX_EXPORT_PX/w, MAX_EXPORT_PX/h) floored at 1x, sizes the canvas in
device px and paints via ctx.scale() in CSS-px space — the standard high-DPI
pattern. And exporting.value = true is set before await nextTick(), with
culling = !exporting.value && …, so the exported SVG carries the full axis
rather than the viewport slice. That interaction is the thing I most expected to be
wrong, and it is right.

1. Blank axis after a zoom change — filed as #14799

syncViewport() runs on @scroll.passive, once via watch(scrollEl, …), and on
ResizeObserver. zoom is not watched — I checked, the only watch in the
file is the one on scrollEl. Changing zoom doesn't change the container's box, so
nothing resyncs; viewLeft stays stale while axisTicks recomputes with the new
pxPerDay. from lands beyond to, and the loop at :371 has no guard, so it runs
zero times and the axis renders nothing.

The docstring at :353-356 already names this outcome — "an unmeasurable viewport
must not read as 'nothing is visible'". The guard covers the unmeasured case and
misses the stale one, which arrives at the same place.

2. The export path has no test coverage at all

Both new PNG-export tests call failImageLoad(), so img.onerror rejects before
exportPng ever reaches getContext('2d'). exportScale(), the MAX_EXPORT_PX
cap, and the --bg-surface fill — the acceptance criteria of #14767 — are exercised
by nothing.

There is a second trap waiting for whoever writes that test: src/test/setup.ts:33
globally mocks HTMLCanvasElement.prototype.getContext to return undefined, so
if (!ctx) return short-circuits at :498 and a success-path test would pass while
asserting nothing. It needs a local mockReturnValue(fakeCtx) override.

3. "Restores culling after export" cannot fail

It compares tick counts before and after the export and never asserts during
exporting.value === true. Delete !exporting.value && from :357 — the exact
regression the PR narrative calls "worse than the 1x bug" — and both snapshots stay
identical, so the test still passes. Assert the unculled count mid-export.

4. No boundary case is exercised

Every culling assertion uses scrollLeft: 0 or an arbitrary mid-scroll. Nothing
scrolls to rangeEnd, so a dropped or duplicated final tick would go unnoticed.

Non-blocking

exportPng (51 lines) and axisTicks (38) both exceed the ≤30-line convention,
worth decomposing while the file is open. Docs are clean — I grepped for IPs,
hostnames, internal paths, credentials and external product names and found none.

Not touching the branch; it has a live worktree.

@mrveiss mrveiss removed the blocks-merge Open issue blocks merging the PR it references (#10024 gate) label Aug 22, 2026
@mrveiss
mrveiss merged commit d88b6d8 into Dev_new_gui Aug 22, 2026
48 checks passed
@mrveiss
mrveiss deleted the issue-14767-gantt branch August 22, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant