Skip to content

perf(j2k-native): skip tiles outside the output region before workspace build - #65

Merged
jcwal1516 merged 3 commits into
frames-sg:mainfrom
CarlAllenn:perf-region-decode-skip-noncovered-tiles
Jul 31, 2026
Merged

perf(j2k-native): skip tiles outside the output region before workspace build#65
jcwal1516 merged 3 commits into
frames-sg:mainfrom
CarlAllenn:perf-region-decode-skip-noncovered-tiles

Conversation

@CarlAllenn

Copy link
Copy Markdown
Contributor

Problem

Region decode iterates every tile in the codestream, running build::build (decomposition/precinct/code-block graphs plus coefficient-buffer resize/zero) and segment::parse per tile before the ROI plan can skip any entropy work. Per-request cost therefore scales with the image's total tile count even for a tiny ROI.

Measured on a 15000×11000 RGB codestream with 1024 px tiles (165 tiles, 7 decomposition levels), M1 Pro, serial mode, warm decoder:

request before after
64 px ROI, one tile 63 ms 4 ms
512 px ROI, one tile 90 ms 27 ms
same ROI on a 35-tile codestream 40 ms 27 ms

(A sampling profile attributed the fixed cost to Vec::resize/__bzero under j2c::build::build and to tag-tree/segment parsing, repeated for all 165 tiles per decode.) After the change, per-request cost is proportional to the ROI, not the codestream.

Change

In the decode tile loop, skip a tile when its rect cannot intersect the output region. The intersection test maps the region onto the reference grid the same way RoiPlan::build does — including both the component subsampling factor and the reduced-resolution shrink factor — and rounds the tile rect outward, so any rounding disagreement decodes a boundary tile rather than skipping it. Skipped tiles contribute no samples to the stored output, so decoded pixels are unchanged.

Tests

Adds scaled_region_decode_on_multi_tile_codestream_matches_scaled_whole_decode_crop: a multi-tile codestream with partial edge tiles, regions interior / straddling / far-corner, at every Downscale variant, compared against the scaled whole-image decode. This covers a gap the existing suite has (multi-tile × scaled region): an earlier draft of this change ignored the resolution shrink factor and every existing test still passed; this test fails on that draft and passes on the final change.

Validation

cargo xtask fmt / clippy / clippy-strict / test / release-integrity / repo-lint on macOS arm64 and Ubuntu x86-64. The only test failure in either environment is the pre-existing unsafe-audit gap on main (#64), which this branch doesn't touch.

🤖 Generated with Claude Code

…ce build

Region decode iterated every tile in the codestream, building the full
decomposition/precinct/code-block workspace (and zeroing its coefficient
buffers) per tile before the ROI plan could skip any entropy work. The
per-request cost therefore scaled with the image's total tile count even
for a single-tile ROI: on a 15000x11000, 1024px-tiled codestream a 64px
region decode spent ~60 ms in build/parse for the 164 tiles that
contribute nothing.

Skip a tile before decode_tile when its rect cannot intersect the output
region. The intersection test maps the region onto the reference grid
exactly as RoiPlan::build does and rounds the tile rect outward, so any
rounding disagreement decodes a boundary tile rather than skipping it.
Skipped tiles contribute no samples to the stored output, so decoded
pixels are unchanged.
@jcwal1516

Copy link
Copy Markdown
Member

This is a real performance problem, and the patch merges cleanly onto current main. I ran the native and facade decode suites successfully. Before merging, I’d like focused coverage for nonzero image/tile origins and component-subsampled codestreams, since an incorrect early-rejection calculation could silently omit contributing tiles. Please also consider sharing or consolidating the overlap calculation with the existing direct-plan tile-intersection logic so the two paths cannot drift. The optimization itself looks worthwhile.

@jcwal1516
jcwal1516 self-requested a review July 31, 2026 19:02
@jcwal1516 jcwal1516 self-assigned this Jul 31, 2026
@jcwal1516
jcwal1516 merged commit 9aafcb9 into frames-sg:main Jul 31, 2026
@jcwal1516

Copy link
Copy Markdown
Member

The extra coverage and shared overlap helper were an easy fix, so I went ahead and added them and merged this. Thanks for the contribution!

CarlAllenn added a commit to monumental-archive/iiif-server that referenced this pull request Aug 4, 2026
…78)

* chore: repoint j2k pin from fork to upstream main (all fixes merged)

All three fixes the fork pin carried are now merged upstream:
frames-sg/j2k#63 (region IDWT), frames-sg/j2k#65 (tile skip), and
frames-sg/j2k#70 (deep region reduction, the maintainer's implementation
of the API proposed in frames-sg/j2k#66). No crates.io release carries
them yet — v0.8.0 predates the merges — so the [patch.crates-io]
section stays, now pointing at upstream rev 85e7e29.

Call-site changes: upstream kept the proposed public signature
decode_region_scaled_pow2_into unchanged, but did not expose the fork's
Rect::scaled_covering_denominator helper; jp2.rs now computes the
covering rect locally. j2k version requirement bumped 0.7 -> 0.8.

Validated: task ci green; spike2 differential goldens 6/6 (lossless
bit-exact vs OpenJPEG, lossy within tolerance, HTJ2K bit-exact);
codec_masters 9/9.

Refs #21.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): allow deps.dev and scorecards egress for dependency review

The dependency-review action fetches license data from api.deps.dev and
OpenSSF Scorecard data from api.securityscorecards.dev; both were
blocked by the egress allowlist, failing the job with a bare
'fetch failed' (observed in the harden-runner agent log as
'domain not allowed'). Allowlist derived from the audit run per
issue #6 canon.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: nudge CI (empty)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants