Hi-res texture packs on Android (opt-in, mobile-safe) + in-place .zip reading#235
Merged
Conversation
Rework the HiResPack engine so a pack is survivable on memory-tight platforms and can be read straight out of a .zip: - Decoded-RGBA8 LRU budget is platform-scaled and runtime-settable (gHiResTextures.CacheBudgetMB): 512 MB desktop / 128 MB Android default, applied in Init() once CVars load. LruCache gains SetBudget/EvictToBudget. - Per-texture upscale cap (kMaxPackTexels, mobile only) rejects a single oversize PNG so it can't blow the budget or the uncompressed GPU upload -- the LRU never evicts its just-inserted tail, so a lone over-budget entry would otherwise exceed the cap outright. - mods/ indexes .zip packs in place via libzip + stbi_load_from_memory (no extraction); loose PNGs and zip members share the same hash grammar, only the decode source differs. This is the distributed pack format. - Wrap the mods/ scan so a filesystem error (locked subdir, symlink loop, file removed mid-walk) disables the pack instead of crashing boot. - Fix the stale "~256 MB" budget figure in the Lookup() docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- gHiResTextures.Enabled defaults via kHiResEnabledDefault: on for desktop, OFF/opt-in on Android so a dropped-in pack can't silently allocate a large decoded working set on a phone. Hook and menu share the constant so the runtime default and the menu widget agree. - PortMenu: platform-default checkbox, an Android memory-cost note, and the dev dump-tooling (source/miss dumps) gated off touch UI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stop excluding port/hires/ on Android and define PORT_HIRES_ENABLED for Android US too, so the opt-in, mobile-budgeted pack compiles into libmain.so. JP still drops the pack entirely (US-only hash inputs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under scoped storage the native HiResPack scanner can only read files the app itself owns; a pack adb-pushed or dropped by a file manager into Android/data/<pkg>/ is owned by another uid and the recursive scan fails with "Permission denied". So the app copies the user-picked zip into its own mods/ (app-owned -> readable), reusing BootActivity's SAF flow. - PackImporter: streams the picked content:// URI straight to mods/<name>.zip (no cacheDir double-copy -- packs are ~200 MB) via a .part temp renamed only after it validates as a real ZIP. Every failure path deletes the partial; a full disk fails fast; the SAF display name is sanitized to a safe basename (no path traversal); a 1 GB cap rejects absurd input. Carries the v1.3 Android-pass discipline: clean up large staged files, validate untrusted input, fail fast on write error. - BootActivity: an "Import texture pack" launcher shortcut routes here with ssb64.import_pack=true -> SAF picker -> background import -> Toast -> normal route. Import runs BEFORE the SDL game starts, so HiResPack's scan sees the new pack single-threaded at boot -- no rescan race against the renderer. - Dev shortcut ssb64.dev_pack=<abs path> mirrors ssb64.dev_rom for scripted install without the picker UI. Verified on the API 34 arm64 emulator: import writes an app-owned mods/devpack.zip, the native scan indexes 4512 textures from it (no Permission denied), lookups hit ~87%, and the decoded-RGBA8 LRU stays pinned at the 128 MB Android budget under the full 219 MB pack with no low-memory kill. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Findings from an adversarial pass over the branch: - [high] ReadZipMember trusted zip_stat's declared member size and did out.resize(st.size) with no bound or bad_alloc handling -- a hostile or corrupt downloaded pack with a huge declared entry crashed the game via uncaught std::bad_alloc. Now reject size 0 / > 256 MB (real PNGs are ~25 MB; stays under INT_MAX so the later (int) cast for stbi can't overflow) and catch bad_alloc. - [low] Init() cleared gIndex/gOpenZips but not gLru -- a future re-scan with a changed pack could serve a stale decoded texture on an LRU hit. Add LruCache::Clear() and call it in Init(). - [low] PackImporter published with delete-then-rename (a window leaving mods/ with neither old nor new pack on rename failure) and could orphan the ~200 MB .part if ZipFile validation threw an Error. Now publish via atomic Files.move(REPLACE_EXISTING) inside a published-flag try/finally that deletes the temp on any non-success exit. - [low] The ssb64.dev_pack BootActivity extra is now gated behind BuildConfig.DEBUG so a release build's exported launcher activity can't be driven by another app into an arbitrary-path import (confused deputy). Re-verified on the emulator: import still writes an app-owned mods/devpack.zip, the scan indexes 4512 textures with zero spurious size rejections, and lookups hit ~86%. The review also confirmed the init-race is absent and the LRU tail-eviction invariant, budget floor, and entry-lifetime are sound. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ards) A deeper review of the branch surfaced three items beyond the first pass: - [medium] BootActivity runs the multi-second pack import on a background thread but had no configChanges, so a config change in that window (multi-window resize / dark mode / font scale; rotation is already locked landscape) recreates the Activity and the worker posts its result to the dead instance, leaving the app stuck on the picker. Give BootActivity a configChanges list so it absorbs those without recreation. Also covers the identical pre-existing ROM-picker case. - [low] safeZipName: a SAF display name of " .zip" trims to ".zip" and would land as a hidden file; reset that to "pack.zip". - [low] ScanZip: log when zip_get_num_entries returns -1 (corrupt central directory) instead of silently treating the pack as empty. The same review confirmed the earlier fixes hold and that the remaining flagged items are non-bugs (log-before-erase, tail-eviction invariant, zero-budget floor, the redundant-but-safe post-Insert Get, boot-only init with no concurrent Lookup). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Enables the hi-res texture pack on Android — previously compiled out — with mobile-safe limits, lets a pack be read straight from a
.zip(no extraction) on every platform, and adds a SAF importer so a phone user can install a pack under scoped storage.What's in it (6 commits, layered)
gHiResTextures.CacheBudgetMB); per-texture upscale cap on mobile; reads.zippacks inmods/in place via libzip +stbi_load_from_memory; the scan is wrapped so a badmods/disables the pack instead of crashing boot; fixed the stale "256 MB" docstring.gHiResTextures.Enableddefaults off/opt-in on Android (a dropped-in pack can't silently allocate a large working set); menu default + dev dump-tooling gated off touch UI.port/hires/on Android, definePORT_HIRES_ENABLEDfor Android US (JP still excluded — US-only hash inputs).PackImportercopies a SAF-picked zip into the app-ownedmods/(the only way the native scanner can read it under scoped storage), via an "Import texture pack" launcher shortcut reusingBootActivity's flow. No double-copy,.part→rename, partial cleanup, ENOSPC fail-fast, filename sanitization, ZIP validation, size cap. Import runs before SDL starts → no rescan race.bad_allocguard (untrusted-pack OOM/crash);LruCache::Clear()on re-init; atomicFiles.movepublish + try/finally.partcleanup; debug-gated thedev_packextra.BootActivityconfigChangesso a config change during import can't recreate it and strand the result; degenerate.zipfilename guard; log on corrupt-zipzip_get_num_entries.Validation
pack.zipinmods/→ 4512 textures indexed from inside it, ~95% lookup hit rate, 0 decode-fails, hi-res rendering confirmed.mods/devpack.zip; native scan indexes 4512 (no "Permission denied"); lookups hit ~87%; decoded-RGBA8 LRU pinned at 128 MB under the full 219 MB pack with no low-memory kill; stress run stable.Builds clean on desktop and the Android NDK toolchain. Superproject-only commits (the libultraship bump already rode in with the viewport fix on main).
🤖 Generated with Claude Code