Skip to content

Fix nondeterministic export output and several correctness bugs#71

Open
xmx-emm wants to merge 1 commit into
r-ex:mainfrom
xmx-emm:fix/export-determinism
Open

Fix nondeterministic export output and several correctness bugs#71
xmx-emm wants to merge 1 commit into
r-ex:mainfrom
xmx-emm:fix/export-determinism

Conversation

@xmx-emm

@xmx-emm xmx-emm commented Jul 20, 2026

Copy link
Copy Markdown

Summary

Re-exporting the same asset could produce output files that differ by a few bytes/pixels on every run. I tracked this down to several independent sources of nondeterminism, plus a few adjacent correctness bugs found along the way. All are fixed in this PR; each fix is small and local.

Nondeterministic export output

  • cpakfile.h getStarPakData(): the destination buffer was allocated uninitialized and the stream read result was never validated. Requested mip sizes are page aligned, so reads near the end of a (opt)starpak can come up short, leaving random heap bytes in the tail of the exported texture data - a few random pixels that change on every run. The buffer is now zero initialized so short reads produce deterministic zeroes.
  • rmax.cpp RMAXExporter::ToFile(): the 16 MiB scratch buffer was uninitialized, while the writer advances with IALIGN16 after every data section and rounds the final file size up to 16 bytes. The alignment gaps were written to disk as heap garbage, so every exported .rmax differed. The buffer is now zero initialized.
  • dx.cpp ExportAsPng() / ExportAsDds(): exports run on a thread pool, and two tasks can write the same output path (e.g. two materials sharing a texture with "Export Material Textures" enabled, or two selected assets sharing a dependency with "Export asset dependencies" enabled). Concurrent writers interleave within a single file and corrupt it differently each run. File writes are now serialized per output path through a small hashed mutex pool.
  • modeldata_qc.cpp: s_QCMaxVerts was a file-scope global mutated by concurrent QC exports, so a parallel export could write another model's $maxverts value into the QC file. It is now a local passed by reference through QC_ParseStudioBodypart / QC_ParseStudioLOD.

Shared texture mutation

  • dx.cpp ExportAsPng() converted the texture to B8G8R8A8 in place via ConvertToFormat, which deletes and replaces the underlying scratch image. Atlas textures (rawTxtr / convertedTxtr) are shared between preview and both export paths, so exporting PNG first silently changed what a later DDS export wrote (converted data instead of the original block-compressed format), and two threads exporting the same texture raced on the delete/replace. The conversion now happens into a temporary ScratchImage, leaving the source texture untouched.

One-pixel offset in ui image atlas slices

  • ui_image_atlas.cpp: posX/posY were truncated while width/height were rounded to nearest (+ 0.5f). When minX * atlasWidth lands just below a whole number (e.g. 99.99997), the slice origin came out one pixel off. The position now uses the same round-to-nearest as the size; this resolves the existing // should this have the same rounding as below ? todo study this note.
  • Also fixed the always-true uiImage->height == uiImage->height comparison in the preview's main-texture check (should compare against uiAsset->height).

Smaller fixes

  • ui_image.cpp: the memcpy_s destination sizes for transparent tile fills did not subtract tileOffset; the BC7 slice-pitch assert compared in the wrong direction (BC1 branch had it right).
  • texture.cpp: the mip table "Type" column sort compared a.type against b.level.
  • imgui_utility.cpp: secondary progress bar names were passed to ImGui::Text as a format string; now TextUnformatted.
  • jsonutils.h/.cpp: the copyright line contained an encoding-mangled character (U+FFFD). On systems with a non-western ANSI codepage (e.g. 936), MSVC raises C4819 which the project's warnings-as-errors turns into a build failure - main currently does not compile on such systems. Replaced with (c).

Not addressed here (worth a follow-up): RTech::DecompressStreamedBuffer passes the decompressed size as the available input byte count to OodleLZDecoder_DecodeSome, which can over-read the compressed input buffer. Fixing it requires a signature change across many call sites.

Test plan

  • Full rebuild of Release_CI succeeds (warnings-as-errors, /W4)
  • Exporting the same asset twice now produces byte-identical files (streamed textures, ui image atlas PNG/DDS, rmax)
  • DDS atlas export after PNG atlas export retains the original block-compressed format
  • Parallel "export all" with material textures + dependencies enabled produces stable output

Every re-export of the same asset could produce a file that differs by
a few bytes/pixels. Multiple independent causes, all fixed here:

- cpakfile.h getStarPakData(): the destination buffer was allocated
  uninitialized and the stream read was never validated. mip sizes are
  page aligned, so reads at the end of a (opt)starpak can come up
  short, leaving random heap bytes in the exported texture tail.
  Zero initialize the buffer so short reads produce deterministic
  zeroes.

- rmax.cpp ToFile(): the 16 MiB scratch buffer was uninitialized while
  the writer aligns every data section with IALIGN16 and rounds the
  final size up to 16, so alignment gaps full of heap garbage were
  written into every .rmax, making each export differ.

- dx.cpp ExportAsPng(): converted the texture to B8G8R8A8 in place.
  Shared textures (ui atlas rawTxtr/convertedTxtr) were permanently
  mutated, so a DDS export after a PNG export silently wrote converted
  data instead of the original format, and two export threads touching
  the same texture raced on delete/replace of the scratch image.
  Convert into a temporary image instead.

- dx.cpp ExportAsPng/ExportAsDds: exports run on a thread pool and two
  tasks can write the same output path (materials sharing textures,
  selected assets sharing dependencies), interleaving writes within one
  file. Serialize writers per output path via a small mutex pool.

- modeldata_qc.cpp: s_QCMaxVerts was a global updated by concurrent QC
  exports; a parallel export could write another model's \
  value. Now passed by reference per export.

- ui_image_atlas.cpp: posX/posY truncated while width/height rounded
  to nearest, so slices could be cut one pixel off when the float
  product lands just below a whole number (marked 'todo study this').
  Also fixed the always-true 'height == height' comparison in the
  preview texture lookup.

- ui_image.cpp: memcpy_s destination sizes for transparent tiles did
  not account for the tile offset; bc7 slice-pitch assert direction.

- texture.cpp: mip table type-column sort compared type against level.

- imgui_utility.cpp: progress bar name was passed to ImGui::Text as a
  format string.

- jsonutils.h/.cpp: replace the invalid encoding-mangled copyright
  symbol (U+FFFD) that fails to compile on non-western codepages
  (C4819 + warnings-as-errors on codepage 936).

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant