OoT support follow-up work

[briaguya0]

Tracking issue for follow-up work deferred out of #219 (OoT support). Grouped here to start; can be split into sub-issues as any of these get picked up.

These were held back from #219 because landing OoT support first meant matching the existing Shipwright/OTRExporter reference O2R byte-for-byte, and some of these cleanups might involve changing the generated O2R. Now that OoT support has landed, we're no longer pinned to reproducing the old zapd output — so if a cleaner approach here does change the O2R, we're free to change it (and update the ports to match) rather than being constrained to match the reference.

1. DisplayListFactory::SearchVtx — reuse Torch's VTX factory

From #219 (comment)

would using Torch's own VTX factory remove the need for [OoT::DListHelpers in DisplayListFactory::SearchVtx]?

2. Move OoT VTX off the OOT ARRAY type onto the shared VTX format

From #219 (comment)

Move from an OOT ARRAY of type VTX to the standard shared VTX format. Related to #1.

3. Reevaluate the alias system

From #219 (comment)

See if we still need the alias system, or whether having different bhash values for the same data in different paths is something we can work with instead.

4. Drop the CRC-swap logic

From #219 (comment)

See what it'd take to fully drop the "swap the CRC" logic — i.e. which ports currently rely on the swapping. Currently gated behind the ROM_CRC_BSWAP compile flag.

[briaguya0]

5. Duplicate entries in the generated O2R

Found while testing the Shipwright migration to Torch. Adding it here since it's exactly the kind of cleanup that changes the generated O2R, which is now fair game.

Torch writes some assets more than once, under the same archive path. For pal_mq:

	torch	OTRExporter reference
unique asset paths	35,352	35,352
physical zip entries	35,377	35,352
duplicated names	24	0

24 paths appear twice, except link_home_room_0Background_002480 which appears three times — 25 redundant entries in total. 23 of the 24 are cutscenes, the odd one out being that room background:

scenes/mq/ice_doukutu_scene/gIceCavernSerenadeCs
scenes/shared/ganon_tou_scene/gRainbowBridgeCs
scenes/shared/daiyousei_izumi_scene/gGreatFairyMagicCs
scenes/shared/tokinoma_scene/gTempleOfTimePreludeCs
scenes/shared/link_home_scene/link_home_room_0Background_002480   (x3)
... 19 more

They're genuine separate members — two central-directory records at different local header offsets, same CRC, and zipinfo lists the path twice:

-rw----  2540 b- defN  scenes/shared/ganon_tou_scene/gRainbowBridgeCs
-rw----  2540 b- defN  scenes/shared/ganon_tou_scene/gRainbowBridgeCs

Every copy is byte-identical to its siblings, so there's no content ambiguity — it's pure size bloat. Cost is 401,652 bytes raw / 75,839 compressed on a ~33 MB archive, so ~76 KB. Identical with USE_STANDALONE ON and OFF, and present across all versions I checked.

I haven't dug into the cause. The population being almost entirely cutscenes in shared/mq scenes suggests an asset reachable from more than one scene header getting exported once per reference, but that's a guess I haven't tested.

Why this isn't urgent — LUS handles it gracefully

Worth recording, since it means this can be cleaned up whenever rather than as a fix:

• Loading goes through zip_name_locate in O2rArchive::LoadFile, which returns a single entry index. With byte-identical copies, which one it picks is unobservable.
• Indexing at open time calls Archive::IndexFile, which does (*mHashes)[CRC64(path)] = path. A repeat insert is an idempotent overwrite of a key with the same value.
• Enumeration (ListFiles, HasFile) is derived from that same map, so it's already unique-by-name.

O2rArchive::Open is the only place LUS walks entries by raw index, and all it does there is feed IndexFile — so the duplicates collapse at the archive's front door. No double-loading, no double-counting, no duplicate list entries.

Two places it does show up

• Anything extracting to a real filesystem. unzip has to collapse two records onto one path, so it prompts replace ...? [y]es, [n]o, ...; non-interactively that's an EOF and a non-zero exit. This is how we found it — it broke an archive-comparison script mid-extract.
• Anything comparing archives keyed on path. Our parity harness hashes per archive path, so duplicate entries silently collapse into one key and the comparison reports full parity. Byte-for-byte asset parity with the reference genuinely does hold — the unique-path sets are identical and every asset matches — but a path-keyed diff structurally cannot see this. Worth knowing for anyone else validating against a reference O2R.

[briaguya0]

6. OVERRIDE_FIND_PACKAGE on zlib breaks embedders that link ZLIB::ZLIB

Another one from the Shipwright migration — not OoT-specific, but found doing OoT work.

Embedding Torch as a static library (USE_STANDALONE=OFF, BUILD_STORMLIB=OFF, BUILD_UI=OFF) breaks the host project's find_package(ZLIB). In Shipwright it failed configure in two independent places:

CMake Error at build/_deps/stormlib-src/CMakeLists.txt:335 (target_link_libraries):
  Target "storm" links to:  ZLIB::ZLIB   but the target was not found.

CMake Error at /usr/share/cmake/Modules/FindPNG.cmake:174 (set_target_properties):
  The link interface of target "PNG::PNG" contains:  ZLIB::ZLIB
  but the target was not found.

Why

CMakeLists.txt:411-429 declares zlib with OVERRIDE_FIND_PACKAGE, unconditionally on every non-Emscripten/Android platform:

FetchContent_Declare(
    zlib
    GIT_REPOSITORY https://github.com/madler/zlib.git
    GIT_TAG v1.3.1
    OVERRIDE_FIND_PACKAGE
)
FetchContent_MakeAvailable(zlib)
set(TORCH_ZLIB_TARGET zlibstatic)

OVERRIDE_FIND_PACKAGE redirects the consumer's find_package(ZLIB) too, not just Torch's own. The fetched copy defines zlib and zlibstatic but no ZLIB::ZLIB imported target, so anything that links it by name — StormLib, CMake's own FindPNG, plenty else — stops resolving.

Interestingly the MSVC path already knows this. CMakeLists.txt:382:

add_library(ZLIB::ZLIB ALIAS zlibstatic)

That alias just doesn't exist in the non-MSVC branch, which is the one embedders on Linux/macOS hit.

zlib also appears to be unused

torch/src has no direct zlib usage at all — no hits for zlib.h, inflate, deflate or compress2. As far as I can tell it's only reachable through StormLib, so with BUILD_STORMLIB=OFF it's fetched, built and linked for nothing.

Possible fixes

Either would let consumers drop the workaround:

1. Gate the fetch on BUILD_STORMLIB OR BUILD_UI, since it seems unreachable otherwise. Also saves a fetch and a build.
2. Or minimally, mirror line 382's ZLIB::ZLIB alias into the non-MSVC branch so the override at least provides what it displaced.

Workaround in the meantime is a one-liner in the host project — resolve the real zlib before Torch is declared, so the imported target already exists:

find_package(ZLIB REQUIRED)

Related, but harmless today

tinyxml2 is declared the same way (CMakeLists.txt:402-409, unconditional OVERRIDE_FIND_PACKAGE) and is likewise fetched even with BUILD_NAUDIO=OFF removing its only Torch consumers. That one causes no trouble because tinyxml2 10.0.0 defines tinyxml2::tinyxml2 as an alias, which is exactly what libultraship links, and 10.0.0 matches the version Shipwright's CI installs anyway. Mentioning it only because it's the same pattern — if the zlib fetch gets gated, tinyxml2 is probably worth the same treatment.

[briaguya0]

7. Nothing encodes a PNG into an N64 texture

Shipwright builds a second archive, soh.o2r, from the port's own assets — fonts, UI textures, presets, translations, shaders — with no ROM involved. Porting that off ZAPD turned out to be almost entirely a Torch job already:

piece	what we used
walk a directory, zip it, write portVersion	Companion::Pack (Companion.cpp:1821), which calls ParseVersionString (:2433)
0x40 resource header	BaseExporter::WriteHeader (factories/BaseFactory.cpp:53)
texture type enum, size arithmetic	TextureType / TextureUtils::CalculateTextureSize (utils/TextureUtils.h)
PNG decode	stbi_load, via the vendored lib/n64graphics/stb_image.h
PNG → N64 texture encode	had to be written

That last row is the only real gap, and it's a directional one: Torch decodes N64 texture data out of a ROM, and libultraship decodes it again for rendering, but nothing goes the other way. So we carry one file that walks RGBA pixels and emits N64 format, then wraps it with Torch's own header writer and type enum. Everything around it is yours.

We've kept it isolated in a single file for exactly that reason — if Companion::Pack (or a pack-adjacent mode) ever learns to recognise <name>.<format>.png and emit a Texture resource named <name>, that file is what moves upstream and the rest of our packer is just SoH-specific staging.

One trap worth flagging for whoever picks this up

lib/n64graphics is sitting right there and looks like the obvious thing to reach for, but its quantisation does not match ZAPD's, so using it silently changes existing assets:

/* n64graphics.c:22 */  #define SCALE_8_4(VAL_) ((VAL_) / 0x11)   /* /17 */
/* ZAPD ZTexture    */  (pixel.r >> 4) & 0xF                      /* /16 */

For r = 0xF0 that's 14 versus 15. The 3-bit path diverges the same way — SCALE_8_3 is /0x24 against ZAPD's >> 5, so 0xE0 gives 6 versus 7. We follow ZAPD's arithmetic, since these bytes have to match archives already in the wild.

Worth deciding deliberately if this lands upstream: matching ZAPD keeps existing ports byte-compatible, whereas n64graphics' rounding is arguably the more correct conversion. Same freedom this issue already notes now that OoT support has landed — just flagging that the two answers differ and the difference is invisible at runtime, since a wrong-but-plausible texture renders perfectly happily.

For reference, ours covers rgba32, rgb5a1, i4, i8, ia4, ia8 and ia16, and refuses ci4/ci8 because building a TLUT is a different problem.

[briaguya0]

8. spdlog is declared without OVERRIDE_FIND_PACKAGE, unlike tinyxml2 and zlib

Same family as #6, and it broke our macOS build outright.

Torch declares three dependencies via FetchContent. Two carry OVERRIDE_FIND_PACKAGE, one doesn't:

dependency	CMakeLists.txt	OVERRIDE_FIND_PACKAGE
tinyxml2	:402-409	yes
zlib	:421-427	yes
spdlog	:388-398	no

FetchContent_Declare is first-declaration-wins. libultraship declares spdlog with OVERRIDE_FIND_PACKAGE (cmake/dependencies/mac.cmake) precisely so its own find_package(spdlog REQUIRED) in src/CMakeLists.txt resolves afterwards. Torch has to be configured before libultraship — that's what makes the tinyxml2 override work — so Torch's spdlog declaration lands first and silently demotes libultraship's to a no-op. libultraship then fails:

CMake Error at libultraship/src/CMakeLists.txt:101 (find_package):
  Could not find a package configuration file provided by "spdlog" with any
  of the following names:

    spdlogConfig.cmake
    spdlog-config.cmake

It only surfaces where neither project finds an installed spdlog and both fall through to fetching. On our Linux and Windows CI spdlog is installed, so Torch's find_package(spdlog QUIET) succeeds and it never declares anything. On macOS it isn't — spdlog is deliberately kept out of our macports list because its fmt dependency breaks the universal build, which is the same reason libultraship builds it from source there.

Workaround

We declare spdlog ourselves, ahead of Torch, with OVERRIDE_FIND_PACKAGE: briaguya0/Shipwright@4bd6581

It keeps macOS on the same source-built v1.16.0 it used before, but it means the version is now pinned in three places (libultraship, Torch, us), and a bump in either of the other two silently loses to ours. That's the part we'd like to delete.

Suggested fix

Add OVERRIDE_FIND_PACKAGE to the spdlog declaration so it matches tinyxml2 and zlib. Then a consumer that configures Torch first still gets a find_package(spdlog) that resolves, and nobody has to pre-declare it defensively.

More generally: OVERRIDE_FIND_PACKAGE on a dependency is part of Torch's contract with whoever embeds it, since it decides whether their find_package calls keep working. Being consistent across all three is what makes embedding predictable — right now the correct integration depends on knowing which of the three got the flag.