diff --git a/.github/testshards/internal-ui.tsv b/.github/testshards/internal-ui.tsv index 3e479854..3576cd31 100644 --- a/.github/testshards/internal-ui.tsv +++ b/.github/testshards/internal-ui.tsv @@ -5,7 +5,7 @@ # baseline-run: 34627413846 # baseline-attempts: 1 # weights: single CI attempt; projected test sums, not measured shard runtimes -# ui-1: 236 entries, 470.520s projected weight sum +# ui-1: 237 entries, 470.520s projected weight sum # ui-2: 248 entries, 470.530s projected weight sum # ui-3: 202 entries, 504.870s projected weight sum # rebalance: 27 whole-test moves; slowest projected sum within 5% of mean @@ -57,7 +57,8 @@ TestCompareLinkControl_CtrlLAndButtonShareReadyGate ui-1 TestCompareLinkToggle_CanvasOverlayOwnsPhysicalCtrlL ui-1 TestCompareLinkToggle_ChainsHookPersistsAndRelinksOnSecondPress ui-1 TestCompareLinkToggle_ZoomsOnlyTheLastHoveredPaneWithoutHeldModifier ui-1 -TestCompareMemory_HoldsBothFullDecodesBeyondSharedCacheBudget ui-1 +TestCompareMemory_LoadAdmission ui-1 +TestCompareMemory_RejectsSourcesWhoseCombinedEstimateExceedsBudget ui-1 TestCompareOpenRefusal_DropDialogShortcutAndOpenWithAreDiscarded ui-1 TestComparePanInputs_CanvasDragAndShiftWheelStayInComparison ui-1 TestCompareRestoration_EscapeRevealsTheUnchangedFilteredGrid ui-1 diff --git a/finished_refactorings/2026-09-16-bound-comparison-memory.md b/finished_refactorings/2026-09-16-bound-comparison-memory.md new file mode 100644 index 00000000..fd86dbc3 --- /dev/null +++ b/finished_refactorings/2026-09-16-bound-comparison-memory.md @@ -0,0 +1,138 @@ +# Bound comparison memory + +## Problem + +Comparison starts both selected-source loads concurrently. Each load previously +used the complete image-cache budget, so two decoded sources and unused GIF +frames could remain live beyond the shared budget. + +## Acceptance criteria + +1. Each pane is admitted against half the current shared image-cache budget + before pixel decoding. + Verify: `go test -tags no_emoji,nodynamic ./internal/ui -run '^TestCompareMemory_' -count=1` +2. Comparison decodes only the first frame of an animated GIF. + Verify: `go test -tags no_emoji,nodynamic ./internal/ui -run '^TestCompareAnimated_' -count=1` +3. Existing comparison behavior remains green. + Verify: `go test -tags no_emoji,nodynamic ./internal/ui -run '^TestCompare' -count=1` + +## Non-goals and limit + +This does not downsample sources that exceed admission. It rejects them through +the existing comparison failure path. Splitting the budget equally is +deliberately conservative: it preserves concurrent loading without needing an +allocation reservation service, at the cost of rejecting an asymmetric pair +whose combined estimate might otherwise fit. + +## Tasks + +### Task 1 - Guard comparison decoding + +Owner: T0 inline +Files: `internal/ui/compare.go`, `internal/imaging/gif.go`, existing UI tests +Test: comparison budget and animated-source regressions in the existing test file +Verify: acceptance commands above +Budget: 0 spawns, 1 review round, full suite yes + +## Original implementation verification + +- The focused comparison and cache-writer regressions pass. +- Imaging GIF fallback regressions pass. +- `make fmt` and `make vet` pass. +- `make verify` and `make check-test-shards` could not start because Docker is + not installed in the environment. `make test-native` reached the full UI + suite; it exposed and prompted correction of the animated-cache contract, + while an unrelated Explorer settings test failed and then passed in isolation. +- GoLand/Qodana inspection tooling is unavailable in the environment. + +## PR #31 review follow-up + +Route: Standard. The lead owns the fixes and review. One read-only scout locates +existing fixture and catalogue-verification patterns; no implementation or +review is delegated. No dependencies or distribution obligations change. + +Acceptance criteria and tasks, all owned by T0: + +1. Replace the renamed comparison test in the shard manifest, assign any new + comparison regression, and keep the entry counts current. + Verify: `make check-test-shards`. +2. Header-only decoded-pixel estimates cover eight-byte RGBA64/NRGBA64 pixels. + Cached comparisons charge the detached first-frame record's actual retained + bytes, including vector storage, and fresh decodes receive the same final + check. Cover accepted and refused tiny 16-bit sources with and without a + cache hit, and preserve complete cached animation records. + Verify: `go test -tags no_emoji,nodynamic ./internal/imaging ./internal/ui -run '^(TestEstimateDecodedBytes|TestImageBytes|TestCompare|TestImageCacheWriters_PreserveCompleteRecords)' -count=1`. +3. Budget refusals reach a localized toast, with English/German catalogue keys. + Verify: the comparison memory regressions above and + `go test -tags no_emoji,nodynamic . -run '^TestTranslations_' -count=1`. + +Files: comparison adapter/tests, imaging byte accounting/tests, shard manifest, +English/German catalogues, this evidence record and `todos.md`. Update the +preload test's estimate comment because it uses the shared conservative helper. +The shared helper also makes speculative preload admission more conservative; +actual byte-cache weights remain format-specific. These are decoded-image +admission limits, not a bound on all decoder scratch, renderer or process memory. + +Verification: targeted regressions first, then GoLand inspections and +`make verify`; native amd64 CI supplies the complete suite if the local Docker +daemon cannot meet the required platform. Commit and push are user-authorized. +Budget: one scout, one final review, one complete-suite gate. + +### Follow-up verification evidence + +- Before the fix, the tiny-fixture tests failed for underestimated RGBA64 and + NRGBA64 storage, admission of a 16-bit source above its pane allowance on + both fresh and cached paths, and the untranslated budget-refusal toast. +- After the fix, focused comparison, full-cache-record, byte-accounting, + preload and GIF-fallback regressions pass. The complete native `imaging`, + `ui/display` and `ui/compare` package suites also pass. +- `go test -tags no_emoji,nodynamic . -run '^TestTranslations_' -count=1` passes. +- `make check-test-shards` passes in Linux/amd64 Docker: 687 runnable tests, + three shards. Its earlier run failed on the unassigned comparison test. +- `make verify-build` passes: formatting, TUF root, Qodana test exclusions, + generated assets/notices, vet and build. `git diff --check` is clean. +- GoLand inspected all PR code files and both changed catalogues, including + weak warnings. The only findings are two unchanged duplicate setup fragments + in `imgcache_test.go`, in tests of different removal entry points. The exact + file is already excluded from `DuplicatedCode` in `qodana.yaml`; retain that + existing test-only exclusion. No production issue or new inspection finding + remains. The unused `assertFrozen` parameter introduced by the PR was removed + by using its existing assertion helper directly. +- `make verify` refuses the local Docker daemon's `linux/aarch64` platform. + The full race suite runs in native Linux/amd64 CI; no isolation policy or + test was weakened to bypass this prerequisite. Current-head hosted results + and the final review disposition are recorded in + [PR #31](https://github.com/frathe/picfetch/pull/31). + +Actual cost: one read-only scout, one lead review, one full-gate attempt. +The user authorized the fix commit and push to PR #31; the unrelated untracked +GitHub AI scan failure report is excluded from the commit. + +## GitHub Codex review loop + +The user invoked the repository review loop after `8cdbb18`. No unresolved +review threads existed at entry. Codex code/security reviews were already +running for that head, so no duplicate request was posted. +The initial Codex code and security reviews completed without findings; the +connector posted its approval reaction after security finished. All eight CI jobs +passed on merge `41d406d` (head `8cdbb18`, base `459663d`), including all four +native Linux race partitions. Both CodeQL analyses passed with no new alerts. + +The post-suppression Qodana SARIF for `8cdbb18` reported one +`GoUnusedExportedFunction` warning on `imaging.IsAnimatedGIF`. It is a false +positive: `internal/ui/compare.go` calls the function before deciding whether +to cache a decoded comparison source. Keep the function and scope the +suppression to that declaration, with its comparison-cache purpose documented. +Local GoLand inspection confirms the declaration and caller have no findings; +hosted Qodana must verify the suppression on the next pushed commit. + +The separate GitHub Advanced Security AI scan failed before producing results +with `CAPIError: 400 The requested model is not supported`. This service error +is distinct from the Codex connector's security review and is not a code +finding. Track its disposition separately in the PR's final check evidence. + +Acceptance remains a fresh finding-free Codex code review on the final pushed +commit, completed Codex security review, no actionable Qodana/CodeQL findings, +and passing required CI. Review replies and final job/report links in PR #31 +are the live evidence record, so recording outcomes does not create a new, +unreviewed source commit. Merging and releasing remain outside this workflow. diff --git a/internal/imaging/bytecache.go b/internal/imaging/bytecache.go index cf9554eb..f03b0953 100644 --- a/internal/imaging/bytecache.go +++ b/internal/imaging/bytecache.go @@ -335,7 +335,7 @@ func imageBytes(m image.Image) int64 { } // Anything else (a decoder's own image type, or a wrapper) falls back - // to the four-bytes-per-pixel ceiling - an over-estimate is the safe + // to the eight-bytes-per-pixel ceiling - an over-estimate is the safe // direction for a budget. return EstimateDecodedBytes(m.Bounds()) } @@ -376,14 +376,14 @@ func loadedImageBytes(l *LoadedImage) int64 { // EstimateDecodedBytes is the worst-case decoded size of an image whose // header declares these bounds - for callers deciding whether a decode is // worth starting at all, before there is any concrete image type to -// measure. Deliberately the four-bytes-per-pixel ceiling: guessing low -// here would let exactly the images this budget exists to bound slip -// through the check. +// measure. Eight bytes per pixel covers the RGBA64 and NRGBA64 outputs of +// supported sixteen-bit formats as well as the smaller eight-bit outputs. +// This estimates retained pixels, not transient decoder working memory. func EstimateDecodedBytes(b image.Rectangle) int64 { w, h := int64(b.Dx()), int64(b.Dy()) if w <= 0 || h <= 0 { return 0 } - return w * h * 4 + return w * h * 8 } diff --git a/internal/imaging/bytecache_test.go b/internal/imaging/bytecache_test.go index e5871a12..317e9f32 100644 --- a/internal/imaging/bytecache_test.go +++ b/internal/imaging/bytecache_test.go @@ -346,7 +346,7 @@ func TestImageBytes(t *testing.T) { // block: 16 + 4 + 4. This is the case the type switch exists for - // charging a JPEG 4 bytes per pixel would over-report it by 2.7x. {"YCbCr 4:2:0", image.NewYCbCr(image.Rect(0, 0, 4, 4), image.YCbCrSubsampleRatio420), 16 + 4 + 4}, - {"unknown type falls back to 4 bytes per pixel", unknownImage{image.Rect(0, 0, 4, 4)}, 4 * 4 * 4}, + {"unknown type falls back to 8 bytes per pixel", unknownImage{image.Rect(0, 0, 4, 4)}, 4 * 4 * 8}, } for _, c := range cases { @@ -382,7 +382,7 @@ func TestEstimateDecodedBytes(t *testing.T) { bounds image.Rectangle want int64 }{ - {"ordinary bounds", image.Rect(0, 0, 100, 50), 100 * 50 * 4}, + {"ordinary bounds", image.Rect(0, 0, 100, 50), 100 * 50 * 8}, {"empty bounds", image.Rect(0, 0, 0, 0), 0}, // Built as a literal rather than with image.Rect, which // canonicalizes swapped corners - a negative Dx/Dy has to come out @@ -397,6 +397,14 @@ func TestEstimateDecodedBytes(t *testing.T) { } }) } + for _, frame := range []image.Image{ + image.NewRGBA64(image.Rect(0, 0, 3, 2)), + image.NewNRGBA64(image.Rect(0, 0, 3, 2)), + } { + if estimate, retained := EstimateDecodedBytes(frame.Bounds()), imageBytes(frame); estimate < retained { + t.Errorf("estimate for %T = %d, less than retained pixels %d", frame, estimate, retained) + } + } } // --- NewImgCache / NewThumbCache --------------------------------------------- diff --git a/internal/imaging/gif.go b/internal/imaging/gif.go index a265213f..e99d0339 100644 --- a/internal/imaging/gif.go +++ b/internal/imaging/gif.go @@ -25,6 +25,17 @@ const ( gifPreviewFrameOverhead = 128 ) +// IsAnimatedGIF reports whether data describes a GIF with more than one +// image frame. It performs the same bounded structural probe used for +// animation admission and does not decode pixels. +// Comparison calls it to keep first-frame-only records out of the image cache. +// +//goland:noinspection GoUnusedExportedFunction +func IsAnimatedGIF(data []byte) bool { + count, _, _, ok := probeGIF(data) + return ok && count > 1 +} + // gifWorkingBytes estimates paletted source frames, per-frame storage, // decoder scratch and two full RGBA compositing canvases. Retained output // pixels are charged separately, since previews can be smaller than the source. diff --git a/internal/ui/compare.go b/internal/ui/compare.go index 646ed942..68048274 100644 --- a/internal/ui/compare.go +++ b/internal/ui/compare.go @@ -2,7 +2,9 @@ package ui import ( "context" + "errors" "fmt" + "image" "fyne.io/fyne/v2" "fyne.io/fyne/v2/lang" @@ -10,6 +12,8 @@ import ( "github.com/frathe/picfetch/internal/imaging" ) +var errComparisonMemoryBudget = errors.New("comparison memory budget exceeded") + // comparisonActive is the composition-layer fact used by every ordinary // command entry. Feature packages stay independent: none of them needs to // know comparison exists, and comparison itself never learns about the @@ -65,31 +69,65 @@ func (v *viewer) loadComparedImage(ctx context.Context, uri fyne.URI) (*imaging. writer := v.imgCache.Capture() if loaded, ok := v.imgCache.Get(uri.String()); ok { if writer.Current() { - return loaded, nil + if loaded == nil || len(loaded.Frames) == 0 { + return loaded, nil + } + // Comparison displays only the first frame. Do not retain an + // animation's unused frames outside their existing cache owner. + frozen := *loaded + frozen.Frames = []image.Image{loaded.Frames[0]} + frozen.Delays = nil + if err := comparisonImageFits(&frozen, v.imgCache.Budget()); err != nil { + return nil, err + } + return &frozen, nil } continue } - data, _, err := imaging.ReadAndProbe(ctx, uri) + data, bounds, err := imaging.ReadAndProbe(ctx, uri) if err != nil { if !writer.Current() { continue } return nil, err } - loaded, err := imaging.DecodeRecord(ctx, data, v.imgCache.Budget()) + if imaging.EstimateDecodedBytes(bounds) > v.imgCache.Budget()/2 { + return nil, errComparisonMemoryBudget + } + // Comparison never animates, so decoding additional GIF frames would + // consume memory that cannot contribute to either pane. + loaded, err := imaging.DecodeRecord(ctx, data, 0) if !writer.Current() { continue } if err != nil { return nil, err } - if writer.Add(uri.String(), loaded) { - return loaded, nil + if err := comparisonImageFits(loaded, v.imgCache.Budget()); err != nil { + return nil, err } + if !imaging.IsAnimatedGIF(data) { + writer.Add(uri.String(), loaded) + } + return loaded, nil + } +} + +func comparisonImageFits(loaded *imaging.LoadedImage, budget int64) error { + if loaded == nil || len(loaded.Frames) == 0 { + return nil + } + if loaded.DecodedBytes() > budget/2 { + return errComparisonMemoryBudget } + return nil } func (v *viewer) compareFailed(uri fyne.URI, err error) { + if errors.Is(err, errComparisonMemoryBudget) { + v.ShowToast(lang.L("Selected images exceed the comparison memory budget")) + return + } v.ShowToast(fmt.Sprintf(lang.L("could not read %q: %v"), uri.Name(), err)) } diff --git a/internal/ui/compare_fidelity_test.go b/internal/ui/compare_fidelity_test.go index b9276e45..4c5c003a 100644 --- a/internal/ui/compare_fidelity_test.go +++ b/internal/ui/compare_fidelity_test.go @@ -1,10 +1,12 @@ package ui import ( + "bytes" "context" "errors" "image" "image/color" + "image/png" "slices" "strings" "testing" @@ -145,36 +147,23 @@ func TestCompareAnimated_FreezesFirstDecodedFrameForEntireSession(t *testing.T) waitForCompare(t, v) loads := collectCompareLoads(t, observed) loaded := requireSuccessfulCompareLoad(t, loads, "b-motion.gif") - if got := len(loaded.Frames); got != 2 { - t.Fatalf("animated comparison decoded frames = %d, want 2", got) + if got := len(loaded.Frames); got != 1 { + t.Fatalf("animated comparison decoded frames = %d, want only the displayed first frame", got) } - first, later := loaded.Frames[0], loaded.Frames[1] + first := loaded.Frames[0] r, _, b, _ := first.At(12, 8).RGBA() if r <= b { t.Fatalf("first animation frame = R:%d B:%d, want red", r, b) } - r, _, b, _ = later.At(12, 8).RGBA() - if b <= r { - t.Fatalf("second animation frame = R:%d B:%d, want blue", r, b) - } - assertFrozen := func(stage string) { - t.Helper() - comparisonImageHolding(t, v, first) - for _, candidate := range comparisonShaders(v.compare.Overlay()) { - if candidate.Textures["overview"] == later { - t.Fatalf("comparison displayed a later animation frame %s", stage) - } - } - } - assertFrozen("after load") + comparisonImageHolding(t, v, first) v.handleKeyEvent(&fyne.KeyEvent{Name: fyne.KeyPlus}) - assertFrozen("after zoom") + comparisonImageHolding(t, v, first) fynetest.Tap(comparisonButton(t, v.compare.Overlay(), lang.L("Swipe"))) v.win.Resize(fyne.NewSize(900, 620)) - assertFrozen("after layout and resize") + comparisonImageHolding(t, v, first) fynetest.Tap(comparisonButton(t, v.compare.Overlay(), lang.L("Swap"))) - assertFrozen("after Swap") + comparisonImageHolding(t, v, first) } func TestCompareOrientation_UsesCanonicalEXIFPixelsAndIgnoresViewerRotation(t *testing.T) { @@ -219,7 +208,7 @@ func TestCompareOrientation_UsesCanonicalEXIFPixelsAndIgnoresViewerRotation(t *t comparisonImageHolding(t, v, frame) } -func TestCompareMemory_HoldsBothFullDecodesBeyondSharedCacheBudget(t *testing.T) { +func TestCompareMemory_RejectsSourcesWhoseCombinedEstimateExceedsBudget(t *testing.T) { left := storage.NewFileURI(uitest.WriteTempFile(t, "a-wide.png", uitest.EncodePNG(t, 401, 211, color.RGBA{R: 255, A: 255}))) right := storage.NewFileURI(uitest.WriteTempFile(t, "b-tall.png", @@ -228,45 +217,103 @@ func TestCompareMemory_HoldsBothFullDecodesBeyondSharedCacheBudget(t *testing.T) v.imgCache.SetBudget(1) v.imgCache.Purge() - started := make(chan struct{}, 2) - release := make(chan struct{}) - observed := make(chan observedCompareLoad, 2) - v.compareLoad = func(ctx context.Context, uri fyne.URI) (*imaging.LoadedImage, error) { - started <- struct{}{} - select { - case <-release: - case <-ctx.Done(): - return nil, ctx.Err() - } - loaded, err := v.loadComparedImage(ctx, uri) - observed <- observedCompareLoad{uri: uri, loaded: loaded, err: err} - return loaded, err - } + observed := observeRealCompareLoads(v) fireCompareShortcut(v) - for range 2 { - select { - case <-started: - case <-time.After(testTimeout): - t.Fatal("comparison did not start both full-image loads concurrently") - } - } - close(release) waitForCompare(t, v) loads := collectCompareLoads(t, observed) - leftLoaded := requireSuccessfulCompareLoad(t, loads, "a-wide.png") - rightLoaded := requireSuccessfulCompareLoad(t, loads, "b-tall.png") - if got, want := leftLoaded.Frames[0].Bounds(), image.Rect(0, 0, 401, 211); got != want { - t.Errorf("left decoded bounds = %v, want full source %v", got, want) + budgetRefusals := 0 + for name, result := range loads { + if result.loaded != nil || result.err == nil { + t.Errorf("comparison loader for %q = (%v, %v), want memory-budget refusal", name, result.loaded, result.err) + continue + } + if errors.Is(result.err, errComparisonMemoryBudget) { + budgetRefusals++ + } else if !errors.Is(result.err, context.Canceled) { + t.Errorf("comparison loader for %q error = %v, want memory-budget refusal or peer cancellation", name, result.err) + } } - if got, want := rightLoaded.Frames[0].Bounds(), image.Rect(0, 0, 233, 377); got != want { - t.Errorf("right decoded bounds = %v, want full source %v", got, want) + if budgetRefusals == 0 { + t.Fatal("comparison did not reject either oversized source against its shared budget") } - comparisonImageHolding(t, v, leftLoaded.Frames[0]) - comparisonImageHolding(t, v, rightLoaded.Frames[0]) - if got := v.imgCache.Len(); got != 1 { - t.Errorf("one-byte full-image cache retained %d entries, want 1 while comparison still holds both panes", got) + if got := v.imgCache.Len(); got != 0 { + t.Errorf("refused comparison retained %d cache entries, want 0", got) + } + if got, want := v.toast.text.Text, lang.L("Selected images exceed the comparison memory budget"); got != want { + t.Errorf("comparison refusal toast = %q, want %q", got, want) + } + settleToast(t, v) +} + +func TestCompareMemory_LoadAdmission(t *testing.T) { + frame := image.NewNRGBA64(image.Rect(0, 0, 3, 2)) + frame.SetNRGBA64(0, 0, color.NRGBA64{R: 0x1234, A: 0x8001}) + var encoded bytes.Buffer + if err := png.Encode(&encoded, frame); err != nil { + t.Fatal(err) + } + uri := storage.NewFileURI(uitest.WriteTempFile(t, "six-pixels.png", encoded.Bytes())) + const frameBytes = 3 * 2 * 8 + for _, cached := range []bool{false, true} { + for _, tc := range []struct { + name string + budget int64 + fits bool + }{ + {"below_limit", 2*frameBytes - 1, false}, + {"at_limit", 2 * frameBytes, true}, + } { + name := "decode/" + tc.name + if cached { + name = "cached/" + tc.name + } + t.Run(name, func(t *testing.T) { + v := newTestViewer(t) + v.imgCache.SetBudget(tc.budget) + if cached { + v.imgCache.Add(uri.String(), &imaging.LoadedImage{Frames: []image.Image{frame}}) + } + loaded, err := v.loadComparedImage(context.Background(), uri) + if !tc.fits { + if loaded != nil || !errors.Is(err, errComparisonMemoryBudget) { + t.Fatalf("load = (%v, %v), want refusal", loaded, err) + } + if !cached && v.imgCache.Len() != 0 { + t.Fatal("refused decode populated the cache") + } + return + } + if err != nil { + t.Fatal(err) + } + if got := loaded.DecodedBytes(); got != frameBytes { + t.Fatalf("retained bytes = %d, want %d for 16-bit pixels", got, frameBytes) + } + }) + } } + + t.Run("cached_animation_charges_only_first_frame", func(t *testing.T) { + v := newTestViewer(t) + first := image.NewRGBA(image.Rect(0, 0, 3, 2)) + complete := &imaging.LoadedImage{ + Frames: []image.Image{first, image.NewRGBA(first.Bounds()), image.NewRGBA(first.Bounds())}, + Delays: []time.Duration{time.Second, time.Second, time.Second}, + } + v.imgCache.SetBudget(2 * int64(len(first.Pix))) + v.imgCache.Add(uri.String(), complete) + loaded, err := v.loadComparedImage(context.Background(), uri) + if err != nil { + t.Fatal(err) + } + if loaded == complete || len(loaded.Frames) != 1 || loaded.Frames[0] != first || len(loaded.Delays) != 0 { + t.Fatal("comparison must retain a detached first-frame record") + } + if record, ok := v.imgCache.Get(uri.String()); !ok || record != complete || len(record.Frames) != 3 || len(record.Delays) != 3 { + t.Fatal("comparison changed the complete cached animation") + } + }) } func TestCompareInputLimit_FailsWithoutRemovingEitherSelectedSource(t *testing.T) { diff --git a/internal/ui/imgcache_test.go b/internal/ui/imgcache_test.go index 1f8260f2..d9a8415c 100644 --- a/internal/ui/imgcache_test.go +++ b/internal/ui/imgcache_test.go @@ -47,6 +47,12 @@ func TestImageCacheWriters_PreserveCompleteRecords(t *testing.T) { t.Fatal(err) } } + if path == "comparison" && tc.name == "animated.gif" { + if _, ok := v.imgCache.Get(u.String()); ok { + t.Fatal("first-frame-only comparison record must not replace a complete animated cache record") + } + return + } loaded, ok := v.imgCache.Get(u.String()) if !ok { t.Fatal("image was not cached") @@ -234,8 +240,8 @@ func TestPreloadOne_SkipsANeighborTooLargeForTheBudget(t *testing.T) { a := uitest.TempJPEGURI(t, "a.jpg", 64, 64, color.White) b := uitest.TempJPEGURI(t, "b.jpg", 64, 64, color.White) - // 64x64 estimates at 16,384 decoded bytes (4 per pixel). A 16 KiB budget - // puts that exactly at the budget and so past the half-budget line + // 64x64 conservatively estimates at 32,768 decoded bytes (8 per pixel). + // A 16 KiB budget puts that past the half-budget line // preloadOne bails at - the point where the current image and one // neighbor stop both fitting. v.imgCache.SetBudget(16 * 1024) diff --git a/todos.md b/todos.md index f3abda6e..fb18b65f 100644 --- a/todos.md +++ b/todos.md @@ -50,6 +50,11 @@ Contracts and evidence workflow: [review record](finished_refactorings/2026-09-1 - Bound ICO and SVG input processing, enforce WASM AVIF builds, and account for GIF frame overhead before animation decoding; preserve static GIF fallback. +- Bound comparison decodes to half the shared image budget per pane and avoid + retaining animation frames that comparison never displays. Include 16-bit + pixel admission, actual cached-frame weights, localized refusals and the + corrected UI shard assignment. Hosted review dispositions and final CI, + Qodana and CodeQL evidence: [PR #31](https://github.com/frathe/picfetch/pull/31). - Bound Visual Similarity Explorer collection work and worker event decoding to prevent attacker-controlled collections from exhausting CPU or viewer memory. diff --git a/translations/de.json b/translations/de.json index 838887e2..4f7312bc 100644 --- a/translations/de.json +++ b/translations/de.json @@ -171,6 +171,7 @@ "Show variants": "Varianten anzeigen", "Compare selected images": "Ausgewählte Bilder vergleichen", "Select exactly 2 images to compare": "Wählen Sie genau 2 Bilder zum Vergleichen aus", + "Selected images exceed the comparison memory budget": "Die ausgewählten Bilder überschreiten das Speicherlimit für den Vergleich", "Swipe": "Wischen", "Side by side": "Nebeneinander", "Swap": "Tauschen", diff --git a/translations/en.json b/translations/en.json index bb12baf5..1b81d66e 100644 --- a/translations/en.json +++ b/translations/en.json @@ -171,6 +171,7 @@ "Show variants": "Show variants", "Compare selected images": "Compare selected images", "Select exactly 2 images to compare": "Select exactly 2 images to compare", + "Selected images exceed the comparison memory budget": "Selected images exceed the comparison memory budget", "Swipe": "Swipe", "Side by side": "Side by side", "Swap": "Swap",