From c2638fd22a0199c32c237cc4e1cffcdc0aefdfcc Mon Sep 17 00:00:00 2001 From: Carl Allen <36766173+CarlAllenn@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:32:04 +0100 Subject: [PATCH 1/2] perf(j2k-native): skip tiles outside the output region before workspace build Region decode iterated every tile in the codestream, building the full decomposition/precinct/code-block workspace (and zeroing its coefficient buffers) per tile before the ROI plan could skip any entropy work. The per-request cost therefore scaled with the image's total tile count even for a single-tile ROI: on a 15000x11000, 1024px-tiled codestream a 64px region decode spent ~60 ms in build/parse for the 164 tiles that contribute nothing. Skip a tile before decode_tile when its rect cannot intersect the output region. The intersection test maps the region onto the reference grid exactly as RoiPlan::build does and rounds the tile rect outward, so any rounding disagreement decodes a boundary tile rather than skipping it. Skipped tiles contribute no samples to the stored output, so decoded pixels are unchanged. --- crates/j2k-native/src/j2c/decode.rs | 60 +++++++++++++++++ crates/j2k/tests/decode.rs | 100 ++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/crates/j2k-native/src/j2c/decode.rs b/crates/j2k-native/src/j2c/decode.rs index 6d874098..87fc661b 100644 --- a/crates/j2k-native/src/j2c/decode.rs +++ b/crates/j2k-native/src/j2c/decode.rs @@ -13,6 +13,7 @@ use super::codestream::{ComponentInfo, Header, QuantizationStyle, WaveletTransfo use super::ht_block_decode::{self, HtBlockDecodeContext}; use super::idwt::IDWTOutput; use super::progression::{progression_iterator, ProgressionData}; +use super::rect::IntRect; use super::roi::RoiPlan; use super::tag_tree::TagNode; use super::tile::{ComponentTile, ResolutionTile, Tile}; @@ -171,6 +172,13 @@ fn decode<'a>( tile.rect.height(), ); + if let Some(output_region) = tile_ctx.output_region { + if !tile_intersects_output_region(tile, header, output_region) { + ltrace!("tile {} outside output region, skipped", tile.idx); + continue; + } + } + decode_tile( tile, header, @@ -207,6 +215,58 @@ fn decode<'a>( Ok(()) } +/// Whether `tile` can contribute any sample to `output_region`. +/// +/// The region is mapped onto the reference grid exactly as +/// [`RoiPlan::build`] does (image-area offset in shrunk-grid units); the +/// tile rect is mapped into the same shrunk grid with outward rounding, so +/// any rounding disagreement keeps a boundary tile decoded rather than +/// skipping it. Tiles that fail this test have an empty intersection with +/// the region at every resolution level and contribute nothing to the +/// stored output, so the caller can skip their decomposition build, +/// segment parse, and code-block work entirely. +fn tile_intersects_output_region( + tile: &Tile<'_>, + header: &Header<'_>, + output_region: OutputRegion, +) -> bool { + let size_data = &header.size_data; + // The output region lives in final image coordinates: the reference + // grid shrunk by both the component subsampling factor and the + // reduced-resolution factor. Saturating keeps crafted headers safe; + // the max(1) guard keeps the division defined (a zero factor is + // rejected by header validation before decode). + let x_shrink = size_data + .x_shrink_factor + .saturating_mul(size_data.x_resolution_shrink_factor) + .max(1); + let y_shrink = size_data + .y_shrink_factor + .saturating_mul(size_data.y_resolution_shrink_factor) + .max(1); + let x_offset = size_data.image_area_x_offset.div_ceil(x_shrink); + let y_offset = size_data.image_area_y_offset.div_ceil(y_shrink); + let region = IntRect::from_ltrb( + output_region.x.saturating_add(x_offset), + output_region.y.saturating_add(y_offset), + output_region + .x + .saturating_add(output_region.width) + .saturating_add(x_offset), + output_region + .y + .saturating_add(output_region.height) + .saturating_add(y_offset), + ); + let tile_rect = IntRect::from_ltrb( + tile.rect.x0 / x_shrink, + tile.rect.y0 / y_shrink, + tile.rect.x1.div_ceil(x_shrink), + tile.rect.y1.div_ceil(y_shrink), + ); + tile_rect.intersects(region) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct OutputRegion { pub(crate) x: u32, diff --git a/crates/j2k/tests/decode.rs b/crates/j2k/tests/decode.rs index cbd6e282..ec94b797 100644 --- a/crates/j2k/tests/decode.rs +++ b/crates/j2k/tests/decode.rs @@ -1599,3 +1599,103 @@ fn zero_dwt53(width: u32, height: u32) -> J2kForwardDwt53Output { }], } } + +#[test] +fn scaled_region_decode_on_multi_tile_codestream_matches_scaled_whole_decode_crop() { + // Reduced-resolution region decode on a multi-tile codestream: the + // output region lives in scaled image coordinates while tile rects + // stay on the reference grid. A tile-relevance test that ignores the + // resolution shrink factor skips tiles that still contribute samples, + // zeroing part of the output. Cover a grid with partial edge tiles, + // interior, straddling, and far-corner regions, at every downscale. + let (width, height) = (320_u32, 288_u32); + let pixels: Vec = (0..height) + .flat_map(|y| { + (0..width).flat_map(move |x| { + [ + (x % 251) as u8, + (y % 241) as u8, + ((x / 8 + y / 8) % 233) as u8, + ] + }) + }) + .collect(); + let options = EncodeOptions { + tile_size: Some((128, 128)), + ..EncodeOptions::default() + }; + let bytes = encode(&pixels, width, height, 3, 8, false, &options).expect("encode"); + let fmt = PixelFormat::Rgb8; + let bpp = fmt.bytes_per_pixel(); + + for scale in [ + Downscale::None, + Downscale::Half, + Downscale::Quarter, + Downscale::Eighth, + ] { + let denom = scale.denominator(); + let (scaled_w, scaled_h) = (width.div_ceil(denom), height.div_ceil(denom)); + let mut whole_decoder = J2kDecoder::new(&bytes).expect("whole decoder"); + let whole_stride = scaled_w as usize * bpp; + let mut whole = vec![0_u8; whole_stride * scaled_h as usize]; + whole_decoder + .decode_scaled_into( + &mut j2k::J2kScratchPool::new(), + &mut whole, + whole_stride, + fmt, + scale, + ) + .expect("scaled whole decode"); + + for roi in [ + // Interior of the top-left tile. + Rect { + x: 24, + y: 24, + w: 64, + h: 64, + }, + // Straddles the first vertical and horizontal tile boundaries. + Rect { + x: 96, + y: 96, + w: 80, + h: 80, + }, + // Reaches into the partial bottom-right edge tiles. + Rect { + x: 240, + y: 232, + w: 72, + h: 48, + }, + ] { + let scaled_roi = roi.scaled_covering(scale); + let mut region_decoder = J2kDecoder::new(&bytes).expect("region decoder"); + let region_stride = scaled_roi.w as usize * bpp; + let mut region = vec![0_u8; region_stride * scaled_roi.h as usize]; + let outcome = region_decoder + .decode_region_scaled_into( + &mut j2k::J2kScratchPool::new(), + &mut region, + region_stride, + fmt, + roi, + scale, + ) + .expect("scaled region decode"); + assert_eq!(outcome.decoded, scaled_roi); + assert_eq!( + region, + crop_bytes(&whole, scaled_w as usize, bpp, scaled_roi), + "region {},{} {}x{} at 1/{denom} disagrees with scaled whole decode", + roi.x, + roi.y, + roi.w, + roi.h, + ); + } + } +} From 94900169e9c258d80ba33445910bb2dd418910ed Mon Sep 17 00:00:00 2001 From: GF Date: Fri, 31 Jul 2026 15:10:43 -0400 Subject: [PATCH 2/2] test: harden region tile skipping geometry --- crates/j2k-native/src/j2c/decode.rs | 57 +------- .../j2k-native/src/j2c/decode/direct_plan.rs | 27 +--- crates/j2k-native/src/j2c/roi.rs | 123 +++++++++++++++++- crates/j2k/tests/decode.rs | 80 ++++++++++-- 4 files changed, 200 insertions(+), 87 deletions(-) diff --git a/crates/j2k-native/src/j2c/decode.rs b/crates/j2k-native/src/j2c/decode.rs index 87fc661b..0c9e10d9 100644 --- a/crates/j2k-native/src/j2c/decode.rs +++ b/crates/j2k-native/src/j2c/decode.rs @@ -13,8 +13,7 @@ use super::codestream::{ComponentInfo, Header, QuantizationStyle, WaveletTransfo use super::ht_block_decode::{self, HtBlockDecodeContext}; use super::idwt::IDWTOutput; use super::progression::{progression_iterator, ProgressionData}; -use super::rect::IntRect; -use super::roi::RoiPlan; +use super::roi::{tile_intersects_output_region, RoiPlan}; use super::tag_tree::TagNode; use super::tile::{ComponentTile, ResolutionTile, Tile}; use super::{bitplane, build, idwt, mct, segment, tile, ComponentData}; @@ -173,7 +172,7 @@ fn decode<'a>( ); if let Some(output_region) = tile_ctx.output_region { - if !tile_intersects_output_region(tile, header, output_region) { + if !tile_intersects_output_region(tile.rect, &header.size_data, output_region) { ltrace!("tile {} outside output region, skipped", tile.idx); continue; } @@ -215,58 +214,6 @@ fn decode<'a>( Ok(()) } -/// Whether `tile` can contribute any sample to `output_region`. -/// -/// The region is mapped onto the reference grid exactly as -/// [`RoiPlan::build`] does (image-area offset in shrunk-grid units); the -/// tile rect is mapped into the same shrunk grid with outward rounding, so -/// any rounding disagreement keeps a boundary tile decoded rather than -/// skipping it. Tiles that fail this test have an empty intersection with -/// the region at every resolution level and contribute nothing to the -/// stored output, so the caller can skip their decomposition build, -/// segment parse, and code-block work entirely. -fn tile_intersects_output_region( - tile: &Tile<'_>, - header: &Header<'_>, - output_region: OutputRegion, -) -> bool { - let size_data = &header.size_data; - // The output region lives in final image coordinates: the reference - // grid shrunk by both the component subsampling factor and the - // reduced-resolution factor. Saturating keeps crafted headers safe; - // the max(1) guard keeps the division defined (a zero factor is - // rejected by header validation before decode). - let x_shrink = size_data - .x_shrink_factor - .saturating_mul(size_data.x_resolution_shrink_factor) - .max(1); - let y_shrink = size_data - .y_shrink_factor - .saturating_mul(size_data.y_resolution_shrink_factor) - .max(1); - let x_offset = size_data.image_area_x_offset.div_ceil(x_shrink); - let y_offset = size_data.image_area_y_offset.div_ceil(y_shrink); - let region = IntRect::from_ltrb( - output_region.x.saturating_add(x_offset), - output_region.y.saturating_add(y_offset), - output_region - .x - .saturating_add(output_region.width) - .saturating_add(x_offset), - output_region - .y - .saturating_add(output_region.height) - .saturating_add(y_offset), - ); - let tile_rect = IntRect::from_ltrb( - tile.rect.x0 / x_shrink, - tile.rect.y0 / y_shrink, - tile.rect.x1.div_ceil(x_shrink), - tile.rect.y1.div_ceil(y_shrink), - ); - tile_rect.intersects(region) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct OutputRegion { pub(crate) x: u32, diff --git a/crates/j2k-native/src/j2c/decode/direct_plan.rs b/crates/j2k-native/src/j2c/decode/direct_plan.rs index 4cb7ee7f..5f4d0163 100644 --- a/crates/j2k-native/src/j2c/decode/direct_plan.rs +++ b/crates/j2k-native/src/j2c/decode/direct_plan.rs @@ -12,6 +12,7 @@ use super::{ Result, RoiPlan, SubBand, SubBandDecodeParameters, Tile, ValidationError, Vec, }; use crate::j2c::rect::IntRect; +use crate::j2c::roi::tile_intersects_output_region; use crate::{ HtCodeBlockPayloadRanges, J2kClassicCodeBlockPayload, J2kCodestreamRange, J2kDirectRgbaPlan, J2kReferencedClassicPlan, J2kReferencedHtj2kPlan, J2kReferencedPayloadRecordSpan, @@ -269,27 +270,11 @@ fn tile_intersects_output( .first() .ok_or(DecodingError::CodeBlockDecodeFailure)?; validate_unit_sampled_component(component_info)?; - let component_tile = ComponentTile::new(tile, component_info); - let resolution_tile = ResolutionTile::new( - component_tile, - component_info.num_resolution_levels() - 1 - header.skipped_resolution_levels, - ); - let x_offset = header - .size_data - .image_area_x_offset - .div_ceil(header.size_data.x_shrink_factor); - let y_offset = header - .size_data - .image_area_y_offset - .div_ceil(header.size_data.y_shrink_factor); - let request_left = output_region.x.saturating_add(x_offset); - let request_top = output_region.y.saturating_add(y_offset); - let request_right = request_left.saturating_add(output_region.width); - let request_bottom = request_top.saturating_add(output_region.height); - Ok(resolution_tile.rect.x0 < request_right - && request_left < resolution_tile.rect.x1 - && resolution_tile.rect.y0 < request_bottom - && request_top < resolution_tile.rect.y1) + Ok(tile_intersects_output_region( + tile.rect, + &header.size_data, + output_region, + )) } fn payload_record_span( diff --git a/crates/j2k-native/src/j2c/roi.rs b/crates/j2k-native/src/j2c/roi.rs index 4377b99c..64954881 100644 --- a/crates/j2k-native/src/j2c/roi.rs +++ b/crates/j2k-native/src/j2c/roi.rs @@ -1,7 +1,7 @@ use alloc::vec::Vec; use super::build::Decomposition; -use super::codestream::{Header, WaveletTransform}; +use super::codestream::{Header, SizeData, WaveletTransform}; use super::decode::{DecompositionStorage, OutputRegion}; use super::rect::IntRect; use super::tile::{ComponentTile, ResolutionTile, Tile}; @@ -9,6 +9,47 @@ use crate::{ idwt_required_input_window_for_rects, try_resize_decode_elements, J2kRequiredBandRegion, Result, }; +/// Whether a reference-grid tile can contribute to a decoded output region. +/// +/// The output region is expressed in final image coordinates. Tile bounds are +/// projected into the same component- and resolution-shrunk grid with outward +/// rounding, which may retain a boundary tile but never drops a contributor. +pub(crate) fn tile_intersects_output_region( + tile_rect: IntRect, + size_data: &SizeData, + output_region: OutputRegion, +) -> bool { + let x_shrink = size_data + .x_shrink_factor + .saturating_mul(size_data.x_resolution_shrink_factor) + .max(1); + let y_shrink = size_data + .y_shrink_factor + .saturating_mul(size_data.y_resolution_shrink_factor) + .max(1); + let x_offset = size_data.image_area_x_offset.div_ceil(x_shrink); + let y_offset = size_data.image_area_y_offset.div_ceil(y_shrink); + let region = IntRect::from_ltrb( + output_region.x.saturating_add(x_offset), + output_region.y.saturating_add(y_offset), + output_region + .x + .saturating_add(output_region.width) + .saturating_add(x_offset), + output_region + .y + .saturating_add(output_region.height) + .saturating_add(y_offset), + ); + let tile_rect = IntRect::from_ltrb( + tile_rect.x0 / x_shrink, + tile_rect.y0 / y_shrink, + tile_rect.x1.div_ceil(x_shrink), + tile_rect.y1.div_ceil(y_shrink), + ); + tile_rect.intersects(region) +} + #[derive(Debug)] #[expect( clippy::struct_field_names, @@ -245,3 +286,83 @@ fn required_region_from_int_rect(rect: IntRect) -> J2kRequiredBandRegion { fn int_rect_from_required_region(region: J2kRequiredBandRegion) -> IntRect { IntRect::from_ltrb(region.x0, region.y0, region.x1, region.y1) } + +#[cfg(test)] +mod tests { + use super::{tile_intersects_output_region, IntRect, OutputRegion, SizeData}; + use crate::j2c::codestream::ComponentSizeInfo; + + fn size_data( + image_offset: (u32, u32), + component_shrink: (u32, u32), + resolution_shrink: (u32, u32), + ) -> SizeData { + SizeData { + reference_grid_width: 515, + reference_grid_height: 389, + image_area_x_offset: image_offset.0, + image_area_y_offset: image_offset.1, + tile_width: 128, + tile_height: 128, + tile_x_offset: 1, + tile_y_offset: 1, + component_sizes: vec![ComponentSizeInfo { + precision: 8, + signed: false, + horizontal_resolution: 1, + vertical_resolution: 1, + }], + x_shrink_factor: component_shrink.0, + y_shrink_factor: component_shrink.1, + x_resolution_shrink_factor: resolution_shrink.0, + y_resolution_shrink_factor: resolution_shrink.1, + } + } + + #[test] + fn tile_intersection_handles_nonzero_image_and_tile_origins() { + let size_data = size_data((3, 5), (1, 1), (4, 2)); + let first_tile = IntRect::from_ltrb(3, 5, 129, 129); + let next_tile = IntRect::from_ltrb(129, 5, 257, 129); + let top_left = OutputRegion { + x: 0, + y: 0, + width: 8, + height: 8, + }; + + assert!(tile_intersects_output_region( + first_tile, &size_data, top_left + )); + assert!(!tile_intersects_output_region( + next_tile, &size_data, top_left + )); + } + + #[test] + fn tile_intersection_combines_component_and_resolution_shrink() { + let size_data = size_data((3, 5), (2, 2), (2, 4)); + let tile = IntRect::from_ltrb(129, 129, 257, 257); + + assert!(tile_intersects_output_region( + tile, + &size_data, + OutputRegion { + x: 31, + y: 15, + width: 2, + height: 2, + } + )); + assert!(!tile_intersects_output_region( + tile, + &size_data, + OutputRegion { + x: 0, + y: 0, + width: 8, + height: 8, + } + )); + } +} diff --git a/crates/j2k/tests/decode.rs b/crates/j2k/tests/decode.rs index 94ccc9e2..1955e8a6 100644 --- a/crates/j2k/tests/decode.rs +++ b/crates/j2k/tests/decode.rs @@ -1785,10 +1785,8 @@ fn scaled_region_decode_on_multi_tile_codestream_matches_scaled_whole_decode_cro Downscale::Eighth, ] { let denominator = scale.denominator(); - let (scaled_width, scaled_height) = ( - width.div_ceil(denominator), - height.div_ceil(denominator), - ); + let (scaled_width, scaled_height) = + (width.div_ceil(denominator), height.div_ceil(denominator)); let mut whole_decoder = J2kDecoder::new(&bytes).expect("whole decoder"); let whole_stride = scaled_width as usize * bytes_per_pixel; let mut whole = vec![0_u8; whole_stride * scaled_height as usize]; @@ -1839,12 +1837,7 @@ fn scaled_region_decode_on_multi_tile_codestream_matches_scaled_whole_decode_cro assert_eq!(outcome.decoded, scaled_roi); assert_eq!( region, - crop_bytes( - &whole, - scaled_width as usize, - bytes_per_pixel, - scaled_roi, - ), + crop_bytes(&whole, scaled_width as usize, bytes_per_pixel, scaled_roi), "region {},{} {}x{} at 1/{denominator} disagrees with scaled whole decode", roi.x, roi.y, @@ -1854,3 +1847,70 @@ fn scaled_region_decode_on_multi_tile_codestream_matches_scaled_whole_decode_cro } } } + +#[test] +fn region_decode_on_subsampled_codestream_matches_whole_decode_crop() { + let (width, height) = (96_u32, 80_u32); + let luma = (0..height) + .flat_map(|y| (0..width).map(move |x| masked_fixture_byte(x + y * 3))) + .collect::>(); + let chroma_width = width / 2; + let chroma_height = height / 2; + let chroma_blue = (0..chroma_height) + .flat_map(|y| (0..chroma_width).map(move |x| masked_fixture_byte(64 + x * 2 + y * 5))) + .collect::>(); + let chroma_red = (0..chroma_height) + .flat_map(|y| (0..chroma_width).map(move |x| masked_fixture_byte(192 + x * 3 + y * 2))) + .collect::>(); + let planes = [ + J2kLosslessComponentPlane { + data: &luma, + x_rsiz: 1, + y_rsiz: 1, + }, + J2kLosslessComponentPlane { + data: &chroma_blue, + x_rsiz: 2, + y_rsiz: 2, + }, + J2kLosslessComponentPlane { + data: &chroma_red, + x_rsiz: 2, + y_rsiz: 2, + }, + ]; + let samples = J2kLosslessComponentSamples::new(&planes, width, height, 8, false) + .expect("subsampled component samples"); + let encoded = encode_j2k_lossless_components( + samples, + &J2kLosslessEncodeOptions::default() + .with_backend(EncodeBackendPreference::CpuOnly) + .with_block_coding_mode(J2kBlockCodingMode::Classic) + .with_reversible_transform(ReversibleTransform::None53) + .with_max_decomposition_levels(Some(1)), + ) + .expect("subsampled encode"); + + for roi in [ + Rect { + x: 8, + y: 6, + w: 16, + h: 18, + }, + Rect { + x: 28, + y: 24, + w: 24, + h: 24, + }, + Rect { + x: 68, + y: 52, + w: 20, + h: 20, + }, + ] { + assert_region_decode_matches_whole_decode_crop(&encoded.codestream, (width, height), roi); + } +}