Repository navigation
fix(j2k): CPU batch regions of large images and at rounded tile edges - #301
Merged
Merged
Conversation
CpuBatchDecoder region requests of large images failed with ImageTooLarge: prepared region plans store only the region, but their sub-bands and IDWT levels span whole resolution levels, so 1024 px tiles of an 18000 x 12000 image asked for whole level-0 and level-1 planes. Preparation keeps that whole-level geometry, which Metal and CUDA execute, and adds a CPU copy whose bands hold only what each store reads: native region decode's IDWT windows, with each sub-band grown to the code-blocks it decodes. The copy is built through fallible allocation and used only when it at least halves the plan's coefficients or the whole-level plan holds 16 Mi or more, since the windowed IDWT is slower per sample than the banded one. Those tiles now decode with output identical to single-image region decode; quarter-resolution tiles, which already fit, take 24 ms instead of 48 ms (HTJ2K) and 310-317 ms instead of 343-346 ms (classic). Fixes #299
Batch preparation failed with "failed to decode code-block" on reduced regions that end where a tile begins on the reduced grid. Tile selection rounds tile bounds outward, so it keeps a boundary tile that contributes no samples, and the referenced plan builders rejected a tile whose store copies nothing. They now leave such a tile out, with its payload records, as single-image decode does. On the 19 x 13 OpenJPH fixture in 11 x 7 tiles, 4,598 of the 17,290 half-scale regions failed to prepare on the CPU and Metal batches; all now match single-image decode. The test oracle now reads signed 8-bit native output one byte per sample. Fixes #298
Record window_referenced_classic_plan_to_stores and window_referenced_htj2k_plan_to_stores in the j2k-native hidden inventory, refresh the reviewed diff report, and update the review's hidden count, fingerprint and rationale.
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.
Fixes #299 and #298, two
CpuBatchDecoderfailures on region requests (separate commits). First of a three-PR stack; the next two build on this branch.Region tiles of large images failed with
ImageTooLarge(#299)Prepared region plans select only the region's code-blocks, but their sub-band and IDWT rectangles span whole resolution levels, so the CPU executors sized scratch for whole levels. 1024 px tiles of an 18000 × 12000 RGB image at full and half resolution asked for more than the 512 MiB cap.
RoiPlanuses), with each sub-band grown to the code-blocks it decodes.Four 1024 px tiles per level, default options, median ms (
docs/benchmark-evidence.mdhas the command):ImageTooLargeImageTooLargeImageTooLargeImageTooLargeImageTooLargeImageTooLargeOn 240 × 240 slide tiles (CMU-1-JP2K-33005), full requests are unchanged (9.8–10.0 ms) and region requests take 9.1–9.2 ms instead of 8.8–9.0 ms, 0.1 ms of it preparation. Without the half-the-coefficients rule they took 9.5–9.7 ms.
Reduced regions ending at a tile edge failed to prepare (#298)
Tile selection rounds tile bounds outward onto the reduced grid, so it can keep a boundary tile that contributes no samples. The referenced plan builders rejected a tile whose store copies nothing with "failed to decode code-block". They now leave it out, with its payload records, as single-image decode does. This affected the CPU and Metal batches alike.
On the 19 × 13 OpenJPH fixture in 11 × 7 tiles, 4,598 of the 17,290 half-scale regions failed to prepare; all now match single-image decode.
Testing
cargo testfor j2k-types, j2k-native, j2k, j2k-ml;J2K_REQUIRE_METAL_RUNTIME=1 cargo test -p j2k-metal;cargo check -p j2k-cuda;cargo xtaskfmt, clippy, clippy-strict, repo-lint, metal-compile, public-support, panic-surface, unsafe-audit; the quick Metal Clippy targets. CUDA execution was not run here (no CUDA host); its prepared geometry is unchanged, which a test pins.