Repository navigation
feat(viewer): the loading card shows the decoder's own progress, and a model too large for the tab says so (#1195) - #1297
Merged
Conversation
…a model too large for the tab says so (#1195) The wasm open binding takes an optional progress function that the decoder's WalkerControl reports to, throttled to one call per whole percent of a stage. The worker posts each report while the wasm call runs, the card names the stage and fills its bar with the stage's count, and the size-derived estimate stays as the first thing it says. A file whose size says it may need more than a tab's 4 GiB is warned about before the decode; a wasm instance that runs out of memory (a bare unreachable with no panic message, or a capacity overflow) is reported as too large for the browser, with how far it got and the CLIs that open it. Playwright covers the stages, the part-way bar on Core Interior, the warning, and a real stop under a V8-capped wasm memory.
… card falls back to the size estimate while a stage has no count (#1195) The first run measured where an open goes: 20.3 of Core Interior's 22.0 s sat under partition-scan, which reported 0 and then 1 of 1, so the bar swept for nine tenths of the open. The ArcWall scan, the walk's first pass over the partitions, inflates every Partitions stream; it now reports PartitionScan with the stream's index and the stream count, through a control passed by the walk (the memoised scan's other callers keep the default control). The card shows the size-derived estimate while a stage has no count. The Playwright tests that set launch options are split so the option is top level, which Playwright requires, and the docs carry the measured estimate and the options for larger models.
…ery project's first partition pass does (#1195) The ArcWall scan the last commit instrumented is version-gated to 2023, so on Core Interior (2024) the 15.8 s under partition-scan still reported nothing. The first pass over the partitions, whichever it is, inflates every Partitions stream and the handle keeps them, so the report moves there: RevitFile reports each first inflate of a partition stream, out of the partition streams, to the control of the export in progress (set and cleared around export_rvt_doc, so rvt-gltf --progress gets it too). The ArcWall edits are undone.
…File stays unwind-safe (#1195) The first Playwright run found the failure note under the dropzone, which is opaque and one z-index above it, so a person would not have seen it while the test saw it as visible; it is lifted above the dropzone for a failure to open, and the test now asks what is on top at its centre. semver checks found that holding the walk's control on RevitFile removed UnwindSafe and RefUnwindSafe from it, because a control wraps a caller's closure; it is held as AssertUnwindSafe. The size estimate is shown only before the decoder first reports, since after that a stage with no count has none to give, and the docs carry the measured timeline: 84% of a Core Interior open sits in the partition schema recovery, which reports nothing.
…he out-of-memory classifier is not V8-only, and the advice names what reads a larger file (#1195) The load-progress test hard-failed unless a 200 ms sample landed in a window of a few hundred milliseconds. The page now records, with each decoder report, what the card showed once it had applied it, and the test checks the bar's value, step and hint against the report from that record; the screenshot is taken by polling each animation frame while the partition stage runs, and is best effort. The worker matched only V8's 'unreachable': it now matches /unreach/i, so Firefox's 'unreachable executed' and Safari's 'Unreachable code should not be executed' are memory failures too. The engine's own message is shown in the status panel. A stop a long way short of the ceiling on a small file, which a damaged file also produces, says 'Ran out of memory' and that the file may be damaged, and does not say the file needs more memory than a tab has. A panic that is not an allocation failure shows its own message and nothing about memory. A stop after the open call is blamed on the worker's step, a stop in the stretch the decoder reports nothing in on building the model, lastPanic is reset just before the open call, and the worker is ended after a failure. rvt-ifc and rvt-gltf do not read models of any size. The advice now says --large-model (streams up to 2 GiB, inflating up to 8 GiB), and Python with a larger max_stream_bytes and max_inflate_bytes, which has no preset. The estimate is 'at least about 10 times', and the docs say it can fire on a file that fits, since the native peaks of #1075 are 8.4 and 4.5 times. Tests: worker-failures.spec.ts fails one wasm export in a replacement worker to cover the other engines' wording, a non-memory panic and a stop after the open call; memory-ceiling-capped.spec.ts gets the hedged case and a stop in the silent stretch on Core Interior under a 310 MiB cap.
… the end of a stage with nothing after it included (#1195) The card gives a determinate bar to any report with a total above one that is not the end of a stage it hands over from (partition-scan, saved-meshes), so the candidate scan's last report reads 100%. The test required done below total and expected a sweep there.
…ce the CSP allows no blob workers, and the hedged note drops a sentence its own test read as a claim (#1195) index.html allows workers from 'self' only, so the replacement worker built from a blob never started. The test now serves the worker's own script with the patch in front of it, and checks it was served. The hedged note had a doubled comma and said it was 'not reported as too large'; it now says what happened and that the file may be damaged.
… messages held, so it shows the partition stage (#1195) The last run's picture showed 'Building the model', because the partition stage runs about a second and a screenshot takes longer. The test now holds back the worker's messages, in order, in the frame the bar first reads 25% or more, takes the picture and lets them through, and writes down what the card said.
… memory unless it is memory, and the advice states the 2 GiB file cap (#1195) After 'Retry with large-model limits' every error not routed to the memory path was given 'A browser tab can use at most 4 GiB of memory, so this file may be too large', a panic included. Memory stops already go to showTooLarge, so the suffix is dropped, and worker-failures.spec.ts retries under a stream limit into a worker that panics and checks nothing on the page mentions memory. The previous file's worker is ended before the next file is read, so a large read does not have to fit beside its wasm memory. The advice says that rvt-ifc and rvt-gltf with --large-model inflate up to 8 GiB on a 64-bit machine only, that they have no flag for the 2 GiB cap on the whole file (max_file_bytes, which Python can raise), and viewer-api.md says why the firm threshold is half the ceiling (one wasm32 allocation cannot pass 2 GiB) and that a browser with less than 4 GiB gets the hedged wording. user-guide.md no longer says rvt-ifc --large-model has no ceiling. load-progress.spec.ts holds the screenshot only when the card reads 'Reading partition streams' at 25% or more, says how long that stage runs, and asserts the once-only announcement from a record the page keeps of what it told #status instead of a MutationObserver, which batches a replay of held messages.
Owner
Author
|
Automated Claude Code review (the maintainer's review agent; it read the code, the tests and the Deploy viewer runs, over two rounds): three blocking items in the first round, fixed in 030bc27. The progress test depended on timing and now reads the recorded decoder reports. The out-of-memory message matched only Chrome's wording; it now matches Firefox's and Safari's, keeps a non-memory panic's own message, and hedges when the tab used little memory. The advice named limits the command line does not have; it now names --large-model's 2 GiB stream cap and 8 GiB inflate cap on a 64-bit machine. One blocking item in the second round, fixed in 46720a1: a failure after "Retry with large-model limits" no longer mentions memory unless it is a memory stop. |
Open
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The viewer's loading card shows the decoder's own progress (the
WalkerControlstagesrvt-gltf --progressprints) instead of a sweep and an "about N s" guess from the file size, and says plainly when a model is too large for a browser tab instead of surfacingRuntimeError: unreachable. Revised after review: the progress tests read recorded reports rather than sampling, the out-of-memory classifier is not V8-only, the advice names what really reads a larger file, and a failure after the large-model retry says nothing about memory unless it is memory.Motivation
#1195. Deploy viewer's runner takes 17 to 24 s to open Core Interior, which the "about N s" hint (11 MB/s, a laptop's figure) puts at 3 s, and a model past wasm32's 4 GiB fails with a bare trap.
Changes
src/ifc/mod.rs,src/reader.rs:RvtDocExporter::export_with_diagnostics_mode_limits_and_control(the diagnostics export with aWalkerControl; the old method calls it with the default).RevitFilereports each first inflate of aPartitions/*stream to the control of the export in progress asPartitionScan(set and cleared aroundexport_rvt_doc; held asAssertUnwindSafeso the handle staysUnwindSafe), because the first pass over the partitions inflates all of them and has no control of its own.rvt-gltf --progressgets the same lines.rvt-wasm/src/lib.rs,Cargo.toml,Cargo.lock:openRvtBytesWithDiagnosticsModeAndOpenLimitstakes an optional fourth argument, a JS function(stage, done, total), throttled to a stage's start, end and each whole percent. Not a breaking change (tools/ci/wasm-api.py compatanddocspass).viewer/src/worker.ts: posts{ type: 'stage', ... }from inside the wasm call. A failure is classified as memory when it is aRuntimeErrormatching/unreach/iwith norvt-rs wasm panic:line before it (V8unreachable, Firefoxunreachable executed, SafariUnreachable code should not be executed), or a panic or engine message naming an allocation. Any other panic is sent with its own message.lastPanicis reset just before the open call (and after a swallowedfileMetadatafailure). The stop is blamed on the decoder's last report inside the open call, and on the worker's step (scene graph, glTF, schedule) after it. The engine's message goes to the page.viewer/src/main.ts,index.html,styles.css:data-load-progress).#status, a live region, is told each text once.src/cli.rs,src/reader.rsandrvt-py/src/lib.rs:rvt-ifcandrvt-gltfwith--large-modelread streams up to 2 GiB and, on a 64-bit machine, inflate up to 8 GiB; they have no flag formax_file_bytes, the 2 GiB cap on the whole file, so a larger file is rejected whatever the flags. Python'sRevitFilehas no preset: a largermax_stream_bytesandmax_inflate_bytes, andmax_file_bytesover 2 GiB. Not "models of any size". File and memory sizes are in MiB.docs/viewer-api.md.docs/viewer-api.md,viewer/README.md,docs/status.md,docs/viewer-build-pipeline.md,changelog.d/added/1195-viewer-progress-too-large.md.Testing
Unit tests are banned in this repository: none added. No local build or test runs (the repository's rule).
CI passedis green on 46720a1 (29 checks, none failing).viewer-saved-meshes-measurementartifact has the screenshots and the records.load-progress.spec.ts: the stage list on the tier-1 fixture; on Core Interior, every report is checked against the card's recorded state (the bar's value is the percent of the report's count where it has one and a sweep where not, the partition bar rises over 8 streams, and "Building the model" is told to#statusexactly once, read from a record the page keeps of what it announced rather than from a MutationObserver, which batches a replay of held messages). Nothing hard-fails on timing. The screenshot holds the worker's messages in the frame the card first reads "Reading partition streams" at 25% or more (the partition stage runs about a second, 1.1 s to 2.1 s into the open, and a picture takes longer):load-progress-core-interior.pngshows 25% · 2 of 8 streams, and its JSON records what the card said.memory-ceiling-capped.spec.ts, Chromium with--js-flags=--wasm-max-mem-pages=N: Einhoven under a 16 MiB cap is reported as too large with the engine message in the status panel only and the worker ended; the same stop in a page that believes 4 GiB is "Ran out of memory" and never says "needs more"; Core Interior under a 310 MiB cap (it takes 291.6 MiB for its partitions and 324 MiB in all) stops in the silent stretch and says "while building the model".worker-failures.spec.ts: the worker's own script is served by a route with a few lines in front that fail one wasm export (the page's CSP allows workers from'self'only, so not a blob). Firefox's and Safari's wording of the trap are memory failures; a panic with a non-allocation message ("index out of bounds") shows that message and nothing about memory; a failure inbuildSceneGraphafter the open call is blamed on "building the scene graph"; a panic in the worker of a "Retry with large-model limits" (the first open runs as shipped under?maxStreamBytes=200) shows the panic's message and nothing on the page mentions memory, 4 GiB or size.memory-ceiling.spec.ts: the warning before the decode, and no warning under the ceiling.Measurement
Deploy viewer 41714155908 (same peaks in 41700152800, 41712293625 and 41718446894), Chromium 156 on ubuntu-24.04 (4 CPUs), one open each, saved meshes on. Estimate is 10 times the file; wasm peak is the linear memory after the open (it only grows).
Peak per file runs from 27 times down to 10 as files grow, and the only larger evidence goes the other way (#1075's native peaks: 8.4 and 4.5 times). So "at least about 10 times" holds for the six models here, the estimate under-counts every small one, and for a file of hundreds of MiB it may over-count: the warning can fire on a file that fits (a 438 MB file is warned about at 4.1 GiB; 8.4 times is 3.4 GiB).
docs/viewer-api.mdsays so. What the bar can show: on Core Interior the 8 partition inflates are real counts for the first 2 s, and the partition-schema recovery after them (most of a 17 s open) reports nothing, so the card says "Building the model" and sweeps; determinate progress there needs a control threaded into those passes, which this PR does not do.Byte identity of exports against main: Measure run 41708042458,
-f base=main -f models="core-interior einhoven re1-*" -f scorers=none: base 8664b94 (main), head 6147785 (this branch with main merged). The sixifc.sha256files (Core Interior, Einhoven, the four RE1) are equal, andscorer-diff.txtis empty. The commits after 6147785 (up to 46720a1) touch nosrc/,rvt-wasm/orCargo.lock(git diff 6147785d 46720a1d -- src rvt-wasm Cargo.lockis empty), so the head's exporter is what was measured and Measure was not rerun.Audit-honesty checklist
changelog.d/added/Legal and contribution hygiene
Breaking changes
None. The wasm API change is a new optional parameter.
Related issues
Refs #1195. Done: the progress bullet, the plain too-large message naming what reads a larger file, and the pre-check. Not done: the issue's last sentence says the options for opening larger models (wasm64, streaming partitions, reading fewer streams) are "measured and the choice documented". They are documented in
docs/viewer-api.mdwith what each would do and its status, but wasm64, partition streaming and a one-copy binding are not built or measured, so the issue stays open for it. Not done either: determinate progress through the partition-schema recovery.