fix(server-rs): derive info.json scaleFactors/sizes from one tile-grid pyramid - #785
Merged
Conversation
subotic
force-pushed
the
worktree-info-json-fix
branch
2 times, most recently
from
August 18, 2026 07:40
b9c6a22 to
ec0ed00
Compare
Overview
Labels (1 changes)
org.opencontainers.image.description=IIIF-compatible media server.
org.opencontainers.image.licenses=AGPL-3.0-only
-org.opencontainers.image.revision=7ee93b4b22e93e60e49663a4b4c13e68bec49df0
+org.opencontainers.image.revision=e8c63231e7d6ceae7324d4ad4dca5a9db285c27e
org.opencontainers.image.source=https://github.com/dasch-swiss/sipi
org.opencontainers.image.title=Sipi
org.opencontainers.image.version=v6.4.0Policies (0 improved, 0 worsened, 2 missing data)
|
…d pyramid scaleFactors advertised consecutive ordinals ([1,2,3,4]) instead of powers of two, and sizes described a different pyramid. OpenLayers derives maxZoom = round(log2(max(scaleFactors))) and orphans deeper levels, so assets blacked out at deep zoom (DEV-6933); OpenSeadragon silently discarded our sizes and rebuilt its own pyramid. Replace the two divergent generators in info.rs with a single pyramid derived from the tile grid, feeding both arrays from one source of truth: scaleFactors = [2^0..2^n] where n = max(levels_w, levels_h) puts the whole image inside one tile on both axes (clamped to 31 so a degenerate 1px tile can't truncate a factor to zero and divide-by-zero the sizes ladder); sizes = the native-inclusive dimensions at each factor; both ascending. clevels is no longer consulted for the descriptor (it was the source of the ordinal bug), and the FFI-seam comments in ffi.rs / sipi_ffi.h / sipi_ffi.cpp that still described it as feeding sizes[]/tiles[] are corrected. Untiled images derive sizes from a 512 reference tile and keep tiles omitted. Descriptor-only fix: the server already serves every level (SipiSize::REDUCE resamples on the fly, all HTTP 200), so correcting the advertised arrays repairs the existing corpus with no re-encoding. Updates the two defect-encoding fixtures (inline lena512 assertion and the e2e golden snapshot) to the corrected pyramid, and adds a conformance test pinning power-of-two factors, sizes/scaleFactors coherence (as literals), single-tile coverage, §5.4 tile uniqueness, and the exponent clamp. Fixes DEV-6981 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The info.json descriptor now derives its pyramid from the tile grid, so nothing on the Rust side reads clevels off the seam. Remove it from both SipiImageDims definitions, its population site, and the ABI guards; the C++ decode/cache paths keep using the engine's own info.clevels. It was the last field, so no offsets shift — the struct narrows 24 → 20 bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
subotic
force-pushed
the
worktree-info-json-fix
branch
from
August 19, 2026 10:59
91a1a73 to
267eedf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes DEV-6981
Motivation
SIPI's IIIF
info.jsonadvertisedscaleFactorsas consecutive ordinals(
[1,2,3,4]) instead of powers of two, and asizesarray that described adifferent pyramid than
scaleFactors. OpenLayers derivesmaxZoom = round(log2(max(scaleFactors)))and keeps every resolution, so anylevel beyond
maxZoomgets no tile URL — assets black out at deep zoom (rootcause of DEV-6933). OpenSeadragon masks the defect by discarding our
sizesandrebuilding its own power-of-two pyramid, so OSD rendering correctly was never
evidence the descriptor was sound.
This is a descriptor-only fix. The server already serves every level (all
HTTP 200;
SipiSize::REDUCEresamples on the fly), so correcting the advertisedarrays repairs the entire existing JP2 corpus with no re-encoding. It also
lands the pyramid contract the pyramidal-TIFF writer will later produce by
construction (project T1).
Summary
scaleFactorsare now contiguous powers of two[2^0..2^n]where the deepestfactor puts the whole image inside a single tile on both axes.
sizesandscaleFactorsare derived from one function, so they describe thesame pyramid (
sizes.length == scaleFactors.length); both emitted ascending.tile_width == 0) derivesizesfrom a 512 reference tile andkeep
tilesomitted.Key Changes
src/server-rs/src/info.rspyramid_scale_factors(w, h, tile_w, tile_h)— the single source of truth,n = max(levels_w, levels_h),u64shift so a deep pyramid can't overflow.size_pyramid()withsizes_for(w, h, &scale_factors)— a puretransform of the factor list (native-inclusive, ascending), so the two arrays
cannot drift apart.
image_info_jsoncomputes the factor list once and feeds bothsizesand thetilesblock from it; theclevelsread and the ordinal(1..cnt).collect()are gone (
clevelswas the source of the bug and is no longer consulted for thedescriptor).
tiles[].width/heightstill pass through the stored tile size verbatim (noregression of report finding 1).
Tests
inline
info_json_matches_lena512_golden(nowscaleFactors [1],sizes [{512,512}]) and the e2e golden snapshotiiif_compliance__info-json-lena512.snap. Reviewer: confirm these are thecorrected pyramid, not a re-blessed bug.
untiled_image_omits_tilesfor the 512-reference ladder.descriptor_conformancetest pins acceptance criteria 1–5 plus §5.4 tileuniqueness across a deep pyramid (3505×5156/1024 →
[1,2,4,8]), a single-tileimage (512×512/512 →
[1]), and a non-square-tile case (4096×4096, 1024×256 →[1,2,4,8,16]).Challenges and Decisions
Local Bazel gates could not run
Problem: the local Bazel graph failed to fetch Kakadu —
gh release download kakadu-v8.7from the privatedasch-swiss/dsp-ci-assetsrepo hit a stable REST404on GitHub's release-by-tag endpoint, even though GraphQL (gh release list) saw the release and repo access was fine (a concurrentgh api userreturned
503, so GitHub's release service looked degraded). Every gate(
bazel-test-unit,bazel-test-e2e,bazel-rustfmt-check,bazel-clippy-check)configures
//src/server-rs:lib, which transitively depends on Kakadu, so allfour were unrunnable.
Solution: the change is pure Rust arithmetic. Verified the pyramid math
standalone with
rustcagainst hand-computed expected values (lena512, the 0803deep-pyramid worked example, the untiled ladder, and the non-square-tile case all
match).
just commit-lintpassed. CI runs the full gate set on this PR.Coupling
sizestoscaleFactorsis convention, not specProblem: the IIIF spec (§5.9) deliberately ships two different ladders, so
coherence is not a MUST.
Solution: we couple them anyway because it is what makes OpenSeadragon use
our
sizes(OSD keepssizesonly whensizes.length ∈ {maxLevel, maxLevel+1},verified against OSD v5.0.1). Including the native size makes our count
n+1 = maxLevel+1, satisfying the check. §5.3 already requires the server toserve any advertised size — verified HTTP 200 — so coupling adds no capability the
server lacks.
Gotchas
to
[1,2,3,4,5,6,7]— that is the bug.descriptor_conformancepins2^itoprevent regression.
clevelsstays in the FFI struct (SipiImageDims, ABI-asserted inffi.rs); this change only stops info.rs from consulting it. No FFI change.Verify a previously-blacking-out asset against OL locally (
just run+ OLviewer) before relying on the exact orphaned-level count.
Test Plan
rustc(all cases match theplan's worked examples)
just commit-lint— greenbazel-test-unit,bazel-test-e2e,bazel-rustfmt-check,bazel-clippy-check(deferred to CI; blocked locally by the Kakadu fetch)0803-class asset with noorphaned levels (needs running SIPI + real asset)
scaleFactorsor asizes/scaleFactorslength mismatch (operator, read-only)