From abe8c7ac815eabff563eab9469d14fe54bb2b18a Mon Sep 17 00:00:00 2001 From: GF Date: Wed, 28 Oct 2026 23:49:50 -0400 Subject: [PATCH 01/10] fix(j2k): run CPU batch regions on region-sized plans 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 --- CHANGELOG.md | 15 + crates/j2k-native/src/direct_roi.rs | 5 + .../j2k-native/src/direct_roi/plan_windows.rs | 572 ++++++++++++++++++ .../src/direct_roi/plan_windows/tests.rs | 197 ++++++ crates/j2k-native/src/lib.rs | 1 + .../j2k/src/owned_batch/cpu_fast/classic.rs | 10 +- crates/j2k/src/owned_batch/cpu_fast/ht.rs | 6 +- crates/j2k/src/owned_batch/cpu_fast/plan.rs | 2 +- .../j2k/src/owned_batch/cpu_fast/plan/ht.rs | 4 +- crates/j2k/src/owned_batch/cpu_prepared.rs | 44 +- crates/j2k/src/owned_batch/prepare/image.rs | 14 +- crates/j2k/src/owned_batch/prepared_plan.rs | 34 +- crates/j2k/tests/owned_batch.rs | 2 + crates/j2k/tests/owned_batch/fixtures.rs | 13 +- .../j2k/tests/owned_batch/region_windows.rs | 404 +++++++++++++ docs/benchmark-evidence.md | 34 ++ docs/public-support.md | 6 +- 17 files changed, 1309 insertions(+), 54 deletions(-) create mode 100644 crates/j2k-native/src/direct_roi/plan_windows.rs create mode 100644 crates/j2k-native/src/direct_roi/plan_windows/tests.rs create mode 100644 crates/j2k/tests/owned_batch/region_windows.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4455d2f84..e7ca96c9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ and stale roadmap entries have been removed from the public documentation set. ## [Unreleased] +- `CpuBatchDecoder` region and region-plus-reduced requests no longer + allocate coefficient and IDWT buffers for whole resolution levels + ([#299](https://github.com/frames-sg/j2k/issues/299)). Preparation keeps + the shared whole-level plan that Metal and CUDA execute, and adds a CPU copy + whose sub-bands and IDWT levels hold only what the region's stores read, + with the windows native region decode uses. On an 18000 x 12000 RGB image, + 1024 px tiles at full and half resolution failed with `ImageTooLarge`; they + now decode with output identical to single-image region decode. Quarter + resolution, which already fit, takes 24 ms instead of 48 ms for four HTJ2K + tiles and 310 to 317 ms instead of 343 to 346 ms for classic. The CPU uses + the copy only when it at least halves the plan's coefficients or the + whole-level plan holds 16 Mi coefficients or more, because the windowed + IDWT is slower per sample than the banded whole-level one; full and reduced + requests, and regions covering most of a small image, are unchanged. See + `docs/benchmark-evidence.md`. - `j2k-tilecodec` LZW decode no longer reports valid TIFF strips and tiles as truncated. weezl reads input ahead of decoding, so the call that consumes the last byte can still hold codes up to the end code, often in diff --git a/crates/j2k-native/src/direct_roi.rs b/crates/j2k-native/src/direct_roi.rs index f1e572ef1..e755337d2 100644 --- a/crates/j2k-native/src/direct_roi.rs +++ b/crates/j2k-native/src/direct_roi.rs @@ -105,6 +105,11 @@ impl From for J2kRect { #[cfg(test)] mod region_tests; +mod plan_windows; +pub use plan_windows::{ + window_referenced_classic_plan_to_stores, window_referenced_htj2k_plan_to_stores, +}; + /// Required input windows for one direct-plan IDWT step. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct J2kIdwtRequiredInputWindows { diff --git a/crates/j2k-native/src/direct_roi/plan_windows.rs b/crates/j2k-native/src/direct_roi/plan_windows.rs new file mode 100644 index 000000000..1d3d91347 --- /dev/null +++ b/crates/j2k-native/src/direct_roi/plan_windows.rs @@ -0,0 +1,572 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Region-sized copies of referenced direct plans for the CPU executors. +//! +//! A region's referenced plan holds only the code-blocks the region needs, +//! but its sub-bands and IDWT levels span whole resolution levels so that +//! every backend reads the same geometry. The CPU executors honour each +//! band's rectangle, so they can run a copy whose bands hold only what the +//! stores read: the IDWT windows native region decode uses, with each +//! sub-band grown to the whole code-blocks it decodes. Execution memory then +//! follows the region instead of the level. + +#[cfg(test)] +mod tests; + +use alloc::vec::Vec; + +use super::{ + idwt_required_input_window_for_rects, idwt_required_output_margin, J2kRequiredBandRegion, +}; +use crate::j2c::codestream::allocation::try_copy_vec; +use crate::{ + try_reserve_decode_elements, DecodingError, HtOwnedCodeBlockBatchJob, HtOwnedSubBandPlan, + J2kDirectBandId, J2kDirectColorPlan, J2kDirectGrayscalePlan, J2kDirectGrayscaleStep, + J2kDirectRgbaPlan, J2kDirectStoreStep, J2kOwnedCodeBlockBatchJob, J2kOwnedSubBandPlan, J2kRect, + J2kReferencedClassicPlan, J2kReferencedHtj2kPlan, J2kReferencedTileGeometry, + J2kReferencedTilePlan, Result, +}; + +/// Copy `plan` with every band sized to what its stores read, or return +/// `None` when that would not pay off: always for full and reduced decodes, +/// and for regions covering most of a small image. +#[doc(hidden)] +pub fn window_referenced_htj2k_plan_to_stores( + plan: &J2kReferencedHtj2kPlan, +) -> Result> { + let Some(tiles) = windowed_tiles(plan.tiles())? else { + return Ok(None); + }; + Ok(Some(match plan { + J2kReferencedHtj2kPlan::Grayscale { + full_dimensions, + output_rect, + payloads, + .. + } => J2kReferencedHtj2kPlan::grayscale( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + ), + J2kReferencedHtj2kPlan::Color { + full_dimensions, + output_rect, + payloads, + .. + } => J2kReferencedHtj2kPlan::color( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + ), + J2kReferencedHtj2kPlan::Rgba { + full_dimensions, + output_rect, + payloads, + .. + } => J2kReferencedHtj2kPlan::rgba( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + ), + })) +} + +/// Copy `plan` with every band sized to what its stores read, or return +/// `None` when that would not pay off: always for full and reduced decodes, +/// and for regions covering most of a small image. +#[doc(hidden)] +pub fn window_referenced_classic_plan_to_stores( + plan: &J2kReferencedClassicPlan, +) -> Result> { + let Some(tiles) = windowed_tiles(plan.tiles())? else { + return Ok(None); + }; + Ok(Some(match plan { + J2kReferencedClassicPlan::Grayscale { + full_dimensions, + output_rect, + payloads, + ranges, + .. + } => J2kReferencedClassicPlan::grayscale( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + try_copy_vec(ranges)?, + ), + J2kReferencedClassicPlan::Color { + full_dimensions, + output_rect, + payloads, + ranges, + .. + } => J2kReferencedClassicPlan::color( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + try_copy_vec(ranges)?, + ), + J2kReferencedClassicPlan::Rgba { + full_dimensions, + output_rect, + payloads, + ranges, + .. + } => J2kReferencedClassicPlan::rgba( + tiles, + *full_dimensions, + *output_rect, + try_copy_vec(payloads)?, + try_copy_vec(ranges)?, + ), + })) +} + +/// A windowed copy trades the banded whole-level IDWT for the slower +/// windowed one, so it pays off once it at least halves the coefficients a +/// plan holds, or once the whole-level plan is large enough that its memory +/// matters more: 16 Mi coefficients, 64 MiB as `f32`. +const ALWAYS_WINDOW_COEFFICIENTS: u64 = 1 << 24; + +/// Windowed copies of `tiles`, or `None` when windowing would not pay off. +fn windowed_tiles(tiles: &[J2kReferencedTilePlan]) -> Result>> { + let mut tile_windows = Vec::new(); + try_reserve_decode_elements(&mut tile_windows, tiles.len())?; + let (mut whole_area, mut windowed_area) = (0_u64, 0_u64); + for tile in tiles { + let windows = component_plans(tile) + .map(band_windows) + .collect::>>()?; + for (plan, windows) in component_plans(tile).zip(&windows) { + let whole = plan_area(plan); + whole_area = whole_area.saturating_add(whole); + windowed_area = + windowed_area.saturating_add(windows.as_ref().map_or(whole, BandWindows::area)); + } + tile_windows.push(windows); + } + let pays_off = + windowed_area.saturating_mul(2) <= whole_area || whole_area >= ALWAYS_WINDOW_COEFFICIENTS; + if windowed_area >= whole_area || !pays_off { + return Ok(None); + } + + let mut windowed = Vec::new(); + try_reserve_decode_elements(&mut windowed, tiles.len())?; + for (tile, windows) in tiles.iter().zip(&tile_windows) { + let mut geometry = tile_geometry(tile)?; + for (plan, windows) in geometry_component_plans(&mut geometry).zip(windows) { + if let Some(windows) = windows { + apply_band_windows(plan, windows)?; + } + } + windowed.push(J2kReferencedTilePlan::new( + tile.tile_index(), + tile.decoded_rect(), + tile.destination_rect(), + tile.payload_records(), + try_copy_vec(tile.classic_payloads())?, + try_copy_vec(tile.classic_ranges())?, + tile.wavelet_transform(), + geometry, + )); + } + Ok(Some(windowed)) +} + +fn component_plans(tile: &J2kReferencedTilePlan) -> impl Iterator { + let plans: &[J2kDirectGrayscalePlan] = tile + .grayscale_geometry() + .map(core::slice::from_ref) + .or_else(|| { + tile.color_geometry() + .map(|plan| plan.component_plans.as_slice()) + }) + .or_else(|| { + tile.rgba_geometry() + .map(|plan| plan.component_plans.as_slice()) + }) + .unwrap_or_default(); + plans.iter() +} + +fn tile_geometry(tile: &J2kReferencedTilePlan) -> Result { + if let Some(plan) = tile.grayscale_geometry() { + return Ok(J2kReferencedTileGeometry::Grayscale(try_copy_plan(plan)?)); + } + if let Some(plan) = tile.color_geometry() { + return Ok(J2kReferencedTileGeometry::Color(J2kDirectColorPlan { + component_plans: try_copy_plans(&plan.component_plans)?, + ..*plan + })); + } + let plan = tile + .rgba_geometry() + .ok_or(DecodingError::CodeBlockDecodeFailure)?; + Ok(J2kReferencedTileGeometry::Rgba(J2kDirectRgbaPlan { + component_plans: try_copy_plans(&plan.component_plans)?, + ..*plan + })) +} + +fn try_copy_plans(plans: &[J2kDirectGrayscalePlan]) -> Result> { + let mut copies = Vec::new(); + try_reserve_decode_elements(&mut copies, plans.len())?; + for plan in plans { + copies.push(try_copy_plan(plan)?); + } + Ok(copies) +} + +/// Copy one component plan through fallible allocations, as the plan +/// builders allocate it. +fn try_copy_plan(plan: &J2kDirectGrayscalePlan) -> Result { + let mut steps = Vec::new(); + try_reserve_decode_elements(&mut steps, plan.steps.len())?; + for step in &plan.steps { + steps.push(match step { + J2kDirectGrayscaleStep::ClassicSubBand(sub_band) => { + let mut jobs = Vec::new(); + try_reserve_decode_elements(&mut jobs, sub_band.jobs.len())?; + for job in &sub_band.jobs { + jobs.push(J2kOwnedCodeBlockBatchJob { + data: try_copy_vec(&job.data)?, + segments: try_copy_vec(&job.segments)?, + ..*job + }); + } + J2kDirectGrayscaleStep::ClassicSubBand(J2kOwnedSubBandPlan { jobs, ..*sub_band }) + } + J2kDirectGrayscaleStep::HtSubBand(sub_band) => { + let mut jobs = Vec::new(); + try_reserve_decode_elements(&mut jobs, sub_band.jobs.len())?; + for job in &sub_band.jobs { + jobs.push(HtOwnedCodeBlockBatchJob { + data: try_copy_vec(&job.data)?, + ..*job + }); + } + J2kDirectGrayscaleStep::HtSubBand(HtOwnedSubBandPlan { jobs, ..*sub_band }) + } + J2kDirectGrayscaleStep::Idwt(idwt) => J2kDirectGrayscaleStep::Idwt(*idwt), + J2kDirectGrayscaleStep::Store(store) => J2kDirectGrayscaleStep::Store(*store), + }); + } + Ok(J2kDirectGrayscalePlan { steps, ..*plan }) +} + +fn geometry_component_plans( + geometry: &mut J2kReferencedTileGeometry, +) -> impl Iterator { + match geometry { + J2kReferencedTileGeometry::Grayscale(plan) => core::slice::from_mut(plan).iter_mut(), + J2kReferencedTileGeometry::Color(plan) => plan.component_plans.iter_mut(), + J2kReferencedTileGeometry::Rgba(plan) => plan.component_plans.iter_mut(), + } +} + +/// The rectangle each band of one component plan keeps. +struct BandWindows { + rects: Vec<(J2kDirectBandId, J2kRect)>, +} + +impl BandWindows { + /// Coefficients the windowed bands hold. + fn area(&self) -> u64 { + self.rects + .iter() + .fold(0, |area, (_, rect)| area.saturating_add(rect_area(*rect))) + } + + fn get(&self, band_id: J2kDirectBandId) -> Result { + self.rects + .iter() + .find(|(id, _)| *id == band_id) + .map(|(_, rect)| *rect) + .ok_or_else(|| DecodingError::CodeBlockDecodeFailure.into()) + } +} + +/// Windows for one component plan, computed from its whole-level geometry, +/// or `None` when every store reads its whole input band. +fn band_windows(plan: &J2kDirectGrayscalePlan) -> Result> { + let mut required: Vec<(J2kDirectBandId, J2kRequiredBandRegion)> = Vec::new(); + let mut whole = true; + for step in &plan.steps { + if let J2kDirectGrayscaleStep::Store(store) = step { + let copy = store_copy_region(store)?; + whole &= copy.is_some_and(|copy| copy.to_rect() == store.input_rect); + if let Some(copy) = copy { + add_required(&mut required, store.input_band_id, copy)?; + } + } + } + if whole { + return Ok(None); + } + + let mut rects = Vec::new(); + // Native region decode expands each level's required output by the + // synthesis margin before deriving the next level down, so the windowed + // IDWT reproduces whole-level samples everywhere a store reads. + for step in plan.steps.iter().rev() { + let J2kDirectGrayscaleStep::Idwt(idwt) = step else { + continue; + }; + let window = lookup(&required, idwt.output_band_id) + .map(|output| { + output.expanded_within_rect(idwt_required_output_margin(idwt.transform), idwt.rect) + }) + .filter(|window| window.width() > 0 && window.height() > 0); + let Some(window) = window else { + push_rect(&mut rects, idwt.output_band_id, empty_at(idwt.rect))?; + continue; + }; + push_rect(&mut rects, idwt.output_band_id, window.to_rect())?; + for (band_id, band_rect, low_x, low_y) in [ + (idwt.ll_band_id, idwt.ll, true, true), + (idwt.hl_band_id, idwt.hl, false, true), + (idwt.lh_band_id, idwt.lh, true, false), + (idwt.hh_band_id, idwt.hh, false, false), + ] { + let input = + idwt_required_input_window_for_rects(window, idwt.rect, band_rect, low_x, low_y); + if input.width() > 0 && input.height() > 0 { + add_required(&mut required, band_id, input)?; + } + } + } + + // Each code-block decodes whole into its band, so a sub-band keeps the + // window the IDWT reads plus every job the plan decodes. A band split + // across classic and HT steps shares one rectangle. + let mut sub_bands: Vec<(J2kDirectBandId, J2kRect, Option)> = Vec::new(); + for step in &plan.steps { + let (band_id, rect, jobs) = match step { + J2kDirectGrayscaleStep::ClassicSubBand(sub_band) => ( + sub_band.band_id, + sub_band.rect, + job_bounds( + sub_band.rect, + sub_band + .jobs + .iter() + .map(|job| (job.output_x, job.output_y, job.width, job.height)), + )?, + ), + J2kDirectGrayscaleStep::HtSubBand(sub_band) => ( + sub_band.band_id, + sub_band.rect, + job_bounds( + sub_band.rect, + sub_band + .jobs + .iter() + .map(|job| (job.output_x, job.output_y, job.width, job.height)), + )?, + ), + J2kDirectGrayscaleStep::Idwt(_) | J2kDirectGrayscaleStep::Store(_) => continue, + }; + if let Some(entry) = sub_bands.iter_mut().find(|(id, _, _)| *id == band_id) { + entry.2 = union(entry.2, jobs); + } else { + let len = sub_bands.len() + 1; + try_reserve_decode_elements(&mut sub_bands, len)?; + sub_bands.push((band_id, rect, union(lookup(&required, band_id), jobs))); + } + } + for (band_id, rect, stored) in sub_bands { + let stored = stored.map_or_else(|| empty_at(rect), |stored| clip(stored.to_rect(), rect)); + push_rect(&mut rects, band_id, stored)?; + } + Ok(Some(BandWindows { rects })) +} + +/// Rewrite one component plan's bands, jobs, IDWT levels and stores to the +/// rectangles in `windows`. +fn apply_band_windows(plan: &mut J2kDirectGrayscalePlan, windows: &BandWindows) -> Result<()> { + for step in &mut plan.steps { + match step { + J2kDirectGrayscaleStep::ClassicSubBand(sub_band) => { + let stored = windows.get(sub_band.band_id)?; + for job in &mut sub_band.jobs { + rebase_job(sub_band.rect, stored, &mut job.output_x, &mut job.output_y)?; + job.output_stride = stored.width() as usize; + } + (sub_band.rect, sub_band.width, sub_band.height) = + (stored, stored.width(), stored.height()); + } + J2kDirectGrayscaleStep::HtSubBand(sub_band) => { + let stored = windows.get(sub_band.band_id)?; + for job in &mut sub_band.jobs { + rebase_job(sub_band.rect, stored, &mut job.output_x, &mut job.output_y)?; + job.output_stride = stored.width() as usize; + } + (sub_band.rect, sub_band.width, sub_band.height) = + (stored, stored.width(), stored.height()); + } + J2kDirectGrayscaleStep::Idwt(idwt) => { + idwt.rect = windows.get(idwt.output_band_id)?; + idwt.ll = windows.get(idwt.ll_band_id)?; + idwt.hl = windows.get(idwt.hl_band_id)?; + idwt.lh = windows.get(idwt.lh_band_id)?; + idwt.hh = windows.get(idwt.hh_band_id)?; + } + J2kDirectGrayscaleStep::Store(store) => { + let input = windows.get(store.input_band_id)?; + if let Some(copy) = store_copy_region(store)? { + if copy.x0 < input.x0 + || copy.y0 < input.y0 + || copy.x1 > input.x1 + || copy.y1 > input.y1 + { + return Err(DecodingError::CodeBlockDecodeFailure.into()); + } + store.source_x = copy.x0 - input.x0; + store.source_y = copy.y0 - input.y0; + } + store.input_rect = input; + } + } + } + Ok(()) +} + +/// Coefficients a whole-level component plan holds across its bands. A band +/// split across classic and HT steps appears as adjacent steps and counts +/// once. +fn plan_area(plan: &J2kDirectGrayscalePlan) -> u64 { + let mut previous_sub_band = None; + plan.steps.iter().fold(0_u64, |area, step| { + let (band_id, rect) = match step { + J2kDirectGrayscaleStep::ClassicSubBand(sub_band) => (sub_band.band_id, sub_band.rect), + J2kDirectGrayscaleStep::HtSubBand(sub_band) => (sub_band.band_id, sub_band.rect), + J2kDirectGrayscaleStep::Idwt(idwt) => return area.saturating_add(rect_area(idwt.rect)), + J2kDirectGrayscaleStep::Store(_) => return area, + }; + if previous_sub_band.replace(band_id) == Some(band_id) { + area + } else { + area.saturating_add(rect_area(rect)) + } + }) +} + +fn rect_area(rect: J2kRect) -> u64 { + u64::from(rect.width()) * u64::from(rect.height()) +} + +/// The absolute input-band region one store copies, if any. +fn store_copy_region(store: &J2kDirectStoreStep) -> Result> { + let x0 = checked_add(store.input_rect.x0, store.source_x)?; + let y0 = checked_add(store.input_rect.y0, store.source_y)?; + Ok(J2kRequiredBandRegion::new( + x0, + y0, + checked_add(x0, store.copy_width)?, + checked_add(y0, store.copy_height)?, + )) +} + +/// The absolute bounds of the jobs a sub-band decodes, from band-relative +/// `(x, y, width, height)` placements. +fn job_bounds( + rect: J2kRect, + jobs: impl Iterator, +) -> Result> { + let mut bounds = None; + for (x, y, width, height) in jobs { + let x0 = checked_add(rect.x0, x)?; + let y0 = checked_add(rect.y0, y)?; + let job = + J2kRequiredBandRegion::new(x0, y0, checked_add(x0, width)?, checked_add(y0, height)?); + bounds = union(bounds, job); + } + Ok(bounds) +} + +fn rebase_job(from: J2kRect, to: J2kRect, output_x: &mut u32, output_y: &mut u32) -> Result<()> { + *output_x = checked_add(from.x0, *output_x)? + .checked_sub(to.x0) + .ok_or(DecodingError::CodeBlockDecodeFailure)?; + *output_y = checked_add(from.y0, *output_y)? + .checked_sub(to.y0) + .ok_or(DecodingError::CodeBlockDecodeFailure)?; + Ok(()) +} + +fn add_required( + required: &mut Vec<(J2kDirectBandId, J2kRequiredBandRegion)>, + band_id: J2kDirectBandId, + region: J2kRequiredBandRegion, +) -> Result<()> { + if let Some(entry) = required.iter_mut().find(|(id, _)| *id == band_id) { + entry.1 = entry.1.union(region); + } else { + let len = required.len() + 1; + try_reserve_decode_elements(required, len)?; + required.push((band_id, region)); + } + Ok(()) +} + +fn push_rect( + rects: &mut Vec<(J2kDirectBandId, J2kRect)>, + band_id: J2kDirectBandId, + rect: J2kRect, +) -> Result<()> { + let len = rects.len() + 1; + try_reserve_decode_elements(rects, len)?; + rects.push((band_id, rect)); + Ok(()) +} + +fn lookup( + required: &[(J2kDirectBandId, J2kRequiredBandRegion)], + band_id: J2kDirectBandId, +) -> Option { + required + .iter() + .find(|(id, _)| *id == band_id) + .map(|(_, region)| *region) +} + +fn union( + a: Option, + b: Option, +) -> Option { + match (a, b) { + (Some(a), Some(b)) => Some(a.union(b)), + (a, b) => a.or(b), + } +} + +fn clip(rect: J2kRect, within: J2kRect) -> J2kRect { + let x0 = rect.x0.max(within.x0).min(within.x1); + let y0 = rect.y0.max(within.y0).min(within.y1); + J2kRect { + x0, + y0, + x1: rect.x1.min(within.x1).max(x0), + y1: rect.y1.min(within.y1).max(y0), + } +} + +const fn empty_at(rect: J2kRect) -> J2kRect { + J2kRect { + x0: rect.x0, + y0: rect.y0, + x1: rect.x0, + y1: rect.y0, + } +} + +fn checked_add(a: u32, b: u32) -> Result { + a.checked_add(b) + .ok_or_else(|| DecodingError::CodeBlockDecodeFailure.into()) +} diff --git a/crates/j2k-native/src/direct_roi/plan_windows/tests.rs b/crates/j2k-native/src/direct_roi/plan_windows/tests.rs new file mode 100644 index 000000000..9421444e0 --- /dev/null +++ b/crates/j2k-native/src/direct_roi/plan_windows/tests.rs @@ -0,0 +1,197 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +use alloc::vec::Vec; + +use super::{ + component_plans, plan_area, window_referenced_classic_plan_to_stores, + window_referenced_htj2k_plan_to_stores, +}; +use crate::{ + encode, encode_htj2k, execute_referenced_classic_plan, execute_referenced_htj2k_plan, + DecodeSettings, DecoderContext, EncodeOptions, Image, J2kDirectCpuScratch, + J2kDirectGrayscalePlan, J2kDirectGrayscaleStep, J2kRect, J2kReferencedTilePlan, +}; + +fn gray_fixture(width: u32, height: u32, ht: bool, reversible: bool) -> Vec { + let samples = (0..width * height) + .map(|index| { + let (x, y) = (index % width, index / width); + u8::try_from((x * 5 + y * 3 + (x ^ y) % 37) & 0xff).expect("masked sample") + }) + .collect::>(); + let options = EncodeOptions { + reversible, + num_decomposition_levels: 4, + ..EncodeOptions::default() + }; + let codec = if ht { encode_htj2k } else { encode }; + codec(&samples, width, height, 1, 8, false, &options).expect("encode gray fixture") +} + +fn contains(outer: J2kRect, inner: J2kRect) -> bool { + outer.x0 <= inner.x0 && inner.x1 <= outer.x1 && outer.y0 <= inner.y0 && inner.y1 <= outer.y1 +} + +/// Every windowed band lies inside its whole-level band, every job inside +/// its windowed band, and every store copies the same samples from inside +/// its windowed input. +fn assert_windows_nest(whole: &J2kDirectGrayscalePlan, windowed: &J2kDirectGrayscalePlan) { + assert_eq!(whole.steps.len(), windowed.steps.len()); + for (whole, windowed) in whole.steps.iter().zip(&windowed.steps) { + match (whole, windowed) { + ( + J2kDirectGrayscaleStep::ClassicSubBand(whole), + J2kDirectGrayscaleStep::ClassicSubBand(windowed), + ) => { + assert!(contains(whole.rect, windowed.rect)); + assert_eq!(whole.jobs.len(), windowed.jobs.len()); + for job in &windowed.jobs { + assert!(job.output_x + job.width <= windowed.width); + assert!(job.output_y + job.height <= windowed.height); + assert_eq!(job.output_stride, windowed.width as usize); + } + } + ( + J2kDirectGrayscaleStep::HtSubBand(whole), + J2kDirectGrayscaleStep::HtSubBand(windowed), + ) => { + assert!(contains(whole.rect, windowed.rect)); + assert_eq!(whole.jobs.len(), windowed.jobs.len()); + for job in &windowed.jobs { + assert!(job.output_x + job.width <= windowed.width); + assert!(job.output_y + job.height <= windowed.height); + assert_eq!(job.output_stride, windowed.width as usize); + } + } + (J2kDirectGrayscaleStep::Idwt(whole), J2kDirectGrayscaleStep::Idwt(windowed)) => { + assert!(contains(whole.rect, windowed.rect)); + } + (J2kDirectGrayscaleStep::Store(whole), J2kDirectGrayscaleStep::Store(windowed)) => { + assert_eq!( + ( + whole.copy_width, + whole.copy_height, + whole.output_x, + whole.output_y + ), + ( + windowed.copy_width, + windowed.copy_height, + windowed.output_x, + windowed.output_y + ) + ); + assert_eq!( + ( + whole.input_rect.x0 + whole.source_x, + whole.input_rect.y0 + whole.source_y + ), + ( + windowed.input_rect.x0 + windowed.source_x, + windowed.input_rect.y0 + windowed.source_y + ) + ); + assert!(windowed.source_x + windowed.copy_width <= windowed.input_rect.width()); + assert!(windowed.source_y + windowed.copy_height <= windowed.input_rect.height()); + } + (whole, windowed) => panic!("step kinds differ: {whole:?} vs {windowed:?}"), + } + } +} + +/// Check nesting and return the whole-level and windowed coefficient areas. +fn checked_areas( + whole: &[J2kReferencedTilePlan], + windowed: &[J2kReferencedTilePlan], +) -> (u64, u64) { + assert_eq!(whole.len(), windowed.len()); + let (mut whole_area, mut windowed_area) = (0, 0); + for (whole, windowed) in whole.iter().zip(windowed) { + for (whole, windowed) in component_plans(whole).zip(component_plans(windowed)) { + assert_windows_nest(whole, windowed); + whole_area += plan_area(whole); + windowed_area += plan_area(windowed); + } + } + (whole_area, windowed_area) +} + +#[test] +fn small_regions_decode_identically_from_windowed_bands() { + let region = (301, 207, 23, 17); + for reversible in [true, false] { + let case = format!("reversible={reversible}"); + let bytes = gray_fixture(512, 384, true, reversible); + let image = Image::new(&bytes, &DecodeSettings::default()).expect("HTJ2K image"); + let whole = image + .build_referenced_htj2k_plan_region_with_context(&mut DecoderContext::default(), region) + .expect("HTJ2K region plan"); + let windowed = window_referenced_htj2k_plan_to_stores(&whole) + .expect("window HTJ2K plan") + .expect("a 23 x 17 region windows a 512 x 384 plan"); + let (whole_area, windowed_area) = checked_areas(whole.tiles(), windowed.tiles()); + assert!( + windowed_area * 2 <= whole_area, + "{case}: {windowed_area} of {whole_area}" + ); + + let (mut whole_scratch, mut windowed_scratch) = + (J2kDirectCpuScratch::new(), J2kDirectCpuScratch::new()); + let expected = execute_referenced_htj2k_plan(&whole, &bytes, false, &mut whole_scratch) + .expect("execute whole-level plan"); + let actual = execute_referenced_htj2k_plan(&windowed, &bytes, false, &mut windowed_scratch) + .expect("execute windowed plan"); + assert_eq!(expected.dimensions(), actual.dimensions(), "{case}"); + assert_eq!( + expected.plane(0).expect("whole plane").samples(), + actual.plane(0).expect("windowed plane").samples(), + "{case}" + ); + + let bytes = gray_fixture(512, 384, false, reversible); + let image = Image::new(&bytes, &DecodeSettings::default()).expect("classic image"); + let whole = image + .build_referenced_classic_plan_region_with_context( + &mut DecoderContext::default(), + region, + ) + .expect("classic region plan"); + let windowed = window_referenced_classic_plan_to_stores(&whole) + .expect("window classic plan") + .expect("a 23 x 17 region windows a 512 x 384 plan"); + let (whole_area, windowed_area) = checked_areas(whole.tiles(), windowed.tiles()); + assert!( + windowed_area * 2 <= whole_area, + "{case}: {windowed_area} of {whole_area}" + ); + + let expected = execute_referenced_classic_plan(&whole, &bytes, false, &mut whole_scratch) + .expect("execute whole-level classic plan"); + let actual = + execute_referenced_classic_plan(&windowed, &bytes, false, &mut windowed_scratch) + .expect("execute windowed classic plan"); + assert_eq!(expected.dimensions(), actual.dimensions(), "{case}"); + assert_eq!( + expected.plane(0).expect("whole plane").samples(), + actual.plane(0).expect("windowed plane").samples(), + "{case}" + ); + } +} + +#[test] +fn full_plans_and_regions_covering_most_of_a_small_image_stay_whole() { + let bytes = gray_fixture(240, 240, true, false); + let image = Image::new(&bytes, &DecodeSettings::default()).expect("HTJ2K image"); + for region in [(0, 0, 240, 240), (20, 20, 200, 200)] { + let whole = image + .build_referenced_htj2k_plan_region_with_context(&mut DecoderContext::default(), region) + .expect("HTJ2K region plan"); + assert!( + window_referenced_htj2k_plan_to_stores(&whole) + .expect("window HTJ2K plan") + .is_none(), + "region {region:?} must keep the shared plan" + ); + } +} diff --git a/crates/j2k-native/src/lib.rs b/crates/j2k-native/src/lib.rs index e596b66c1..d385338aa 100644 --- a/crates/j2k-native/src/lib.rs +++ b/crates/j2k-native/src/lib.rs @@ -203,6 +203,7 @@ pub use direct_plan::{ #[doc(hidden)] pub use direct_roi::{ idwt_required_input_window_for_rects, idwt_required_input_windows, idwt_required_output_margin, + window_referenced_classic_plan_to_stores, window_referenced_htj2k_plan_to_stores, J2kIdwtRequiredInputWindows, J2kRequiredBandRegion, }; pub use inspect::{ diff --git a/crates/j2k/src/owned_batch/cpu_fast/classic.rs b/crates/j2k/src/owned_batch/cpu_fast/classic.rs index 9b2c6b2ae..8ad462980 100644 --- a/crates/j2k/src/owned_batch/cpu_fast/classic.rs +++ b/crates/j2k/src/owned_batch/cpu_fast/classic.rs @@ -52,7 +52,7 @@ impl CpuGroupFastWorkspace { .classic_plan() .ok_or(BatchInfrastructureError::MissingResult { index: image_slot })?; let span = self.image_spans[image_slot]; - visit_classic_jobs(plan.native_plan(), |payload_index, block_index| { + visit_classic_jobs(plan.cpu_plan(), |payload_index, block_index| { self.jobs.push(CpuFlattenedPayloadJob { source_index: group.source_indices[image_slot], image_slot, @@ -74,7 +74,7 @@ impl CpuGroupFastWorkspace { .ok_or(BatchInfrastructureError::MissingResult { index: job.source_index, })?; - let payload = plan.native_plan().payloads().get(job.payload_index).ok_or( + let payload = plan.cpu_plan().payloads().get(job.payload_index).ok_or( BatchInfrastructureError::MissingResult { index: job.source_index, }, @@ -84,15 +84,15 @@ impl CpuGroupFastWorkspace { .end_range() .ok_or(BatchInfrastructureError::ResultIndexOutOfBounds { index: payload.first_range, - job_count: plan.native_plan().ranges().len(), + job_count: plan.cpu_plan().ranges().len(), })?; let fragments = plan - .native_plan() + .cpu_plan() .ranges() .get(payload.first_range..end_range) .ok_or(BatchInfrastructureError::ResultIndexOutOfBounds { index: end_range, - job_count: plan.native_plan().ranges().len(), + job_count: plan.cpu_plan().ranges().len(), })?; let start = self.compressed_arena.len(); for range in fragments { diff --git a/crates/j2k/src/owned_batch/cpu_fast/ht.rs b/crates/j2k/src/owned_batch/cpu_fast/ht.rs index 62171c245..21573e77c 100644 --- a/crates/j2k/src/owned_batch/cpu_fast/ht.rs +++ b/crates/j2k/src/owned_batch/cpu_fast/ht.rs @@ -51,7 +51,7 @@ impl CpuGroupFastWorkspace { let plan = image .htj2k_plan() .ok_or(BatchInfrastructureError::MissingResult { index: 0 })?; - visit_ht_jobs(plan.native_plan(), |_, _, _, _| {}) + visit_ht_jobs(plan.cpu_plan(), |_, _, _, _| {}) }) } @@ -72,7 +72,7 @@ impl CpuGroupFastWorkspace { let span = self.image_spans[image_slot]; let mut bucket_ordinals = [0_usize; 3]; visit_ht_jobs( - plan.native_plan(), + plan.cpu_plan(), |job_index, payload_records, block_index, coding_passes| { if ht_bucket(coding_passes) == bucket { let bucket_index = ht_bucket_index(bucket); @@ -130,7 +130,7 @@ impl CpuGroupFastWorkspace { let payloads = image .htj2k_plan() .and_then(|plan| { - plan.native_plan() + plan.cpu_plan() .payloads() .get(job.payload_index..payload_end) }) diff --git a/crates/j2k/src/owned_batch/cpu_fast/plan.rs b/crates/j2k/src/owned_batch/cpu_fast/plan.rs index d9dcc31e1..c2a39adfa 100644 --- a/crates/j2k/src/owned_batch/cpu_fast/plan.rs +++ b/crates/j2k/src/owned_batch/cpu_fast/plan.rs @@ -28,7 +28,7 @@ pub(super) fn classic_group_requirements( .classic_plan() .ok_or(BatchInfrastructureError::MissingResult { index: image_slot })?; payload_count = checked_add(payload_count, plan.payload_count(), "payload jobs")?; - for payload in plan.native_plan().payloads() { + for payload in plan.cpu_plan().payloads() { payload_bytes = checked_add( payload_bytes, payload.combined_length, diff --git a/crates/j2k/src/owned_batch/cpu_fast/plan/ht.rs b/crates/j2k/src/owned_batch/cpu_fast/plan/ht.rs index 885ef9f2b..7f37d6280 100644 --- a/crates/j2k/src/owned_batch/cpu_fast/plan/ht.rs +++ b/crates/j2k/src/owned_batch/cpu_fast/plan/ht.rs @@ -24,9 +24,9 @@ pub(in crate::owned_batch::cpu_fast) fn ht_group_requirements( let plan = image .htj2k_plan() .ok_or(BatchInfrastructureError::MissingResult { index: image_slot })?; - let job_count = visit_ht_jobs(plan.native_plan(), |_, _, _, _| {})?; + let job_count = visit_ht_jobs(plan.cpu_plan(), |_, _, _, _| {})?; payload_count = checked_add(payload_count, job_count, "payload jobs")?; - for payload in plan.native_plan().payloads() { + for payload in plan.cpu_plan().payloads() { payload_bytes = checked_add( payload_bytes, payload.cleanup.length, diff --git a/crates/j2k/src/owned_batch/cpu_prepared.rs b/crates/j2k/src/owned_batch/cpu_prepared.rs index 06e197356..f1ed9e88e 100644 --- a/crates/j2k/src/owned_batch/cpu_prepared.rs +++ b/crates/j2k/src/owned_batch/cpu_prepared.rs @@ -17,10 +17,10 @@ pub(super) fn decode_prepared_plan_samples( ) -> Result { let components = match image.codec_plan() { PreparedCodecPlan::Htj2k(prepared_plan) => worker - .execute_prepared_htj2k_plan(prepared_plan.native_plan(), image.bytes(), info.signed) + .execute_prepared_htj2k_plan(prepared_plan.cpu_plan(), image.bytes(), info.signed) .map_err(J2kError::from_native_decode_error)?, PreparedCodecPlan::Classic(prepared_plan) => worker - .execute_prepared_classic_plan(prepared_plan.native_plan(), image.bytes(), info.signed) + .execute_prepared_classic_plan(prepared_plan.cpu_plan(), image.bytes(), info.signed) .map_err(J2kError::from_native_decode_error)?, PreparedCodecPlan::MetadataOnly => return Ok(false), }; @@ -38,10 +38,10 @@ pub(super) fn prepare_staged_image( ) -> Result<(), J2kError> { match (route, image.codec_plan()) { (BatchCodecRoute::Htj2k, PreparedCodecPlan::Htj2k(plan)) => worker - .prepare_staged_htj2k_plan(plan.native_plan(), scratch) + .prepare_staged_htj2k_plan(plan.cpu_plan(), scratch) .map_err(J2kError::from_native_decode_error), (BatchCodecRoute::Classic, PreparedCodecPlan::Classic(plan)) => worker - .prepare_staged_classic_plan(plan.native_plan(), scratch) + .prepare_staged_classic_plan(plan.cpu_plan(), scratch) .map_err(J2kError::from_native_decode_error), _ => Err(J2kError::internal_backend( "staged CPU route does not match its prepared codec plan", @@ -63,7 +63,7 @@ pub(super) fn execute_staged_entropy_job( })?; worker .execute_staged_htj2k_job( - plan.native_plan(), + plan.cpu_plan(), job.block_index, flattened.arena(), payload, @@ -77,7 +77,7 @@ pub(super) fn execute_staged_entropy_job( })?; worker .execute_staged_classic_job( - plan.native_plan(), + plan.cpu_plan(), job.block_index, flattened.arena(), payload, @@ -98,10 +98,10 @@ pub(super) fn prepare_staged_entropy_worker( ) -> Result<(), J2kError> { match (route, image.codec_plan()) { (BatchCodecRoute::Htj2k, PreparedCodecPlan::Htj2k(plan)) => worker - .prepare_staged_htj2k_entropy_workspace(plan.native_plan()) + .prepare_staged_htj2k_entropy_workspace(plan.cpu_plan()) .map_err(J2kError::from_native_decode_error), (BatchCodecRoute::Classic, PreparedCodecPlan::Classic(plan)) => worker - .prepare_staged_classic_entropy_workspace(plan.native_plan()) + .prepare_staged_classic_entropy_workspace(plan.cpu_plan()) .map_err(J2kError::from_native_decode_error), _ => Err(J2kError::internal_backend( "staged CPU worker route does not match its prepared codec plan", @@ -115,10 +115,10 @@ pub(super) fn staged_tile_count( ) -> Result { match (route, image.codec_plan()) { (BatchCodecRoute::Htj2k, PreparedCodecPlan::Htj2k(plan)) => { - Ok(plan.native_plan().tiles().len()) + Ok(plan.cpu_plan().tiles().len()) } (BatchCodecRoute::Classic, PreparedCodecPlan::Classic(plan)) => { - Ok(plan.native_plan().tiles().len()) + Ok(plan.cpu_plan().tiles().len()) } _ => Err(J2kError::internal_backend( "staged CPU route does not match its prepared codec plan", @@ -134,20 +134,12 @@ pub(super) fn prepare_staged_tile( ) -> Result<(), J2kError> { match (route, image.codec_plan()) { (BatchCodecRoute::Htj2k, PreparedCodecPlan::Htj2k(plan)) => { - j2k_native::prepare_referenced_htj2k_tile_staged( - plan.native_plan(), - tile_index, - scratch, - ) - .map_err(J2kError::from_native_decode_error) + j2k_native::prepare_referenced_htj2k_tile_staged(plan.cpu_plan(), tile_index, scratch) + .map_err(J2kError::from_native_decode_error) } (BatchCodecRoute::Classic, PreparedCodecPlan::Classic(plan)) => { - j2k_native::prepare_referenced_classic_tile_staged( - plan.native_plan(), - tile_index, - scratch, - ) - .map_err(J2kError::from_native_decode_error) + j2k_native::prepare_referenced_classic_tile_staged(plan.cpu_plan(), tile_index, scratch) + .map_err(J2kError::from_native_decode_error) } _ => Err(J2kError::internal_backend( "staged CPU route does not match its prepared codec plan", @@ -165,7 +157,7 @@ pub(super) fn finish_staged_tile( match (route, image.codec_plan()) { (BatchCodecRoute::Htj2k, PreparedCodecPlan::Htj2k(plan)) => { j2k_native::finish_referenced_htj2k_tile_staged( - plan.native_plan(), + plan.cpu_plan(), tile_index, signed, scratch, @@ -174,7 +166,7 @@ pub(super) fn finish_staged_tile( } (BatchCodecRoute::Classic, PreparedCodecPlan::Classic(plan)) => { j2k_native::finish_referenced_classic_tile_staged( - plan.native_plan(), + plan.cpu_plan(), tile_index, signed, scratch, @@ -196,11 +188,11 @@ pub(super) fn finish_staged_plan_samples( ) -> Result<(), J2kError> { let components = match image.codec_plan() { PreparedCodecPlan::Htj2k(plan) => { - j2k_native::finish_referenced_htj2k_staged(plan.native_plan(), info.signed, scratch) + j2k_native::finish_referenced_htj2k_staged(plan.cpu_plan(), info.signed, scratch) .map_err(J2kError::from_native_decode_error)? } PreparedCodecPlan::Classic(plan) => { - j2k_native::finish_referenced_classic_staged(plan.native_plan(), info.signed, scratch) + j2k_native::finish_referenced_classic_staged(plan.cpu_plan(), info.signed, scratch) .map_err(J2kError::from_native_decode_error)? } PreparedCodecPlan::MetadataOnly => { diff --git a/crates/j2k/src/owned_batch/prepare/image.rs b/crates/j2k/src/owned_batch/prepare/image.rs index 0fba6c78f..02aab69e6 100644 --- a/crates/j2k/src/owned_batch/prepare/image.rs +++ b/crates/j2k/src/owned_batch/prepare/image.rs @@ -114,7 +114,12 @@ fn prepare_classic_offset_plan( } let output = plan.output_rect(); match worker.build_prepared_classic_plan(image, (output.x, output.y, output.w, output.h)) { - Ok(plan) => Ok(Some(PreparedClassicPlan::from_native(plan))), + Ok(plan) => PreparedClassicPlan::from_native(plan) + .map(Some) + .map_err(|source| BatchItemError::Codec { + stage: BatchErrorStage::Prepare, + source: Arc::new(J2kError::from_native_decode_error(source)), + }), Err( source @ j2k_native::DecodeError::Decoding( j2k_native::DecodingError::DirectPlanUnsupported(_) @@ -144,7 +149,12 @@ fn prepare_htj2k_offset_plan( } let output = plan.output_rect(); match worker.build_prepared_htj2k_plan(image, (output.x, output.y, output.w, output.h)) { - Ok(plan) => Ok(Some(PreparedHtj2kPlan::from_native(plan))), + Ok(plan) => PreparedHtj2kPlan::from_native(plan) + .map(Some) + .map_err(|source| BatchItemError::Codec { + stage: BatchErrorStage::Prepare, + source: Arc::new(J2kError::from_native_decode_error(source)), + }), Err(source) if matches!( source.classify(), diff --git a/crates/j2k/src/owned_batch/prepared_plan.rs b/crates/j2k/src/owned_batch/prepared_plan.rs index 0a163628a..10e2d1b55 100644 --- a/crates/j2k/src/owned_batch/prepared_plan.rs +++ b/crates/j2k/src/owned_batch/prepared_plan.rs @@ -22,17 +22,22 @@ pub type PreparedImageGeometry<'a> = j2k_types::J2kReferencedImageGeometry<'a>; #[derive(Debug, Clone)] pub struct PreparedClassicPlan { plan: Arc, + /// The geometry the CPU executors run: for a region, a copy whose bands + /// hold only what the region reads instead of whole levels; otherwise + /// the shared plan. + cpu: Arc, } impl PreparedClassicPlan { - pub(super) fn from_native(plan: ClassicPreparedGeometry) -> Self { - Self { - plan: Arc::new(plan), - } + pub(super) fn from_native(plan: ClassicPreparedGeometry) -> j2k_native::Result { + let cpu = j2k_native::window_referenced_classic_plan_to_stores(&plan)?; + let plan = Arc::new(plan); + let cpu = cpu.map_or_else(|| Arc::clone(&plan), Arc::new); + Ok(Self { plan, cpu }) } - pub(crate) fn native_plan(&self) -> &ClassicPreparedGeometry { - &self.plan + pub(crate) fn cpu_plan(&self) -> &ClassicPreparedGeometry { + &self.cpu } /// Borrow the immutable execution geometry shared by all decode backends. @@ -114,17 +119,22 @@ impl PreparedClassicPlan { #[derive(Debug, Clone)] pub struct PreparedHtj2kPlan { plan: Arc, + /// The geometry the CPU executors run: for a region, a copy whose bands + /// hold only what the region reads instead of whole levels; otherwise + /// the shared plan. + cpu: Arc, } impl PreparedHtj2kPlan { - pub(super) fn from_native(plan: Htj2kPreparedGeometry) -> Self { - Self { - plan: Arc::new(plan), - } + pub(super) fn from_native(plan: Htj2kPreparedGeometry) -> j2k_native::Result { + let cpu = j2k_native::window_referenced_htj2k_plan_to_stores(&plan)?; + let plan = Arc::new(plan); + let cpu = cpu.map_or_else(|| Arc::clone(&plan), Arc::new); + Ok(Self { plan, cpu }) } - pub(crate) fn native_plan(&self) -> &Htj2kPreparedGeometry { - &self.plan + pub(crate) fn cpu_plan(&self) -> &Htj2kPreparedGeometry { + &self.cpu } /// Borrow the immutable execution geometry shared by all decode backends. diff --git a/crates/j2k/tests/owned_batch.rs b/crates/j2k/tests/owned_batch.rs index 537fc8815..d2a466114 100644 --- a/crates/j2k/tests/owned_batch.rs +++ b/crates/j2k/tests/owned_batch.rs @@ -17,6 +17,8 @@ mod grouping; mod native_types_and_requests; #[path = "owned_batch/payload_ranges.rs"] mod payload_ranges; +#[path = "owned_batch/region_windows.rs"] +mod region_windows; #[path = "owned_batch/reuse.rs"] mod reuse; #[path = "owned_batch/rgba.rs"] diff --git a/crates/j2k/tests/owned_batch/fixtures.rs b/crates/j2k/tests/owned_batch/fixtures.rs index 8a7f437ab..9ae178075 100644 --- a/crates/j2k/tests/owned_batch/fixtures.rs +++ b/crates/j2k/tests/owned_batch/fixtures.rs @@ -173,6 +173,15 @@ fn rgba_channel_definitions(alpha_type: J2kChannelType) -> [J2kChannelDefinition } pub(super) fn wrap_rgba_jph(codestream: &[u8], alpha: Htj2kRgbaAlpha) -> Vec { + wrap_rgba(codestream, alpha, J2kFileWrapOptions::jph()) +} + +/// Wrap a classic four-component codestream as JP2 with explicit alpha. +pub(super) fn wrap_rgba_jp2(codestream: &[u8], alpha: Htj2kRgbaAlpha) -> Vec { + wrap_rgba(codestream, alpha, J2kFileWrapOptions::jp2()) +} + +fn wrap_rgba(codestream: &[u8], alpha: Htj2kRgbaAlpha, options: J2kFileWrapOptions) -> Vec { let alpha_type = match alpha { Htj2kRgbaAlpha::Straight => J2kChannelType::Opacity, Htj2kRgbaAlpha::Premultiplied => J2kChannelType::PremultipliedOpacity, @@ -180,7 +189,7 @@ pub(super) fn wrap_rgba_jph(codestream: &[u8], alpha: Htj2kRgbaAlpha) -> Vec let channel_definitions = rgba_channel_definitions(alpha_type); wrap_j2k_codestream( codestream, - J2kFileWrapOptions::jph() + options .with_color(J2kFileColorSpec::Enumerated(Colorspace::SRgb)) .with_metadata(J2kFileBoxMetadata { palette: None, @@ -188,7 +197,7 @@ pub(super) fn wrap_rgba_jph(codestream: &[u8], alpha: Htj2kRgbaAlpha) -> Vec channel_definitions: &channel_definitions, }), ) - .expect("wrap explicit HTJ2K RGBA image") + .expect("wrap explicit RGBA image") } pub(super) fn signed_gray16_fixture(samples: &[i16], width: u32, height: u32) -> Vec { diff --git a/crates/j2k/tests/owned_batch/region_windows.rs b/crates/j2k/tests/owned_batch/region_windows.rs new file mode 100644 index 000000000..3f647881e --- /dev/null +++ b/crates/j2k/tests/owned_batch/region_windows.rs @@ -0,0 +1,404 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Region requests decode through prepared plans sized to the region rather +//! than to whole resolution levels. + +use std::{num::NonZeroUsize, sync::Arc}; + +use j2k::{ + BatchDecodeOptions, BatchLayout, CpuBatchDecoder, DecodeRequest, Downscale, EncodedImage, + PreparationDepth, Rect, +}; +use j2k_native::{encode, encode_htj2k, EncodeOptions}; +use j2k_test_support::Htj2kRgbaAlpha; + +use super::fixtures::{wrap_rgba_jp2, wrap_rgba_jph}; +use super::oracles::{decoded_samples_for_source, native_request_oracle}; + +/// Neutral RGB codestream re-declared as one 18000 x 12000 tile. Every packet +/// is empty, so it is cheap to build and decode, but a whole level-0 plane is +/// far larger than the default decode allocation limit. +fn empty_large_rgb_codestream(ht: bool) -> Vec { + let options = EncodeOptions { + reversible: true, + num_decomposition_levels: 5, + ..EncodeOptions::default() + }; + let codec = if ht { encode_htj2k } else { encode }; + let mut bytes = + codec(&[128; 32 * 32 * 3], 32, 32, 3, 8, false, &options).expect("encode neutral RGB8"); + let siz = bytes + .windows(2) + .position(|marker| marker == [0xff, 0x51]) + .expect("SIZ"); + for (offset, value) in [(6, 18_000_u32), (10, 12_000), (22, 18_000), (26, 12_000)] { + bytes[siz + offset..siz + offset + 4].copy_from_slice(&value.to_be_bytes()); + } + bytes +} + +/// Deterministic samples with smooth gradients, edges, and per-component +/// variation, so every sub-band carries coefficients. +fn textured_samples(width: u32, height: u32, components: u32) -> Vec { + (0..height) + .flat_map(|y| (0..width).flat_map(move |x| (0..components).map(move |c| (x, y, c)))) + .map(|(x, y, c)| { + let gradient = x * 3 + y * 2 + c * 61; + let edge = if (x / 23 + y / 17 + c) % 2 == 0 { + 0 + } else { + 97 + }; + let noise = (x.wrapping_mul(73_856_093) ^ y.wrapping_mul(19_349_663) ^ c) >> 7; + u8::try_from((gradient + edge + noise % 29) % 256).expect("byte sample") + }) + .collect() +} + +/// One worker runs each image's plan on its own; several workers share +/// entropy decoding across the batch before each image's IDWT. +const EXECUTION_MODES: [(usize, BatchLayout); 2] = [(1, BatchLayout::Nchw), (4, BatchLayout::Nhwc)]; + +fn decode_and_compare(encoded: &Arc<[u8]>, requests: &[DecodeRequest], case: &str) { + for (workers, layout) in EXECUTION_MODES { + let case = format!("{case} workers={workers} {layout:?}"); + decode_and_compare_with(encoded, requests, &case, NonZeroUsize::new(workers), layout); + } +} + +fn decode_and_compare_with( + encoded: &Arc<[u8]>, + requests: &[DecodeRequest], + case: &str, + workers: Option, + layout: BatchLayout, +) { + let mut options = BatchDecodeOptions::default(); + options.workers = workers; + options.layout = layout; + let mut decoder = CpuBatchDecoder::new(options); + let prepared = decoder + .prepare( + requests + .iter() + .map(|&request| EncodedImage::new(Arc::clone(encoded), request)) + .collect(), + ) + .unwrap_or_else(|error| panic!("{case}: prepare: {error}")); + assert!( + prepared.errors().is_empty(), + "{case}: {:?}", + prepared.errors() + ); + let images = prepared + .groups() + .iter() + .flat_map(j2k::PreparedBatchGroup::images) + .collect::>(); + assert!( + images + .iter() + .all(|image| image.preparation_depth() != PreparationDepth::MetadataOnly), + "{case}: every request must keep a prepared offset plan" + ); + + let result = decoder + .decode_prepared(&prepared) + .unwrap_or_else(|error| panic!("{case}: decode: {error}")); + assert!(result.errors().is_empty(), "{case}: {:?}", result.errors()); + for image in images { + let source_index = image.source_index(); + assert_eq!( + decoded_samples_for_source(&result, source_index), + native_request_oracle(image, layout), + "{case} request={:?}", + requests[source_index] + ); + } +} + +#[test] +fn cpu_batch_decodes_small_regions_of_very_large_images() { + // A whole level-0 plane of this image needs 864 MB per component; the + // region needs a few code-blocks. + let roi = Rect { + x: 8_501, + y: 5_503, + w: 16, + h: 16, + }; + let requests = [ + DecodeRequest::Region { roi }, + DecodeRequest::RegionReduced { + roi, + scale: Downscale::Half, + }, + ]; + for ht in [false, true] { + let encoded = Arc::<[u8]>::from(empty_large_rgb_codestream(ht)); + decode_and_compare(&encoded, &requests, &format!("large ht={ht}")); + } +} + +#[test] +fn region_plans_keep_whole_level_geometry_for_every_backend() { + // Metal and CUDA execute the prepared geometry as whole levels; only the + // CPU executors run the region-sized copy. + let roi = Rect { + x: 8_501, + y: 5_503, + w: 16, + h: 16, + }; + for ht in [false, true] { + let encoded = Arc::<[u8]>::from(empty_large_rgb_codestream(ht)); + let mut options = BatchDecodeOptions::default(); + options.workers = NonZeroUsize::new(1); + let prepared = CpuBatchDecoder::new(options) + .prepare(vec![EncodedImage::new( + encoded, + DecodeRequest::Region { roi }, + )]) + .expect("prepare large region"); + let image = &prepared.groups()[0].images()[0]; + let tiles = image.htj2k_plan().map_or_else( + || { + image + .classic_plan() + .expect("classic offset plan") + .geometry() + .tiles() + }, + |plan| plan.geometry().tiles(), + ); + let components = &tiles[0].color_geometry().expect("RGB tile").component_plans; + for component in components { + let last_idwt = component + .steps + .iter() + .rev() + .find_map(|step| match step { + j2k_native::J2kDirectGrayscaleStep::Idwt(idwt) => Some(idwt.rect), + _ => None, + }) + .expect("level-0 IDWT"); + assert_eq!( + (last_idwt.width(), last_idwt.height()), + (18_000, 12_000), + "ht={ht}: shared geometry must keep whole levels" + ); + } + } +} + +#[test] +fn cpu_batch_region_windows_match_single_image_decode() { + const WIDTH: u32 = 389; + const HEIGHT: u32 = 263; + let regions = [ + (0, 0, 1, 1), + (WIDTH - 1, HEIGHT - 1, 1, 1), + (37, 19, 64, 41), + (1, 1, WIDTH - 2, HEIGHT - 2), + (150, 0, 17, HEIGHT), + (0, 131, WIDTH, 3), + (201, 97, 63, 65), + (0, 0, WIDTH, HEIGHT), + ]; + let requests = regions + .iter() + .flat_map(|&(x, y, w, h)| { + let roi = Rect { x, y, w, h }; + [ + DecodeRequest::Region { roi }, + DecodeRequest::RegionReduced { + roi, + scale: Downscale::Half, + }, + DecodeRequest::RegionReduced { + roi, + scale: Downscale::Quarter, + }, + ] + }) + .collect::>(); + + for components in [1_u16, 3] { + let samples = textured_samples(WIDTH, HEIGHT, u32::from(components)); + for ht in [false, true] { + for reversible in [true, false] { + // Exponents are stored minus 2: 4 is 64 x 64, 2 is 16 x 16. + for (tile_size, code_block_exp) in [(None, 4), (None, 2), (Some((128, 96)), 2)] { + let options = EncodeOptions { + reversible, + num_decomposition_levels: 5, + code_block_width_exp: code_block_exp, + code_block_height_exp: code_block_exp, + tile_size, + use_mct: components == 3, + ..EncodeOptions::default() + }; + let codec = if ht { encode_htj2k } else { encode }; + let encoded = Arc::<[u8]>::from( + codec(&samples, WIDTH, HEIGHT, components, 8, false, &options) + .expect("encode textured fixture"), + ); + let case = format!( + "components={components} ht={ht} reversible={reversible} \ + tiles={tile_size:?} code_block_exp={code_block_exp}" + ); + decode_and_compare(&encoded, &requests, &case); + } + } + } + } +} + +#[test] +fn cpu_batch_region_windows_match_single_image_decode_with_three_levels() { + // The fixture and regions j2k-metal checks its region batches against. + let (width, height) = (300_u32, 260_u32); + let regions = [ + ((0, 0, 40, 36), Downscale::None), + ((131, 97, 23, 41), Downscale::None), + ((251, 213, 49, 47), Downscale::None), + ((100, 64, 96, 80), Downscale::Half), + ((37, 150, 120, 110), Downscale::Quarter), + ]; + let requests = regions + .iter() + .map(|&((x, y, w, h), scale)| { + let roi = Rect { x, y, w, h }; + if scale == Downscale::None { + DecodeRequest::Region { roi } + } else { + DecodeRequest::RegionReduced { roi, scale } + } + }) + .collect::>(); + // Four components exercise the RGBA plan, with explicit alpha. + for components in [1_u16, 3, 4] { + let pixels: Vec = (0..width * height * u32::from(components)) + .map(|index| { + let pixel = index / u32::from(components); + let (x, y) = (pixel % width, pixel / width); + u8::try_from((x * 7 + y * 13 + (x * y) / 5 + index % 3 * 41) & 0xff) + .expect("masked") + }) + .collect(); + for ht in [false, true] { + for reversible in [true, false] { + let options = EncodeOptions { + reversible, + use_mct: components >= 3, + num_decomposition_levels: 3, + code_block_width_exp: 4, + code_block_height_exp: 4, + ..EncodeOptions::default() + }; + let codec = if ht { encode_htj2k } else { encode }; + let codestream = codec(&pixels, width, height, components, 8, false, &options) + .expect("encode three-level fixture"); + let encoded = Arc::<[u8]>::from(match (components, ht) { + (4, true) => wrap_rgba_jph(&codestream, Htj2kRgbaAlpha::Straight), + (4, false) => wrap_rgba_jp2(&codestream, Htj2kRgbaAlpha::Straight), + _ => codestream, + }); + let case = format!("components={components} ht={ht} reversible={reversible}"); + decode_and_compare(&encoded, &requests, &case); + } + } + } +} + +#[test] +#[ignore = "release large-region CPU batch characterization; requires J2K_LARGE_REGION_INPUT"] +fn local_large_region_cpu_batch_characterization() { + use j2k::{CpuBatchSamples, CpuDecodeParallelism, J2kDecoder, J2kScratchPool, PixelFormat}; + + let path = std::env::var_os("J2K_LARGE_REGION_INPUT").expect("trusted large codestream"); + let bytes: Arc<[u8]> = std::fs::read(path).expect("large codestream").into(); + for (reduction, scale) in [Downscale::None, Downscale::Half, Downscale::Quarter] + .into_iter() + .enumerate() + { + let side = 1024 * scale.denominator(); + let rois: Vec<_> = (0..4) + .map(|index| Rect { + x: 2048 + (index % 2) * side, + y: 1024 + (index / 2) * side, + w: side, + h: side, + }) + .collect(); + let inputs: Vec<_> = rois + .iter() + .map(|&roi| { + let request = if scale == Downscale::None { + DecodeRequest::Region { roi } + } else { + DecodeRequest::RegionReduced { roi, scale } + }; + EncodedImage::new(bytes.clone(), request) + }) + .collect(); + for sample in 0..7 { + let mut options = BatchDecodeOptions::default(); + options.layout = BatchLayout::Nhwc; + let mut decoder = CpuBatchDecoder::new(options); + let started = std::time::Instant::now(); + let prepared = decoder.prepare(inputs.clone()).expect("prepare"); + let result = decoder.decode_prepared(&prepared).expect("decode"); + let batch_ms = started.elapsed().as_secs_f64() * 1000.0; + // Report rather than fail, so the same run characterizes builds + // that cannot decode a level. + if let Some(error) = result.errors().first() { + eprintln!("large_region level={reduction} cpu_batch_error={error}"); + break; + } + + let started = std::time::Instant::now(); + let tiles = std::thread::scope(|scope| { + let handles: Vec<_> = rois + .iter() + .map(|&roi| { + let bytes = &bytes; + scope.spawn(move || { + let mut decoder = J2kDecoder::new(bytes).expect("CPU decoder"); + decoder.set_cpu_decode_parallelism(CpuDecodeParallelism::Serial); + let mut rgb = vec![0; 1024 * 1024 * 3]; + decoder + .decode_region_scaled_pow2_into( + &mut J2kScratchPool::default(), + &mut rgb, + 1024 * 3, + PixelFormat::Rgb8, + roi, + u8::try_from(reduction).expect("reduction"), + ) + .expect("CPU region decode"); + rgb + }) + }) + .collect(); + handles + .into_iter() + .map(|handle| handle.join().expect("tile thread")) + .collect::>() + }); + let tiles_ms = started.elapsed().as_secs_f64() * 1000.0; + if sample == 0 { + for (index, tile) in tiles.iter().enumerate() { + let CpuBatchSamples::U8(samples) = decoded_samples_for_source(&result, index) + else { + panic!("RGB8 batch samples"); + }; + assert!(samples == *tile, "level {reduction} tile {index} differs"); + } + } + eprintln!( + "large_region level={reduction} sample={sample} tiles=4 cpu_batch_ms={batch_ms:.3} cpu_tiles_ms={tiles_ms:.3}" + ); + } + } +} diff --git a/docs/benchmark-evidence.md b/docs/benchmark-evidence.md index 7510b5607..a1a53993f 100644 --- a/docs/benchmark-evidence.md +++ b/docs/benchmark-evidence.md @@ -19,6 +19,40 @@ cargo run -p xtask --features adoption -- adoption-report --run-dir target/j2k-a Speed comparisons must use the manifest-backed external rows, not the generated in-repo fixtures. +## CPU owned batches of large-image regions - 2026-10-29 + +Same machine and inputs as the next section, load average 4 to 7. Each sample +decodes one round of four 1024 px output tiles at one reduction through +`CpuBatchDecoder::prepare` plus `decode_prepared` with default options; +"CPU tiles" is one serial `decode_region_scaled_pow2_into` per tile, the four +in parallel. The first sample checks that both produce the same bytes. Seven +samples, median ms. "Before" is `main` at 6ca1bdbf with the same test file. + +```bash +J2K_LARGE_REGION_INPUT= cargo test --release -p j2k \ + --test owned_batch local_large_region_cpu_batch_characterization \ + -- --ignored --nocapture +``` + +| Input | Level | CPU batch before | CPU batch now | CPU tiles | +| --- | ---: | ---: | ---: | ---: | +| large-ht.j2c | 0 | `ImageTooLarge` | 28 | 26 | +| large-ht.j2c | 1 | `ImageTooLarge` | 27 | 26 | +| large-ht.j2c | 2 | 48 | 24 | 22 | +| large.jp2 | 0 | `ImageTooLarge` | 152 | 149 | +| large.jp2 | 1 | `ImageTooLarge` | 244 | 229 | +| large.jp2 | 2 | 346 | 317 | 312 | +| large-plt.jp2 | 0 | `ImageTooLarge` | 136 | 133 | +| large-plt.jp2 | 1 | `ImageTooLarge` | 224 | 220 | +| large-plt.jp2 | 2 | 343 | 310 | 306 | + +Small slide tiles, where the rule keeps the whole-level plan: 32 level-0 +`CMU-1-JP2K-33005` tiles (240 x 240), batched with one worker per core, took +9.8 to 10.0 ms before and after for full requests and 8.8 to 9.0 ms before +against 9.1 to 9.2 ms after for a centred half-size region. Preparation, which +now weighs the windowed copy, accounts for 0.1 ms of that. Windowing those +regions without the rule measured 9.5 to 9.7 ms. + ## Metal owned batches of large-image regions - 2026-10-28 Same machine as the next section, load average 5 to 11. Inputs are the diff --git a/docs/public-support.md b/docs/public-support.md index 247f6e06d..b3179452f 100644 --- a/docs/public-support.md +++ b/docs/public-support.md @@ -44,7 +44,11 @@ The owned-batch decoder accepts full, region, reduced, and region-plus-reduced requests and groups Gray, RGB, or RGBA output by shape without padding. Output is `U8`, `U16`, or `I16` samples in NCHW or NHWC layout. When components use different sampling factors, the subsampled ones are replicated onto the -image grid, as the single-image decoders do. Images whose components all +image grid, as the single-image decoders do. On the CPU, a region request runs a +copy of the prepared plan sized to the region when that copy holds at most +half the coefficients, or when the whole-level plan holds 16 Mi or more; +otherwise it keeps whole levels. The prepared geometry GPU backends read +always keeps whole levels. Images whose components all share one subsampling decode to that smaller grid and are not batched. Other component counts, mixed component metadata, and precision above 16 bits use the component-plane APIs. Explicit GPU requests return an error for From 05d5220a87987974104630d8ccb0f777273e0332 Mon Sep 17 00:00:00 2001 From: GF Date: Wed, 28 Oct 2026 23:49:50 -0400 Subject: [PATCH 02/10] fix(j2k-native): leave out tiles a reduced region does not reach 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 --- CHANGELOG.md | 8 ++ .../src/batch_decoder/region_group_tests.rs | 43 +++++++ .../j2k-native/src/j2c/decode/direct_plan.rs | 16 ++- .../decode/direct_plan/referenced_color.rs | 14 ++- .../direct_plan/referenced_grayscale.rs | 12 +- crates/j2k/tests/owned_batch.rs | 2 + crates/j2k/tests/owned_batch/oracles.rs | 17 ++- .../tests/owned_batch/reduced_region_tiles.rs | 117 ++++++++++++++++++ 8 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 crates/j2k/tests/owned_batch/reduced_region_tiles.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index e7ca96c9c..e831ea8e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,14 @@ and stale roadmap entries have been removed from the public documentation set. IDWT is slower per sample than the banded whole-level one; full and reduced requests, and regions covering most of a small image, are unchanged. See `docs/benchmark-evidence.md`. +- Owned-batch preparation no longer fails with "failed to decode code-block" + on reduced-resolution regions that end where a tile begins on the reduced + grid ([#298](https://github.com/frames-sg/j2k/issues/298)). Tile selection + rounds tile bounds outward, so it can keep a boundary tile that contributes + no samples; the referenced plan builders rejected such a tile and now leave + it out. This affected the CPU and Metal batches alike. On the 19 x 13 + OpenJPH fixture in 11 x 7 tiles, 4,598 of the 17,290 half-scale regions + failed to prepare; all now match single-image decode. - `j2k-tilecodec` LZW decode no longer reports valid TIFF strips and tiles as truncated. weezl reads input ahead of decoding, so the call that consumes the last byte can still hold codes up to the end code, often in diff --git a/crates/j2k-metal/src/batch_decoder/region_group_tests.rs b/crates/j2k-metal/src/batch_decoder/region_group_tests.rs index 6cc323a3b..e7385b6db 100644 --- a/crates/j2k-metal/src/batch_decoder/region_group_tests.rs +++ b/crates/j2k-metal/src/batch_decoder/region_group_tests.rs @@ -332,3 +332,46 @@ fn local_large_region_batch_characterization() { } } } + +#[test] +fn reduced_regions_ending_at_rounded_tile_edges_match_the_cpu() { + // 19 x 13 OpenJPH images in 11 x 7 tiles: these regions end where a tile + // begins on the reduced grid, so tile selection keeps a tile that + // contributes no samples. + if !j2k_test_support::metal_runtime_gate(module_path!()) { + return; + } + let regions = [ + (0, 0, 11, 1), + (0, 0, 11, 7), + (0, 0, 19, 7), + (10, 6, 2, 2), + (11, 7, 8, 6), + ]; + for fixture in j2k_test_support::openjph_batch_fixtures() + .iter() + .filter(|fixture| fixture.precision == 8 && !fixture.signed) + { + let bytes = Arc::<[u8]>::from(fixture.encoded); + let inputs: Vec<_> = [Downscale::Half, Downscale::Quarter] + .into_iter() + .flat_map(|scale| { + regions + .iter() + .map(move |&(x, y, w, h)| (Rect { x, y, w, h }, scale)) + }) + .map(|(roi, scale)| EncodedImage::new(bytes.clone(), request(roi, scale))) + .collect(); + let mut decoder = + MetalBatchDecoder::system_default_with_options(options()).expect("decoder"); + let expected = cpu_images(&inputs); + let actual = metal_images(&mut decoder, &inputs); + for (index, (actual, expected)) in actual.iter().zip(&expected).enumerate() { + assert!( + actual == expected, + "{} request {index} differs from CPU", + fixture.name + ); + } + } +} diff --git a/crates/j2k-native/src/j2c/decode/direct_plan.rs b/crates/j2k-native/src/j2c/decode/direct_plan.rs index af2463fff..99f9d47a4 100644 --- a/crates/j2k-native/src/j2c/decode/direct_plan.rs +++ b/crates/j2k-native/src/j2c/decode/direct_plan.rs @@ -384,10 +384,13 @@ fn append_classic_payload_records( append_decode_elements(ranges, tile_ranges) } +/// A tile's decoded and destination rectangles, or `None` when it stores +/// nothing: tile selection rounds tile bounds outward onto the reduced grid, +/// so it can keep a boundary tile that contributes no samples. fn grayscale_plan_rects( geometry: &J2kDirectGrayscalePlan, output_rect: J2kRect, -) -> Result<(J2kRect, J2kRect)> { +) -> Result> { let mut stores = geometry.steps.iter().filter_map(|step| match step { J2kDirectGrayscaleStep::Store(store) => Some(store), J2kDirectGrayscaleStep::ClassicSubBand(_) @@ -395,9 +398,12 @@ fn grayscale_plan_rects( | J2kDirectGrayscaleStep::Idwt(_) => None, }); let store = stores.next().ok_or(DecodingError::CodeBlockDecodeFailure)?; - if stores.next().is_some() || store.copy_width == 0 || store.copy_height == 0 { + if stores.next().is_some() { bail!(DecodingError::CodeBlockDecodeFailure); } + if store.copy_width == 0 || store.copy_height == 0 { + return Ok(None); + } let destination_rect = J2kRect { x0: store.output_x, y0: store.output_y, @@ -410,13 +416,15 @@ fn grayscale_plan_rects( x1: output_rect.x0.saturating_add(destination_rect.x1), y1: output_rect.y0.saturating_add(destination_rect.y1), }; - Ok((decoded_rect, destination_rect)) + Ok(Some((decoded_rect, destination_rect))) } +/// The rectangles every component of a tile shares, or `None` when none of +/// them stores anything. fn color_plan_rects( component_plans: &[J2kDirectGrayscalePlan], output_rect: J2kRect, -) -> Result<(J2kRect, J2kRect)> { +) -> Result> { if component_plans.len() != COMPONENT_COUNT { bail!(DecodingError::CodeBlockDecodeFailure); } diff --git a/crates/j2k-native/src/j2c/decode/direct_plan/referenced_color.rs b/crates/j2k-native/src/j2c/decode/direct_plan/referenced_color.rs index c42e22201..a5a084ce7 100644 --- a/crates/j2k-native/src/j2c/decode/direct_plan/referenced_color.rs +++ b/crates/j2k-native/src/j2c/decode/direct_plan/referenced_color.rs @@ -121,11 +121,14 @@ fn build_referenced_htj2k_color_components_plan<'a, const COMPONENT_COUNT: usize &mut plans.component_plans, tile_classic_payloads.len(), )?; + let Some((decoded_rect, destination_rect)) = + color_plan_rects::(&plans.component_plans, output_rect)? + else { + continue; + }; let record_count = tile_payloads.len(); append_retained_records(&mut retained, &mut payloads, &mut tile_payloads)?; let payload_records = payload_record_span(first_record, record_count)?; - let (decoded_rect, destination_rect) = - color_plan_rects::(&plans.component_plans, output_rect)?; let geometry = into_geometry(plans); tile_plans.push(J2kReferencedTilePlan::new( usize::try_from(tile.idx).map_err(|_| ValidationError::ImageTooLarge)?, @@ -259,6 +262,11 @@ fn build_referenced_classic_color_components_plan<'a, const COMPONENT_COUNT: usi &mut plans.component_plans, tile_payloads.len(), )?; + let Some((decoded_rect, destination_rect)) = + color_plan_rects::(&plans.component_plans, output_rect)? + else { + continue; + }; let record_count = tile_payloads.len(); append_classic_payload_records( &mut payloads, @@ -267,8 +275,6 @@ fn build_referenced_classic_color_components_plan<'a, const COMPONENT_COUNT: usi &mut tile_ranges, )?; let payload_records = payload_record_span(first_record, record_count)?; - let (decoded_rect, destination_rect) = - color_plan_rects::(&plans.component_plans, output_rect)?; tile_plans.push(J2kReferencedTilePlan::new( usize::try_from(tile.idx).map_err(|_| ValidationError::ImageTooLarge)?, decoded_rect, diff --git a/crates/j2k-native/src/j2c/decode/direct_plan/referenced_grayscale.rs b/crates/j2k-native/src/j2c/decode/direct_plan/referenced_grayscale.rs index b64a48bed..5c3507efb 100644 --- a/crates/j2k-native/src/j2c/decode/direct_plan/referenced_grayscale.rs +++ b/crates/j2k-native/src/j2c/decode/direct_plan/referenced_grayscale.rs @@ -66,9 +66,13 @@ pub(crate) fn build_referenced_htj2k_grayscale_plan<'a>( if classic_job_count != tile_classic_payloads.len() { bail!(DecodingError::CodeBlockDecodeFailure); } + let Some((decoded_rect, destination_rect)) = + grayscale_plan_rects(&geometry, output_rect)? + else { + continue; + }; append_retained_records(&mut retained, &mut payloads, &mut tile_payloads)?; let payload_records = payload_record_span(first_record, record_count)?; - let (decoded_rect, destination_rect) = grayscale_plan_rects(&geometry, output_rect)?; tile_plans.push(J2kReferencedTilePlan::new( usize::try_from(tile.idx).map_err(|_| ValidationError::ImageTooLarge)?, decoded_rect, @@ -143,6 +147,11 @@ pub(crate) fn build_referenced_classic_grayscale_plan<'a>( if job_count != tile_payloads.len() { bail!(DecodingError::CodeBlockDecodeFailure); } + let Some((decoded_rect, destination_rect)) = + grayscale_plan_rects(&geometry, output_rect)? + else { + continue; + }; append_classic_payload_records( &mut payloads, &mut ranges, @@ -150,7 +159,6 @@ pub(crate) fn build_referenced_classic_grayscale_plan<'a>( &mut tile_ranges, )?; let payload_records = payload_record_span(first_record, job_count)?; - let (decoded_rect, destination_rect) = grayscale_plan_rects(&geometry, output_rect)?; tile_plans.push(J2kReferencedTilePlan::new( usize::try_from(tile.idx).map_err(|_| ValidationError::ImageTooLarge)?, decoded_rect, diff --git a/crates/j2k/tests/owned_batch.rs b/crates/j2k/tests/owned_batch.rs index d2a466114..b8ef9d643 100644 --- a/crates/j2k/tests/owned_batch.rs +++ b/crates/j2k/tests/owned_batch.rs @@ -17,6 +17,8 @@ mod grouping; mod native_types_and_requests; #[path = "owned_batch/payload_ranges.rs"] mod payload_ranges; +#[path = "owned_batch/reduced_region_tiles.rs"] +mod reduced_region_tiles; #[path = "owned_batch/region_windows.rs"] mod region_windows; #[path = "owned_batch/reuse.rs"] diff --git a/crates/j2k/tests/owned_batch/oracles.rs b/crates/j2k/tests/owned_batch/oracles.rs index c3510da4b..a45ecbff9 100644 --- a/crates/j2k/tests/owned_batch/oracles.rs +++ b/crates/j2k/tests/owned_batch/oracles.rs @@ -30,17 +30,22 @@ pub(super) fn native_request_oracle( let pixel_count = output.w as usize * output.h as usize; let channels = raw.num_components as usize; if raw.signed { - CpuBatchSamples::I16(apply_batch_layout( + // Native output keeps one byte per signed sample up to 8 bits; the + // batch widens every signed sample to i16. + let samples = if raw.bit_depth <= 8 { + raw.data + .iter() + .map(|&sample| i16::from(i8::from_ne_bytes([sample]))) + .collect() + } else { raw.data .as_chunks::<2>() .0 .iter() .map(|sample| i16::from_le_bytes(*sample)) - .collect(), - pixel_count, - channels, - layout, - )) + .collect() + }; + CpuBatchSamples::I16(apply_batch_layout(samples, pixel_count, channels, layout)) } else if raw.bit_depth <= 8 { CpuBatchSamples::U8(apply_batch_layout(raw.data, pixel_count, channels, layout)) } else { diff --git a/crates/j2k/tests/owned_batch/reduced_region_tiles.rs b/crates/j2k/tests/owned_batch/reduced_region_tiles.rs new file mode 100644 index 000000000..3f29a674c --- /dev/null +++ b/crates/j2k/tests/owned_batch/reduced_region_tiles.rs @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Reduced regions that end where a tile begins once tile bounds are rounded +//! onto the reduced grid. +//! +//! The `OpenJPH` fixtures are 19 x 13 images in 11 x 7 tiles. At half scale the +//! second tile column starts at column 6, but tile selection rounds bounds +//! outward and keeps that tile for regions ending at column 6; the tile then +//! contributes no samples. + +use std::{num::NonZeroUsize, sync::Arc}; + +use j2k::{ + BatchDecodeOptions, BatchLayout, CpuBatchDecoder, DecodeRequest, Downscale, EncodedImage, Rect, +}; +use j2k_test_support::openjph_batch_fixtures; + +use super::oracles::{decoded_samples_for_source, native_request_oracle}; + +fn assert_batch_matches_single_image(encoded: &Arc<[u8]>, requests: &[DecodeRequest], case: &str) { + let mut options = BatchDecodeOptions::default(); + options.workers = NonZeroUsize::new(4); + options.layout = BatchLayout::Nhwc; + let mut decoder = CpuBatchDecoder::new(options); + let prepared = decoder + .prepare( + requests + .iter() + .map(|&request| EncodedImage::new(Arc::clone(encoded), request)) + .collect(), + ) + .unwrap_or_else(|error| panic!("{case}: prepare: {error}")); + let failures = prepared + .errors() + .iter() + .take(3) + .map(|error| format!("{:?}: {error}", requests[error.index])) + .collect::>(); + assert!( + failures.is_empty(), + "{case}: {} of {} requests failed to prepare, first {failures:?}", + prepared.errors().len(), + requests.len() + ); + + let result = decoder + .decode_prepared(&prepared) + .unwrap_or_else(|error| panic!("{case}: decode: {error}")); + assert!(result.errors().is_empty(), "{case}: {:?}", result.errors()); + for image in prepared + .groups() + .iter() + .flat_map(j2k::PreparedBatchGroup::images) + { + let source_index = image.source_index(); + assert_eq!( + decoded_samples_for_source(&result, source_index), + native_request_oracle(image, BatchLayout::Nhwc), + "{case} request={:?}", + requests[source_index] + ); + } +} + +#[test] +fn every_half_scale_region_of_a_tiled_image_prepares_and_decodes() { + // The reproduction in #298: about a quarter of these failed to prepare. + let fixture = openjph_batch_fixtures() + .iter() + .find(|fixture| fixture.name == "openjph-rgb-u8-53-raw") + .expect("tiled RGB fixture"); + let (width, height) = (fixture.width, fixture.height); + let requests = (0..height) + .flat_map(|y| (0..width).map(move |x| (x, y))) + .flat_map(|(x, y)| { + (1..=height - y).flat_map(move |h| { + (1..=width - x).map(move |w| DecodeRequest::RegionReduced { + roi: Rect { x, y, w, h }, + scale: Downscale::Half, + }) + }) + }) + .collect::>(); + let encoded = Arc::<[u8]>::from(fixture.encoded); + assert_batch_matches_single_image(&encoded, &requests, fixture.name); +} + +#[test] +fn reduced_regions_ending_at_rounded_tile_edges_prepare_for_every_fixture() { + // Regions that end at, start at, or straddle the tile edges at x = 11 and + // y = 7, at both reductions the two-level fixtures allow. + let regions = [ + (0, 0, 11, 1), + (0, 0, 11, 7), + (0, 0, 19, 7), + (0, 0, 11, 13), + (10, 6, 2, 2), + (11, 7, 8, 6), + (5, 3, 7, 5), + (0, 0, 19, 13), + ]; + for fixture in openjph_batch_fixtures() { + let requests = [Downscale::Half, Downscale::Quarter] + .into_iter() + .flat_map(|scale| { + regions + .iter() + .map(move |&(x, y, w, h)| DecodeRequest::RegionReduced { + roi: Rect { x, y, w, h }, + scale, + }) + }) + .collect::>(); + let encoded = Arc::<[u8]>::from(fixture.encoded); + assert_batch_matches_single_image(&encoded, &requests, fixture.name); + } +} From 101cd417f5807e0735685df9536f8559ffedb097 Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 01:07:38 -0400 Subject: [PATCH 03/10] chore(semver): review the CPU region window API additions 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. --- xtask/api/stable-api-1.0.implementation-public-api.txt | 2 ++ .../public-api/public-api-review-0.13.0.yml | 6 +++--- .../public-api/reviewed-public-api-diff-0.13.0.md | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/xtask/api/stable-api-1.0.implementation-public-api.txt b/xtask/api/stable-api-1.0.implementation-public-api.txt index a6cdf02dd..319468601 100644 --- a/xtask/api/stable-api-1.0.implementation-public-api.txt +++ b/xtask/api/stable-api-1.0.implementation-public-api.txt @@ -2644,6 +2644,8 @@ pub fn j2k_native::prepare_referenced_htj2k_tile_staged(&j2k_types::decode_plan: pub fn j2k_native::quantize_reversible_reference(&[f32], u16, u16, u8, bool) -> alloc::vec::Vec pub fn j2k_native::quantize_subband_reference(&[f32], u16, u16, u8, bool) -> alloc::vec::Vec pub fn j2k_native::try_deinterleave_reference(&[u8], usize, u16, u8, bool) -> j2k_native::Result>> +pub fn j2k_native::window_referenced_classic_plan_to_stores(&j2k_types::decode_plan::referenced::J2kReferencedClassicPlan) -> j2k_native::Result> +pub fn j2k_native::window_referenced_htj2k_plan_to_stores(&j2k_types::decode_plan::referenced::J2kReferencedHtj2kPlan) -> j2k_native::Result> pub fn j2k_types::decode_plan::J2kRect::from(j2k_native::J2kRequiredBandRegion) -> Self pub j2k_native::ComponentPlaneParts::bit_depth: u8 pub j2k_native::ComponentPlaneParts::dimensions: (u32, u32) diff --git a/xtask/release-evidence/public-api/public-api-review-0.13.0.yml b/xtask/release-evidence/public-api/public-api-review-0.13.0.yml index 2a22d169d..8a1a35036 100644 --- a/xtask/release-evidence/public-api/public-api-review-0.13.0.yml +++ b/xtask/release-evidence/public-api/public-api-review-0.13.0.yml @@ -210,10 +210,10 @@ reviews: j2k-native: removed_fingerprint: "none" added_fingerprint: "fnv1a64:7debc570e375354a" - hidden_count: 582 - hidden_fingerprint: "fnv1a64:56243dbdb7c7dd62" + hidden_count: 584 + hidden_fingerprint: "fnv1a64:0fb38a581f613d0f" rationale: "Compared the ordinary API with v0.12.0; existing signatures are unchanged. Adds selected wavelet extraction and shared wavelet type re-exports. It decodes the requested entropy bands without inverse DWT or color conversion and preserves exact reversible integer coefficients; existing pixel APIs retain their signatures. Also records the already-merged irreversible_quantization_profile option. Profiles supply per-component band weights; callers using Default or struct update syntax retain the previous no-profile behavior. Exhaustive native EncodeOptions literals must add irreversible_quantization_profile: None or use struct update syntax. Adds DecodingError::IncompletePlaneStore, returned when component planes reused without zeroing are not fully written; DecodingError is non_exhaustive." - hidden_rationale: "Adds an HT coding probe, parallel scratch toggle, and J2kClassicReconstruction hooks to the native implementation inventory. Existing classic adapters retain their defaults; supporting reversible midpoint reconstruction requires opting into the new reconstruction hooks. Also records the already-merged inverse_dwt97_reference, inverse_ict_reference, and irreversible_quantization_profile_scale_ceiling helpers. These are additive reference and profile-validation entry points. Adds irreversible_quantization_scale_bounds so facade rate search uses the same weighted-step limits as native quantization, including the HT coded-bitplane ceiling. Existing signatures and the legacy scale-ceiling helper remain compatible; no caller migration is required. irreversible_quantization_step_for_subband also takes the sub-band's decomposition level and the level count, so default 9/7 steps can be weighted by synthesis norm; callers pass both. Adds IrreversibleScaleEncoder so facade rate and PSNR search reuses one colour transform and DWT across quantization-scale candidates; each candidate equals the independent CPU encode at that scale. It is additive; no caller migration is required. Adds the ComponentRowSink trait and Image::decode_region_rows_for_integer_output_with_context, which hand a serial region decode's finished integer-output rows to a sink as disjoint rectangles instead of filling component planes; decodes that cannot stream hand their planes over as one rectangle. They are additive; existing region methods retain their signatures." + hidden_rationale: "Adds an HT coding probe, parallel scratch toggle, and J2kClassicReconstruction hooks to the native implementation inventory. Existing classic adapters retain their defaults; supporting reversible midpoint reconstruction requires opting into the new reconstruction hooks. Also records the already-merged inverse_dwt97_reference, inverse_ict_reference, and irreversible_quantization_profile_scale_ceiling helpers. These are additive reference and profile-validation entry points. Adds irreversible_quantization_scale_bounds so facade rate search uses the same weighted-step limits as native quantization, including the HT coded-bitplane ceiling. Existing signatures and the legacy scale-ceiling helper remain compatible; no caller migration is required. irreversible_quantization_step_for_subband also takes the sub-band's decomposition level and the level count, so default 9/7 steps can be weighted by synthesis norm; callers pass both. Adds IrreversibleScaleEncoder so facade rate and PSNR search reuses one colour transform and DWT across quantization-scale candidates; each candidate equals the independent CPU encode at that scale. It is additive; no caller migration is required. Adds the ComponentRowSink trait and Image::decode_region_rows_for_integer_output_with_context, which hand a serial region decode's finished integer-output rows to a sink as disjoint rectangles instead of filling component planes; decodes that cannot stream hand their planes over as one rectangle. They are additive; existing region methods retain their signatures. Adds window_referenced_classic_plan_to_stores and window_referenced_htj2k_plan_to_stores, which return a copy of a referenced region plan whose sub-bands and IDWT levels hold only what its stores read, or None when that would not pay off; the facade's CPU batch executes the copy while GPU backends keep the shared whole-level plan. They are additive; no caller migration is required." j2k-jpeg: removed_fingerprint: "fnv1a64:c2093d3bff880a40" added_fingerprint: "fnv1a64:dc6430a33ad716c8" diff --git a/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.0.md b/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.0.md index 089058f0b..1b9c1d32c 100644 --- a/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.0.md +++ b/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.0.md @@ -22,7 +22,7 @@ This report is generated by `cargo xtask semver --write-report`. Normal `cargo x | `j2k-cuda-transcode-engine` | `0.12.0` | `0.13.0` | `major` | 0 | 0 | `none` | `none` | 84 | `fnv1a64:b2fcaa9b0171aba7` | | `j2k-metal-support` | `0.12.0` | `0.13.0` | `major` | 3 | 0 | `none` | `fnv1a64:cb0fe39460064a9f` | 11 | `fnv1a64:02139606f1202938` | | `j2k-mpsgraph-support` | `0.12.0` | `0.13.0` | `major` | 0 | 0 | `none` | `none` | 0 | `none` | -| `j2k-native` | `0.12.0` | `0.13.0` | `major` | 12 | 0 | `none` | `fnv1a64:7debc570e375354a` | 582 | `fnv1a64:56243dbdb7c7dd62` | +| `j2k-native` | `0.12.0` | `0.13.0` | `major` | 12 | 0 | `none` | `fnv1a64:7debc570e375354a` | 584 | `fnv1a64:0fb38a581f613d0f` | | `j2k-jpeg` | `0.12.0` | `0.13.0` | `major` | 98 | 1 | `fnv1a64:c2093d3bff880a40` | `fnv1a64:dc6430a33ad716c8` | 701 | `fnv1a64:bd0cf738a0f96592` | | `j2k-tilecodec` | `0.12.0` | `0.13.0` | `major` | 0 | 0 | `none` | `none` | 32 | `fnv1a64:eb677842559d9b0e` | | `j2k` | `0.12.0` | `0.13.0` | `major` | 21 | 6 | `fnv1a64:ffea85eb442c661d` | `fnv1a64:6d29f512d27db446` | 108 | `fnv1a64:01e74ad6c3ff416e` | @@ -248,7 +248,7 @@ None. ### `j2k-native` -Baseline items: 410. Candidate items: 422. Computed release type: `major`. Rustdoc-hidden candidate items: 582. Full hidden-inventory fingerprint: `fnv1a64:56243dbdb7c7dd62`. +Baseline items: 410. Candidate items: 422. Computed release type: `major`. Rustdoc-hidden candidate items: 584. Full hidden-inventory fingerprint: `fnv1a64:0fb38a581f613d0f`. #### Removed or changed baseline API items From a8e87489fee2efd377a4d61af8e12e5c5fbfa63b Mon Sep 17 00:00:00 2001 From: GF Date: Fri, 23 Oct 2026 01:44:43 -0400 Subject: [PATCH 04/10] fix(metal): seed never-included code-blocks with zero in the zero-bitplane tree The Metal packetizer gave a code-block with no coding passes its own zero-bitplane count (all bitplanes) in the zero-bitplane tag tree, while the CPU packetizer seeds it with zero. Decoders never read that leaf, but it lowers or raises the parent minimum, so headers with an empty block beside coded ones differed: all-stage Metal lossy encodes were valid but not byte-identical to the CPU (mosaic qfactor 86: 2 bytes shorter). The Metal tree now seeds such blocks as the CPU does. (cherry picked from commit 575f6d2e2f3b3743d71ce91fbf60c660d55c2873) (cherry picked from commit 6341a18dd2c33d30ece345ddb1b8a2b0c946706b) --- CHANGELOG.md | 4 ++ crates/j2k-metal/src/encode/tests/kernels.rs | 54 +++++++++++++++++++ .../src/encode_bitstream_packetize.metal | 14 +++-- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e831ea8e8..8a794be18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ and stale roadmap entries have been removed from the public documentation set. it out. This affected the CPU and Metal batches alike. On the 19 x 13 OpenJPH fixture in 11 x 7 tiles, 4,598 of the 17,290 half-scale regions failed to prepare; all now match single-image decode. +- All-stage Metal lossy encodes seed code-blocks without coding passes with zero + in the zero-bitplane tag tree, as the CPU encoder does, so their packet headers + match the CPU's byte for byte. Earlier codestreams were valid but could differ + from the CPU's by a few header bytes. - `j2k-tilecodec` LZW decode no longer reports valid TIFF strips and tiles as truncated. weezl reads input ahead of decoding, so the call that consumes the last byte can still hold codes up to the end code, often in diff --git a/crates/j2k-metal/src/encode/tests/kernels.rs b/crates/j2k-metal/src/encode/tests/kernels.rs index dd46ba104..ac230ea91 100644 --- a/crates/j2k-metal/src/encode/tests/kernels.rs +++ b/crates/j2k-metal/src/encode/tests/kernels.rs @@ -546,6 +546,60 @@ fn metal_htj2k_quality_layers_use_native_candidates_and_match_scalar_output() { assert!(accelerator.ht_code_block_dispatches() > 0); } +#[cfg(target_os = "macos")] +#[test] +fn metal_all_stage_lossy_encodes_match_cpu_bytes_with_empty_code_blocks() { + if !should_run_metal_runtime() { + return; + } + + // The left half is textured and the right half flat, so detail bands hold + // empty code-blocks beside coded ones in the same tag trees. + let (width, height) = (256_u32, 256_u32); + let pixels: Vec = (0..width * height * 3) + .map(|idx| { + let x = (idx / 3) % width; + let texture = if x < width / 2 { + (idx * 37 + idx / 7) & 0x07 + } else { + 0 + }; + u8::try_from((x / 4 + texture) & 0xFF).expect("masked pixel") + }) + .collect(); + let samples = + J2kLossySamples::new(&pixels, width, height, 3, 8, false).expect("valid RGB samples"); + for (mode, qfactor) in [ + (J2kBlockCodingMode::HighThroughput, Some(60)), + (J2kBlockCodingMode::HighThroughput, None), + (J2kBlockCodingMode::Classic, None), + ] { + let options = J2kLossyEncodeOptions::default() + .with_block_coding_mode(mode) + .with_max_decomposition_levels(Some(2)) + .with_qfactor(qfactor) + .with_validation(J2kEncodeValidation::External); + let cpu = j2k::encode_j2k_lossy( + samples, + &options + .clone() + .with_backend(EncodeBackendPreference::CpuOnly), + ) + .expect("CPU lossy encode"); + let mut accelerator = MetalEncodeStageAccelerator::default(); + let metal = encode_j2k_lossy_with_accelerator( + samples, + &options.with_backend(EncodeBackendPreference::RequireDevice), + BackendKind::Metal, + &mut accelerator, + ) + .expect("Metal all-stage lossy encode"); + + assert_eq!(metal.backend, BackendKind::Metal, "{mode:?} {qfactor:?}"); + assert_eq!(metal.codestream, cpu.codestream, "{mode:?} {qfactor:?}"); + } +} + #[cfg(target_os = "macos")] #[test] fn metal_htj2k_lossy_rgb_facade_reports_forward_ict_dispatch() { diff --git a/crates/j2k-metal/src/encode_bitstream_packetize.metal b/crates/j2k-metal/src/encode_bitstream_packetize.metal index 2a8f6cf1f..7b760eb35 100644 --- a/crates/j2k-metal/src/encode_bitstream_packetize.metal +++ b/crates/j2k-metal/src/encode_bitstream_packetize.metal @@ -830,9 +830,12 @@ inline bool j2k_packet_prepare_tree( } for (uint idx = 0u; idx < block_count; ++idx) { const J2kPacketBlock block = blocks[block_offset + idx]; + // A block that is never included contributes zero to the zero-bitplane + // tree, as the CPU packetizer seeds it, so headers match byte for byte. + const bool included = block.num_coding_passes > 0u; value[idx] = zero_bitplanes - ? block.num_zero_bitplanes - : (block.num_coding_passes > 0u ? inclusion_layer : 0x7FFFFFFFu); + ? (included ? block.num_zero_bitplanes : 0u) + : (included ? inclusion_layer : 0x7FFFFFFFu); } for (uint level = 1u; level < levels; ++level) { const uint prev_w = level_widths[level - 1u]; @@ -910,9 +913,12 @@ inline bool j2k_packet_prepare_tree_resident_classic( tier1_job_count, block_offset + idx ); + // A block that is never included contributes zero to the zero-bitplane + // tree, as the CPU packetizer seeds it, so headers match byte for byte. + const bool included = block.num_coding_passes > 0u; value[idx] = zero_bitplanes - ? block.num_zero_bitplanes - : (block.num_coding_passes > 0u ? inclusion_layer : 0x7FFFFFFFu); + ? (included ? block.num_zero_bitplanes : 0u) + : (included ? inclusion_layer : 0x7FFFFFFFu); } for (uint level = 1u; level < levels; ++level) { const uint prev_w = level_widths[level - 1u]; From 5486a3a5057d9c2675da9ebe8520926d8d572a5f Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:16:00 -0400 Subject: [PATCH 05/10] perf(j2k-native): read MQ code bytes ahead of the C register The classic MQ decoder ran BYTEIN once per code byte, from the renormalization slow path, so about one symbol in ten of a lossless decode took that path. C is now 64 bits wide: the half that DECODE compares with A sits in bits 48-63, and up to 48 bits of code bytes are read ahead below it, several bytes per refill. A byte after 0xFF may carry into the bits above it, so it is still read only once no read-ahead bits remain, as in Annex C. Every other byte lands on zero bits, so reading it early changes nothing DECODE sees. Output is unchanged. --- .../j2k-native/src/j2c/arithmetic_decoder.rs | 65 ++++++++++--------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/crates/j2k-native/src/j2c/arithmetic_decoder.rs b/crates/j2k-native/src/j2c/arithmetic_decoder.rs index 28e562468..5b1c40632 100644 --- a/crates/j2k-native/src/j2c/arithmetic_decoder.rs +++ b/crates/j2k-native/src/j2c/arithmetic_decoder.rs @@ -13,13 +13,15 @@ use super::mq::PACKED_DECODER_STATES; pub(crate) struct ArithmeticDecoder<'a> { /// The underlying encoded data. data: &'a [u8], - /// The C-register (see Table C.1). - c: u32, + /// The C-register (see Table C.1) in bits 32-63: its upper half, which + /// DECODE compares with A, in bits 48-63, followed by `shift_count` bits + /// of code bytes read ahead. + c: u64, /// The A-register (see Table C.1). a: u32, /// The pointer to the current byte. base_pointer: u32, - /// The bit shift counter. + /// The number of read-ahead bits below the upper half of C. shift_count: u32, } @@ -52,43 +54,48 @@ impl<'a> ArithmeticDecoder<'a> { /// /// We use the version from Annex G in . pub(crate) fn initialize(&mut self) { - self.c = (u32::from(self.current_byte()) ^ 0xff) << 16; - self.read_byte(); + self.c = u64::from(self.current_byte() ^ 0xff) << 48; + self.read_bytes(); self.c <<= 7; self.shift_count -= 7; self.a = 0x8000; } - /// The BYTEIN procedure from C.3.4. + /// The BYTEIN procedure from C.3.4, repeated while another byte fits + /// below the upper half of C. /// /// We use the version from Annex G from . + /// A byte after 0xFF may carry into the bits above it, so it is only read + /// once no read-ahead bits remain, as in the procedure; any other byte + /// lands on zero bits, so reading it early changes nothing DECODE sees. #[expect( clippy::inline_always, reason = "MQ state transitions are measured per-symbol hot paths" )] #[inline(always)] - fn read_byte(&mut self) { - if self.current_byte() == 0xff { - let b1 = self.next_byte(); + fn read_bytes(&mut self) { + while self.shift_count <= 40 { + if self.current_byte() == 0xff { + let b1 = self.next_byte(); - if b1 > 0x8f { - self.shift_count = 8; + if b1 > 0x8f { + self.shift_count += 8; + } else if self.shift_count == 0 { + self.base_pointer += 1; + self.c = self + .c + .wrapping_add(0x7f << 41) + .wrapping_sub(u64::from(b1) << 41); + self.shift_count = 7; + } else { + break; + } } else { self.base_pointer += 1; - self.c = self - .c - .wrapping_add(0xfe00) - .wrapping_sub(u32::from(self.current_byte()) << 9); - self.shift_count = 7; + self.c += u64::from(self.current_byte() ^ 0xff) << (40 - self.shift_count); + self.shift_count += 8; } - } else { - self.base_pointer += 1; - self.c = self - .c - .wrapping_add(0xff00) - .wrapping_sub(u32::from(self.current_byte()) << 8); - self.shift_count = 8; } } @@ -102,7 +109,7 @@ impl<'a> ArithmeticDecoder<'a> { // Original code: // loop { // if self.shift_count == 0 { - // self.read_byte(); + // BYTEIN // } // // self.a <<= 1; @@ -114,11 +121,9 @@ impl<'a> ArithmeticDecoder<'a> { // } // } - // Optimization: Batch shifts. + // Optimization: Batch shifts over several bytes read ahead. while self.a & 0x8000 == 0 { - if self.shift_count == 0 { - self.read_byte(); - } + self.read_bytes(); let shifts_needed = self.a.leading_zeros() - 16; let batch = shifts_needed.min(self.shift_count); @@ -155,14 +160,14 @@ impl<'a> ArithmeticDecoder<'a> { // exchange_mps: d = mps ^ cond, flip when cond, index = cond*nlps + inv*nmps // exchange_lps: d = mps ^ inv_cond, flip when inv_cond, index = cond*nmps + inv*nlps // so both are handled by XOR-ing cond with is_lps. - let is_lps = u32::from((self.c >> 16) >= a); + let is_lps = u32::from((self.c >> 48) as u32 >= a); // A probability-state transition happens exactly when A renormalizes: // on every LPS, and on an MPS that leaves A below 0x8000. let transition = is_lps | u32::from(a < 0x8000); // LPS: C -= A << 16 and A = Qe; MPS: A stays A - Qe. let lps_mask = is_lps.wrapping_neg(); - self.c -= (a << 16) & lps_mask; + self.c -= (u64::from(a) << 48) & u64::from(is_lps).wrapping_neg(); self.a = (a & !lps_mask) | (qe & lps_mask); // Same condition as in exchange_mps / exchange_lps. Without a From 8f1f438a3e9f64479f554f56300ba5de6311cdaf Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:16:00 -0400 Subject: [PATCH 06/10] perf(j2k-native): visit classic Tier-1 candidate rows as bit sets The packed-column passes tested each of a column's four rows with its own data-dependent branch. Each pass now forms the column's candidate rows as a set of bits, one per row at the row's window shift, and visits them lowest first: significance propagation takes uncoded rows with a significant neighbor, refinement the rows significant before the pass, and cleanup the uncoded rows. When significance propagation makes a row significant, it re-derives the rows below it, which now have a significant neighbor. Decisions, contexts and their order are unchanged, so output is unchanged. Add a changelog entry for both classic Tier-1 changes. --- CHANGELOG.md | 4 + .../src/j2c/bitplane/flag_passes.rs | 88 ++++++++++++------- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a794be18..54bd59c16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ and stale roadmap entries have been removed from the public documentation set. in the zero-bitplane tag tree, as the CPU encoder does, so their packet headers match the CPU's byte for byte. Earlier codestreams were valid but could differ from the CPU's by a few header bytes. +- CPU classic (Part 1) code-block decoding reads several MQ code bytes ahead per + refill instead of one per renormalization, and visits each column's candidate + rows in the significance propagation, refinement and cleanup passes as a bit + set instead of testing every row. Output is unchanged. - `j2k-tilecodec` LZW decode no longer reports valid TIFF strips and tiles as truncated. weezl reads input ahead of decoding, so the call that consumes the last byte can still hold codes up to the end code, often in diff --git a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs index 5ab9af691..2530a37b0 100644 --- a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs +++ b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs @@ -14,8 +14,28 @@ use super::flags::{ }; use super::state::{BitPlaneDecodeContext, Coefficient}; -/// Significance bits of the column's own four rows. -const SIGMA_ROWS: u32 = SIGMA_THIS | SIGMA_THIS << 3 | SIGMA_THIS << 6 | SIGMA_THIS << 9; +/// Bit `3 * row` for each of a column's four rows, the shift that aligns the +/// row's window with bit 0. Passes visit a column's rows as a set of these +/// bits, lowest first, instead of branching on each row. +const ROW_BITS: u32 = 0x249; + +/// Rows neither significant nor coded in this bitplane's significance +/// propagation pass, as `ROW_BITS`. +#[expect(clippy::inline_always, reason = "Tier-1 coefficient-loop hot path")] +#[inline(always)] +fn uncoded_rows(own: u32) -> u32 { + !(own >> SIGMA_THIS.trailing_zeros() | own >> PI_THIS.trailing_zeros()) & ROW_BITS +} + +/// Uncoded rows with a significant neighbor, as `ROW_BITS`. +#[expect(clippy::inline_always, reason = "Tier-1 coefficient-loop hot path")] +#[inline(always)] +fn propagation_rows(own: u32) -> u32 { + // Bit 3k: any significance in window row k (row k - 1 of the stripe). + let any_in_row = own | own >> 1 | own >> 2; + let neighbors = any_in_row | any_in_row >> 6 | own >> 3 | own >> 5; + neighbors & uncoded_rows(own) +} /// Register copies of everything a pass touches. The MQ decoder is a local /// copy and the arrays are plain slices, so neither the MQ registers nor the @@ -53,16 +73,13 @@ impl<'c, 'd> Scan<'c, 'd> { } /// Flag and top-row coefficient indices of column 0 in `stripe`, and the - /// number of rows the stripe holds. - #[expect( - clippy::cast_possible_truncation, - reason = "a stripe holds at most four rows" - )] + /// `ROW_BITS` of the rows the stripe holds. fn stripe_start(&self, stripe: usize) -> (usize, usize, u32) { + let rows = (self.height - 4 * stripe).min(4); ( (stripe + 1) * self.stride + 1, (4 * stripe + 1) * self.stride + 1, - (self.height - 4 * stripe).min(4) as u32, + ROW_BITS & ((1 << (3 * rows)) - 1), ) } @@ -90,13 +107,16 @@ impl<'c, 'd> Scan<'c, 'd> { set_significant(self.flags, own, flag_index, self.stride, row, sign); } - /// Zero-code the insignificant coefficient at `row`. + /// Zero-code the insignificant coefficient at `row`; returns whether it + /// became significant. #[expect(clippy::inline_always, reason = "Tier-1 coefficient-loop hot path")] #[inline(always)] - fn zero_code(&mut self, own: &mut u32, flag_index: usize, top: usize, row: u32) { - if self.read(zero_coding_label(self.zero_coding, *own, row)) != 0 { + fn zero_code(&mut self, own: &mut u32, flag_index: usize, top: usize, row: u32) -> bool { + let significant = self.read(zero_coding_label(self.zero_coding, *own, row)) != 0; + if significant { self.become_significant(own, flag_index, top, row); } + significant } } @@ -110,15 +130,18 @@ pub(super) fn significance_propagation_pass_flags( for x in 0..scan.width { let flag_index = flag_row + x; let mut own = scan.flags[flag_index]; - // Nothing significant in or around the column: no candidates. - if own == 0 { + let mut candidates = propagation_rows(own) & rows; + if candidates == 0 { continue; } - for row in 0..rows { - let window = own >> (3 * row); - if window & (SIGMA_THIS | PI_THIS) == 0 && window & SIGMA_NEIGHBOURS != 0 { - own |= PI_THIS << (3 * row); - scan.zero_code(&mut own, flag_index, top_row + x, row); + while candidates != 0 { + let shift = candidates.trailing_zeros(); + own |= PI_THIS << shift; + if scan.zero_code(&mut own, flag_index, top_row + x, shift / 3) { + // The new significant row neighbors the rows below it. + candidates = propagation_rows(own) & rows & (u32::MAX << (shift + 1)); + } else { + candidates &= candidates - 1; } } scan.flags[flag_index] = own; @@ -137,15 +160,15 @@ pub(super) fn magnitude_refinement_pass_flags( for x in 0..scan.width { let flag_index = flag_row + x; let mut own = scan.flags[flag_index]; - if own & SIGMA_ROWS == 0 { + // Significant before this bitplane's significance pass. + let mut candidates = + (own >> SIGMA_THIS.trailing_zeros()) & !(own >> PI_THIS.trailing_zeros()) & rows; + if candidates == 0 { continue; } - for row in 0..rows { - let window = own >> (3 * row); - // Significant before this bitplane's significance pass. - if window & (SIGMA_THIS | PI_THIS) != SIGMA_THIS { - continue; - } + while candidates != 0 { + let shift = candidates.trailing_zeros(); + let window = own >> shift; let label = if window & MU_THIS != 0 { 16 } else if window & SIGMA_NEIGHBOURS != 0 { @@ -154,9 +177,10 @@ pub(super) fn magnitude_refinement_pass_flags( 14 }; let bit = scan.read(label); - scan.coefficients[top_row + x + row as usize * scan.stride] + scan.coefficients[top_row + x + (shift / 3) as usize * scan.stride] .push_bit_at(bit, scan.bit_position); - own |= MU_THIS << (3 * row); + own |= MU_THIS << shift; + candidates &= candidates - 1; } scan.flags[flag_index] = own; } @@ -182,7 +206,7 @@ pub(super) fn cleanup_pass_flags( let mut own = scan.flags[flag_index]; // A zero word is a full column of insignificant, unvisited // coefficients with zero contexts: Annex D's run-length case. - if rows == 4 && own == 0 { + if rows == ROW_BITS && own == 0 { if scan.decoder.read_bit(&mut run_length) == 0 { continue; } @@ -193,10 +217,10 @@ pub(super) fn cleanup_pass_flags( scan.zero_code(&mut own, flag_index, top, row); } } else { - for row in 0..rows { - if (own >> (3 * row)) & (SIGMA_THIS | PI_THIS) == 0 { - scan.zero_code(&mut own, flag_index, top, row); - } + let mut candidates = uncoded_rows(own) & rows; + while candidates != 0 { + scan.zero_code(&mut own, flag_index, top, candidates.trailing_zeros() / 3); + candidates &= candidates - 1; } } // Visit marks last one bitplane. From 9bb7523bb6d5fe619b439e72ff4a8d5d449b95aa Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:16:00 -0400 Subject: [PATCH 07/10] perf(j2k-native): pass row window shifts to the packed-column helpers The candidate loops find each row as its window shift, the bit index `3 * row` of its neighborhood in the column word, then divided it by three to call the zero-coding, sign-coding and significance helpers, which multiplied it by three again. On AArch64 that put a multiply and three more instructions between the candidate and its context label. The helpers now take the shift; only coefficient addresses still need the row. The classic encoder's passes, which share the helpers and also track each row as its shift, pass it unchanged too. Decoded samples and encoded codestreams are unchanged. --- .../src/j2c/bitplane/flag_passes.rs | 35 ++++++++++--------- crates/j2k-native/src/j2c/bitplane/flags.rs | 33 ++++++++--------- crates/j2k-native/src/j2c/bitplane/tests.rs | 6 ++-- .../src/j2c/bitplane_encode/flag_passes.rs | 6 ++-- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs index 2530a37b0..19f5e2ce6 100644 --- a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs +++ b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs @@ -89,32 +89,33 @@ impl<'c, 'd> Scan<'c, 'd> { self.decoder.read_bit(&mut self.contexts[label as usize]) } - /// The coefficient at `row` just decoded a one while insignificant: record - /// the bit, decode its sign, and mark it significant. + /// The coefficient in the row at window shift `shift` just decoded a one + /// while insignificant: record the bit, decode its sign, and mark it + /// significant. #[expect(clippy::inline_always, reason = "Tier-1 coefficient-loop hot path")] #[inline(always)] - fn become_significant(&mut self, own: &mut u32, flag_index: usize, top: usize, row: u32) { - let coefficient = &mut self.coefficients[top + row as usize * self.stride]; - coefficient.push_bit_at(1, self.bit_position); + fn become_significant(&mut self, own: &mut u32, flag_index: usize, top: usize, shift: u32) { + let index = top + (shift / 3) as usize * self.stride; + self.coefficients[index].push_bit_at(1, self.bit_position); let (label, xor) = sign_coding_label( *own, self.flags[flag_index - 1], self.flags[flag_index + 1], - row, + shift, ); let sign = self.read(label) ^ u32::from(xor); - self.coefficients[top + row as usize * self.stride].set_sign(u8::from(sign != 0)); - set_significant(self.flags, own, flag_index, self.stride, row, sign); + self.coefficients[index].set_sign(u8::from(sign != 0)); + set_significant(self.flags, own, flag_index, self.stride, shift, sign); } - /// Zero-code the insignificant coefficient at `row`; returns whether it - /// became significant. + /// Zero-code the insignificant coefficient in the row at window shift + /// `shift`; returns whether it became significant. #[expect(clippy::inline_always, reason = "Tier-1 coefficient-loop hot path")] #[inline(always)] - fn zero_code(&mut self, own: &mut u32, flag_index: usize, top: usize, row: u32) -> bool { - let significant = self.read(zero_coding_label(self.zero_coding, *own, row)) != 0; + fn zero_code(&mut self, own: &mut u32, flag_index: usize, top: usize, shift: u32) -> bool { + let significant = self.read(zero_coding_label(self.zero_coding, *own, shift)) != 0; if significant { - self.become_significant(own, flag_index, top, row); + self.become_significant(own, flag_index, top, shift); } significant } @@ -137,7 +138,7 @@ pub(super) fn significance_propagation_pass_flags( while candidates != 0 { let shift = candidates.trailing_zeros(); own |= PI_THIS << shift; - if scan.zero_code(&mut own, flag_index, top_row + x, shift / 3) { + if scan.zero_code(&mut own, flag_index, top_row + x, shift) { // The new significant row neighbors the rows below it. candidates = propagation_rows(own) & rows & (u32::MAX << (shift + 1)); } else { @@ -212,14 +213,14 @@ pub(super) fn cleanup_pass_flags( } let first = (scan.decoder.read_bit(&mut uniform) << 1) | scan.decoder.read_bit(&mut uniform); - scan.become_significant(&mut own, flag_index, top, first); + scan.become_significant(&mut own, flag_index, top, 3 * first); for row in first + 1..4 { - scan.zero_code(&mut own, flag_index, top, row); + scan.zero_code(&mut own, flag_index, top, 3 * row); } } else { let mut candidates = uncoded_rows(own) & rows; while candidates != 0 { - scan.zero_code(&mut own, flag_index, top, candidates.trailing_zeros() / 3); + scan.zero_code(&mut own, flag_index, top, candidates.trailing_zeros()); candidates &= candidates - 1; } } diff --git a/crates/j2k-native/src/j2c/bitplane/flags.rs b/crates/j2k-native/src/j2c/bitplane/flags.rs index b935b817b..9512ab222 100644 --- a/crates/j2k-native/src/j2c/bitplane/flags.rs +++ b/crates/j2k-native/src/j2c/bitplane/flags.rs @@ -72,40 +72,42 @@ pub(in crate::j2c) fn zero_coding_table(sub_band_type: SubBandType) -> &'static } } -/// Zero-coding context label for row `row` of the column word `flags`. +/// Zero-coding context label for the row of the column word `flags` whose +/// window starts at bit `shift` (three times the row). #[expect( clippy::inline_always, reason = "Tier-1 context lookup is a measured coefficient-loop hot path" )] #[inline(always)] -pub(in crate::j2c) fn zero_coding_label(table: &[u8; 512], flags: u32, row: u32) -> u8 { - table[((flags >> (3 * row)) & SIGMA_NEIGHBOURS) as usize] +pub(in crate::j2c) fn zero_coding_label(table: &[u8; 512], flags: u32, shift: u32) -> u8 { + table[((flags >> shift) & SIGMA_NEIGHBOURS) as usize] } -/// Sign-coding context label and XOR bit (Table D.3) for row `row`, from the -/// column word and its left and right neighbors. +/// Sign-coding context label and XOR bit (Table D.3) for the row whose window +/// starts at bit `shift`, from the column word and its left and right neighbors. #[expect( clippy::inline_always, reason = "Tier-1 context lookup is a measured coefficient-loop hot path" )] #[inline(always)] -pub(in crate::j2c) fn sign_coding_label(flags: u32, left: u32, right: u32, row: u32) -> (u8, u8) { - let window = flags >> (3 * row); +pub(in crate::j2c) fn sign_coding_label(flags: u32, left: u32, right: u32, shift: u32) -> (u8, u8) { + let window = flags >> shift; let significances = (((window >> 1) & 1) << 6) | (((window >> 3) & 1) << 4) | (((window >> 5) & 1) << 2) | ((window >> 7) & 1); // The sign of row - 1 is bit 18 for row 0 and 19 + 3 * (row - 1) after. - let north_sign_bit = if row == 0 { 18 } else { 16 + 3 * row }; + let north_sign_bit = if shift == 0 { 18 } else { 16 + shift }; let signs = (((flags >> north_sign_bit) & 1) << 6) - | (((left >> (19 + 3 * row)) & 1) << 4) - | (((right >> (19 + 3 * row)) & 1) << 2) - | ((flags >> (22 + 3 * row)) & 1); + | (((left >> (19 + shift)) & 1) << 4) + | (((right >> (19 + shift)) & 1) << 2) + | ((flags >> (22 + shift)) & 1); let merged = ((significances & signs) << 1) | (significances & !signs); SIGN_CONTEXT_LOOKUP[merged as usize] } -/// Record that row `row` of the column word at `index` became significant +/// Record that the row at window shift `shift` (three times the row) of the +/// column word at `index` became significant /// with sign `sign`. The column's own word is the caller's register copy /// `own`; neighbor words are updated in `flags`, whose row stride is /// `stride` (one padding column and one padding stripe on every side). @@ -116,20 +118,19 @@ pub(in crate::j2c) fn set_significant( own: &mut u32, index: usize, stride: usize, - row: u32, + shift: u32, sign: u32, ) { - let shift = 3 * row; *own |= (SIGMA_THIS | sign << 19) << shift; flags[index - 1] |= (1 << 5) << shift; flags[index + 1] |= (1 << 3) << shift; - if row == 0 { + if shift == 0 { // Row 4 of the stripe above: significance bits 15-17, sign bit 31. let north = index - stride; flags[north - 1] |= 1 << 17; flags[north] |= 1 << 16 | sign << 31; flags[north + 1] |= 1 << 15; - } else if row == 3 { + } else if shift == 9 { // Row -1 of the stripe below: significance bits 0-2, sign bit 18. let south = index + stride; flags[south - 1] |= 1 << 2; diff --git a/crates/j2k-native/src/j2c/bitplane/tests.rs b/crates/j2k-native/src/j2c/bitplane/tests.rs index c13b66d19..892b5d4be 100644 --- a/crates/j2k-native/src/j2c/bitplane/tests.rs +++ b/crates/j2k-native/src/j2c/bitplane/tests.rs @@ -569,7 +569,7 @@ fn packed_column_contexts_match_per_coefficient_contexts() { let index = flag_index(x, y); let mut own = flags[index]; let row = u32::try_from(y % 4).expect("row below four"); - set_significant(&mut flags, &mut own, index, stride, row, sign); + set_significant(&mut flags, &mut own, index, stride, 3 * row, sign); flags[index] = own; } } @@ -589,13 +589,13 @@ fn packed_column_contexts_match_per_coefficient_contexts() { SubBandType::HighHigh, ] { assert_eq!( - zero_coding_label(zero_coding_table(sub_band_type), word, row), + zero_coding_label(zero_coding_table(sub_band_type), word, 3 * row), context_label_zero_coding_from_neighbors(neighbors, sub_band_type), "{width}x{height} ({x}, {y})" ); } assert_eq!( - sign_coding_label(word, flags[index - 1], flags[index + 1], row), + sign_coding_label(word, flags[index - 1], flags[index + 1], 3 * row), context_label_sign_coding_index(idx, y, &ctx), "{width}x{height} ({x}, {y})" ); diff --git a/crates/j2k-native/src/j2c/bitplane_encode/flag_passes.rs b/crates/j2k-native/src/j2c/bitplane_encode/flag_passes.rs index 214454f1c..c4ae12c31 100644 --- a/crates/j2k-native/src/j2c/bitplane_encode/flag_passes.rs +++ b/crates/j2k-native/src/j2c/bitplane_encode/flag_passes.rs @@ -122,10 +122,10 @@ impl Scan<'_> { *own, self.flags[flag_index - 1], self.flags[flag_index + 1], - shift / 3, + shift, ); self.code(sign ^ u32::from(xor), label); - set_significant(self.flags, own, flag_index, self.stride, shift / 3, sign); + set_significant(self.flags, own, flag_index, self.stride, shift, sign); } /// Zero-code the insignificant coefficient in the row at window shift @@ -135,7 +135,7 @@ impl Scan<'_> { fn zero_code(&mut self, own: &mut u32, flag_index: usize, column: usize, shift: u32) -> bool { let sample = self.sample(column, shift); let bit = (sample >> self.bitplane) & 1; - self.code(bit, zero_coding_label(self.zero_coding, *own, shift / 3)); + self.code(bit, zero_coding_label(self.zero_coding, *own, shift)); if bit != 0 { self.become_significant(own, flag_index, sample, shift); } From a223a2f8875a798254970a2d7678414a4ec90acb Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:16:01 -0400 Subject: [PATCH 08/10] perf(j2k-native): pick the next MQ state with one shift The packed decoder state holds the next MPS state in byte 3 and the next LPS state in byte 4. DECODE extracted both and chose between them with two XOR-mask selects; it now shifts by 24 or 32 and keeps the old state unless the probability state transitions. Output is unchanged. --- crates/j2k-native/src/j2c/arithmetic_decoder.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/j2k-native/src/j2c/arithmetic_decoder.rs b/crates/j2k-native/src/j2c/arithmetic_decoder.rs index 5b1c40632..f698f937a 100644 --- a/crates/j2k-native/src/j2c/arithmetic_decoder.rs +++ b/crates/j2k-native/src/j2c/arithmetic_decoder.rs @@ -139,8 +139,7 @@ impl<'a> ArithmeticDecoder<'a> { #[expect( clippy::cast_possible_truncation, clippy::inline_always, - clippy::similar_names, - reason = "packed MQ fields are explicitly bounded and MPS/LPS are paired domain terms on a measured per-symbol hot path" + reason = "packed MQ fields are explicitly bounded on a measured per-symbol hot path" )] #[inline(always)] pub(crate) fn decode(&mut self, context: &mut ArithmeticDecoderContext) -> u32 { @@ -176,12 +175,9 @@ impl<'a> ArithmeticDecoder<'a> { let pick_nlps = cond ^ is_lps; let d = mps ^ pick_nlps; - let pick_mask = u8::from(pick_nlps != 0).wrapping_neg(); - let next_mps = (state >> 24) as u8; - let next_lps = (state >> 32) as u8; - let next = next_mps ^ ((next_mps ^ next_lps) & pick_mask); - let keep_mask = u8::from(transition == 0).wrapping_neg(); - context.0 = next ^ ((next ^ context.0) & keep_mask); + // The next MPS state is byte 3 of `state` and the next LPS state byte 4. + let next = (state >> (24 + 8 * pick_nlps)) as u8; + context.0 = if transition == 0 { context.0 } else { next }; // Shift A back to at least 0x8000 in one step when the buffered bits // suffice (zero shift in the fast case); otherwise take the byte-in From 08aa76bc6b18b49180d6215cbc656dc32fc7028b Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:16:01 -0400 Subject: [PATCH 09/10] perf(j2k-native): index classic MQ contexts by label without a bounds check Context labels come from the zero-coding and sign-coding tables as `u8`, so indexing the 19 contexts checked every label against 18. The context array now has 256 slots, which any `u8` label indexes in bounds; labels 0-18 are still the only ones used. Output is unchanged. Add a changelog entry for the classic Tier-1 changes. --- CHANGELOG.md | 4 ++++ crates/j2k-native/src/j2c/bitplane/flag_passes.rs | 5 +++-- crates/j2k-native/src/j2c/bitplane/state.rs | 7 ++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54bd59c16..bbbec8445 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ and stale roadmap entries have been removed from the public documentation set. in the zero-bitplane tag tree, as the CPU encoder does, so their packet headers match the CPU's byte for byte. Earlier codestreams were valid but could differ from the CPU's by a few header bytes. +- CPU classic (Part 1) code-block decoding passes each row's window shift + instead of its row to the context helpers, picks the next MQ probability + state with one shift, and indexes its contexts by label without a bounds + check. Output is unchanged. - CPU classic (Part 1) code-block decoding reads several MQ code bytes ahead per refill instead of one per renormalization, and visits each column's candidate rows in the significance propagation, refinement and cleanup passes as a bit diff --git a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs index 19f5e2ce6..9710f4760 100644 --- a/crates/j2k-native/src/j2c/bitplane/flag_passes.rs +++ b/crates/j2k-native/src/j2c/bitplane/flag_passes.rs @@ -42,7 +42,7 @@ fn propagation_rows(own: u32) -> u32 { /// slice pointers round-trip through memory that the pass's stores alias. struct Scan<'c, 'd> { decoder: ArithmeticDecoder<'d>, - contexts: &'c mut [ArithmeticDecoderContext; 19], + contexts: &'c mut [ArithmeticDecoderContext; 256], flags: &'c mut [u32], coefficients: &'c mut [Coefficient], zero_coding: &'static [u8; 512], @@ -86,7 +86,8 @@ impl<'c, 'd> Scan<'c, 'd> { #[expect(clippy::inline_always, reason = "per-symbol MQ decode")] #[inline(always)] fn read(&mut self, label: u8) -> u32 { - self.decoder.read_bit(&mut self.contexts[label as usize]) + self.decoder + .read_bit(&mut self.contexts[usize::from(label)]) } /// The coefficient in the row at window shift `shift` just decoded a one diff --git a/crates/j2k-native/src/j2c/bitplane/state.rs b/crates/j2k-native/src/j2c/bitplane/state.rs index 490b39e41..e0bf791c3 100644 --- a/crates/j2k-native/src/j2c/bitplane/state.rs +++ b/crates/j2k-native/src/j2c/bitplane/state.rs @@ -127,8 +127,9 @@ pub(crate) struct BitPlaneDecodeContext { pub(super) max_coding_passes: u8, /// The type of sub-band the current code block belongs to. pub(super) sub_band_type: SubBandType, - /// The arithmetic decoder contexts for each context label. - pub(super) contexts: [ArithmeticDecoderContext; 19], + /// The arithmetic decoder contexts for each context label (0-18). Any + /// `u8` label indexes the array, so lookups need no bounds check. + pub(super) contexts: [ArithmeticDecoderContext; 256], /// The bit position for the current bitplane. pub(super) current_bit_position: u8, } @@ -165,7 +166,7 @@ impl BitPlaneDecodeContext { max_coding_passes: 0, strict: false, sub_band_type: SubBandType::LowLow, - contexts: [ArithmeticDecoderContext::empty(); 19], + contexts: [ArithmeticDecoderContext::empty(); 256], current_bit_position: 0, } } From 8c8a9ca9158e191022d6d31c8b45c47bbc4c1d1f Mon Sep 17 00:00:00 2001 From: GF Date: Thu, 29 Oct 2026 00:19:42 -0400 Subject: [PATCH 10/10] docs: measure the classic Tier-1 decode changes Interleaved A/B rounds against the parent of the classic Tier-1 commits, with a second build of the parent as the A/A control, on an M4 Pro. On a 4096 x 2048 RGB tissue region, one-thread decodes are 11.2% faster lossless and 8.6% faster at 2 bpp, 12-thread lossless decodes 11.3% faster, and eight concurrent serial 1024 x 1024 region decodes 9.2% faster, each beyond the A/A spread; classic lossless encode is not slower. Every cell's output is byte-identical. The 256-slot contexts add 0.7 to 1.8 points of the one-thread gain and 237 bytes per decoding context. Fold the two classic Tier-1 changelog entries into one with these numbers. --- CHANGELOG.md | 19 ++++++++++-------- docs/benchmark-evidence.md | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbbec8445..e01053fd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,14 +32,17 @@ and stale roadmap entries have been removed from the public documentation set. in the zero-bitplane tag tree, as the CPU encoder does, so their packet headers match the CPU's byte for byte. Earlier codestreams were valid but could differ from the CPU's by a few header bytes. -- CPU classic (Part 1) code-block decoding passes each row's window shift - instead of its row to the context helpers, picks the next MQ probability - state with one shift, and indexes its contexts by label without a bounds - check. Output is unchanged. -- CPU classic (Part 1) code-block decoding reads several MQ code bytes ahead per - refill instead of one per renormalization, and visits each column's candidate - rows in the significance propagation, refinement and cleanup passes as a bit - set instead of testing every row. Output is unchanged. +- CPU classic (Part 1) code-block decoding reads several MQ code bytes ahead + per refill instead of one per renormalization, visits each column's + candidate rows in the significance propagation, refinement and cleanup + passes as a bit set instead of testing every row, passes each row's window + shift instead of its row to the context helpers (the classic encoder shares + them), picks the next MQ probability state with one shift, and indexes its + contexts by label without a bounds check. On a 4096 x 2048 RGB image, + one-thread decodes are 11% faster lossless and 9% faster at 2 bpp, and + 12-thread lossless decodes 11% faster; eight concurrent 1024 x 1024 region + decodes of an 18000 x 12000 image are 9% faster. Decoded samples and + encoded codestreams are unchanged. See `docs/benchmark-evidence.md`. - `j2k-tilecodec` LZW decode no longer reports valid TIFF strips and tiles as truncated. weezl reads input ahead of decoding, so the call that consumes the last byte can still hold codes up to the end code, often in diff --git a/docs/benchmark-evidence.md b/docs/benchmark-evidence.md index a1a53993f..5c1e45407 100644 --- a/docs/benchmark-evidence.md +++ b/docs/benchmark-evidence.md @@ -19,6 +19,46 @@ cargo run -p xtask --features adoption -- adoption-report --run-dir target/j2k-a Speed comparisons must use the manifest-backed external rows, not the generated in-repo fixtures. +## CPU classic Tier-1 decoding - 2026-10-29 + +Apple M4 Pro (12 CPU cores: 8 performance, 4 efficiency; 48 GB), macOS +27.0.1, Rust 1.99.0 release builds. "Before" is the parent of the classic +Tier-1 commits; "after" adds the MQ read-ahead, bit-set candidate rows, window +shifts, one-shift state selection and 256-slot contexts. A second build of +"before" from its own source copy is the A/A control. One harness was built +against each tree; every round runs each build once in a shuffled order, and +each cell reports the per-round median change against "before" in the same +round, with its range. + +Inputs: a 4096 x 2048 RGB8 region of the 18000 x 12000 classic `large.jp2` +(real tissue), encoded once by "before" as classic lossless (5/3, 10.6 MB) +and at 2 bpp (9/7, 2.09 MB); the 18000 x 12000 classic 9/7 `large-plt.jp2` +and `large.jp2` themselves. Full decodes are `J2kDecoder::decode_into` to +RGB8 inside a Rayon pool of 1 or 12 threads. "Region" is one serial 1024 x +1024 `decode_region_into` from the middle of `large-plt.jp2`; "8 concurrent" +is eight threads each decoding a different 1024 x 1024 region of `large.jp2` +serially, timed together. Every build produced byte-identical output in every +cell, encodes included. + +| Cell | Before (ms) | A/A | After | +| --- | ---: | ---: | ---: | +| Lossless decode, 1 thread | 1026 | -0.4% [-1.4, +2.3] | -11.2% | +| 2 bpp decode, 1 thread | 260 | +0.8% [-1.8, +2.2] | -8.6% | +| Region, 1 thread | 117 | -1.8% [-2.3, +3.5] | -12.0% | +| Lossless decode, 12 threads | 122 | +0.3% [-7.1, +8.5] | -11.3% [-25.7, -5.9] | +| 2 bpp decode, 12 threads | 37.1 | -2.8% [-17.5, +16.3] | -4.4% [-31.3, +4.8] | +| 8 concurrent regions | 185 | -0.6% [-6.1, +1.1] | -9.2% [-20.3, -2.0] | +| Lossless encode, 12 threads | 353 | +0.3% [-4.0, +4.1] | -3.0% [-14.4, +10.6] | + +The single-thread rows are seven rounds at load average 9 to 33, so their +"after" ranges are left out: one round of each was an outlier (+13% to +61%), +and the other six were -6.1% to -14.1%. The multi-thread, concurrent and +encode rows are a second run of nine rounds at load average 8 to 13. Without +the 256-slot contexts, single-thread decodes are 0.7 to 1.8 points less fast. `ArithmeticDecoderContext` is one byte, so those contexts add 237 +bytes per decoding context; peak heap of the serial region decode was +unchanged, and multi-thread peaks differed by no more than the A/A builds' +own few kilobytes. + ## CPU owned batches of large-image regions - 2026-10-29 Same machine and inputs as the next section, load average 4 to 7. Each sample