Repository navigation
J2C fetches decode at the level their bytes reach, not the level requested - #552
Merged
Merged
Conversation
calcDataSizeJ2C estimates a discard level's bytes from the texture's dimensions alone: 128 KB for a 2048 at discard 1, about 9% of a median 1.39 MB file. Second Life's LRCP codestreams hold only their first quality layers in that much, and its single-layer ones are cut inside a low resolution, so a decode at the level comes out soft or very blurry: 2048 multi-layer at discard 1 measured 43.9 dB PSNR at the median and 29.9 dB at p10 against the decode of the whole file, single-layer 23.9 dB. ALJ2CLayout reads SIZ, COD and the first SOT without OpenJPEG. A codestream of one tile in one tile-part, the shape all 52,000 complete entries of a texture cache have, ends at that SOT's offset plus Psot plus the EOC, and all of it sits in the first 600 bytes a fetch reads. LLImageJ2C parses it whenever setData or appendData changes its data (LLImageFormatted::onDataChanged) and in updateData, and calcDataSize and calcDiscardLevelBytes follow it: the target for discard d is the codestream's length times the fraction that decodes within 45 dB of the whole file's decode at that level, by the texture's longer side. The quality layers' byte tiers are fixed, so a small file needs a larger share: 90% at discard 1 up to 128 pixels, 65% from 1024. The sizes-only estimate stays for a codestream whose headers are not in hand. The fractions were calibrated on half of that cache and checked on the other half: at its targets every class of longer side and layer count decodes within 45 dB for 93.5% of its files or more. Against the starting table of 75/50/25/10/10% for every size, 2048s take 65/30/12/5/3%. TextureReverseByteRange decoded at half the estimate, which put back the softness the targets remove, and TextureNewByteRange chose between the estimate's two formulas. Both settings go, with LLImage's copies: the fractions carry the quality bar, and the estimate is always the smaller formula. Tests: the parser over the viewer's encoder and OpenJPEG's (layered, single-layer, four tiles, every cut of the header, a missing SOT), and decodes at every target of layered and single-layer codestreams, which pass 45 dB where the sizes-only estimate gives 30-36 dB at discard 1. With calcDataSize back on the estimate they fail at every level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the level its bytes reach With RenderMaxTextureResolution at 1024, 2048 textures stayed blurry after a relog until the setting was toggled. The fresh worker that asked for discard 1 read calcDataSizeJ2C's estimate from the cache (128 KB of a complete 1.39 MB entry), and CACHE_POST, like the HTTP completion, recorded the requested discard as loaded. A decode of the first quality layers at reduce 1 therefore counted as discard 1, the DONE -> INIT loop had nothing left to fetch, and the cap, which pins 2048s at discard 1, made it permanent. Toggling to 2048 asked for discard 0, which fetches everything. The worker copies the codestream's layout from its formatted image once the headers are in (updateLayout) and keeps it for the texture's later requests, since endWork() lets go of the image after each one. It sizes every request for discard 1 and up from it, in createRequest and setDesiredDiscard as well as in the running request; discard 0 still asks for everything. When the headers arrive with a cache read the estimate sized and the entry filled that read, it reads on from the entry instead of decoding what it has. CACHE_POST and the HTTP completion set mLoadedDiscard to the finest level the bytes reach and never finer than the request (calcLoadedDiscard), so a response short of the target decodes as a preview and the DONE -> INIT loop fetches the rest. DONE weighs that level as well as the decode's, which a codestream with fewer resolutions than the level loaded makes finer than the bytes support. A cache read that needs half the codestream or more takes all of it: local disk is cheap next to a second read. A request whose network fetch fails with data in hand decodes what it holds and finishes, as before (mFetchFailed). Without that, a preview short of the target made DONE ask again after every failure, with no backoff for ordinary textures: a decode, a cache read and a request per cycle for as long as the server failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ends The SOT walker mapped tile-part ends to discard levels on the assumption of one tile-part per resolution. Every Second Life texture has one tile-part, so every level got the whole file, and a partial walk mapped discard 0 to the last tile-part walked rather than the end of the codestream. LLImageJ2C takes its targets from ALJ2CLayout now. getMetadata's guess at mDiscardLevel, which updateData overwrote, goes with it. The registry's openjpeg 2.5.4#31 drops opj_j2k_walk_tile_part_ends; nothing here calls it, so the viewer builds against either port version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 47.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 12 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main JPEG 2000 fetch behavior change: decoding follows the level supported by received bytes rather than the requested level. Description check The description gives detailed problem, cause, implementation, testing, and remaining validation context. It does not include the template's Related Issues, Checklist, or Additional Notes sections, bu… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description gives detailed problem, cause, implementation, testing, and remaining validation context. It does not include the template's Related Issues, Checklist, or Additional Notes sections, but the core information is complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 47.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 12 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit watched the headers parse,
Then counted bytes across the grass.
The cache sent crumbs; the stream sent more,
Each discard level found its score.
The rabbit thumped and hopped away,
While textures loaded clear as day.
Comment @coderabbitai help to get the list of available commands.
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.
Problem
With Maximum LOD resolution (
RenderMaxTextureResolution) at 1024, 2048 textures stayed very blurry after a relog until the setting was toggled up and back down.Cause
A fetch for discard 1 and above asked for
calcDataSizeJ2C's estimate, which is based on dimensions only. For a 2048 at discard 1 that is 128 KB, about 9% of a median 1.39 MB file.CACHE_POSTand the HTTP completion then recorded the requested discard as loaded. So a reduce-1 decode of the first few quality layers counted as discard 1, and nothing fetched the rest. The cap holds 2048s at discard 1, so they never recovered. Toggling to 2048 asks for discard 0, which always fetches the whole file.This started with ed0162a, which replaced LL's inflated curve (effectively the whole file for discards 0–2) with the old sqrt estimate. The same flaw softened every texture whose desired discard stays above 0, cap or no cap.
Measured on a 52k-texture cache:
Change
ALJ2CLayoutreads SIZ, COD and the first SOT without OpenJPEG. Total length = SOT offset + Psot + EOC, and all of it is in the first 600 bytes a fetch reads.LLImageJ2C::calcDataSizetargets the codestream length times the fraction that decodes within 45 dB of the full decode at that level, by longer side. For 2048s that is 65/30/12/5/3% at discards 1–5. The fractions were calibrated on half the cache and validated on the other half: at least 93.5% of each class decodes within 45 dB. The dimensions-only estimate stays for a codestream whose header is not in hand.mLoadedDiscardis the finest level the bytes reach (calcLoadedDiscard), so a short response decodes as a preview and the existingDONE -> INITloop fetches the rest.mFetchFailed), rather than retrying every cycle.TextureReverseByteRangeandTextureNewByteRange. The first decoded at half the estimate, which brought the softness back.LLImageJ2COJ::getMetadata. It mapped every SL texture's discard levels to the whole file, and it never ran on the fetch path.The matching registry change, openjpeg 2.5.4#31 without
opj_j2k_walk_tile_part_ends, is pushed to alchemy-registry (dc47644). Nothing here calls the walker, so this builds against #30 or #31, and the baseline bump can come separately.Testing
alj2clayout_testcovers:calcDataSizeput back on the old estimate, the tests fail at every level.ctest -L llimagepasses 6/6, and the viewer builds (Windows, RelWithDebInfo).🤖 Generated with Claude Code