The ISO reader revert is confirmed working by the reporter, so this ships it.
Upload signing is now requested explicitly with -Parmsx3.uploadSigning rather
than used whenever keystore.properties happens to exist. Once that file was
created, every release build silently started signing with the upload key, and
an APK signed differently from the one already installed cannot be installed
over it -- so a sideload build becomes something testers cannot install, and
Android's error does not mention signatures.
It was being worked around by hiding keystore.properties by hand before each
build, which is the kind of step that gets forgotten exactly once and then
wastes a tester's evening. build-play-aab.sh passes the flag; nothing else
does, and asking for it without the keystore present is now an error rather
than a silent fallback.
The upstream merge brought a six-commit ISO series that regressed reading on
some images. A tester's ISOs that loaded on 0.9.2 now fail at the very first
header read:
ISO: init: Failed to read region information (region_count=0)
ISO: iso_archive: Corrupt ISO file: Decryption failed
Both of his games fail identically, so it is not one bad dump, and other
people's ISOs still load, so it depends on the image format -- most likely the
3k3y path, which has its own watermark detection and key extraction at 0xF70
and 0xF80, separate from the plain one.
The reader is reverted to its 0.9.2 state. The change implicated is the region
count read moving from char_arr_BE_to_uint(sec0_sec1.data()) to
read_from_ptr<be_t<u32>>(sec0_sec1), alongside decrypt_data moving from raw
pointers to std::span, but this is not a targeted fix: it restores code that
demonstrably worked rather than guessing which line of the refactor is wrong.
is_valid() is kept, because System.cpp calls it and it is one line that has
nothing to do with the failure.
What this gives up is real: the series contained out-of-bounds write and
overflow hardening for ISO parsing. A reader that cannot open a user's games
is the worse of the two, and the hardening should come back with a version of
the refactor that works. The path validation from the same series that lives
outside these two files is untouched.
Unverified against the actual failure -- the ISOs here all load either way, so
this could not be reproduced locally. It is verified not to break what already
worked.
Builds were reporting whichever commit cmake last configured against, not the
one being built. rpcs3/git-version.h is generated at CONFIGURE time, and
build-variants.sh deliberately skips reconfiguring an already-correct build
dir because re-running cmake regenerates LLVM's headers and costs a full
rebuild. So the stamp froze, and every build after that lied about itself.
The gap was wide. HEAD is 20120-da26a455; the header on disk still said
19985-91952ae4, a commit from before 0.9.2.
This is not cosmetic. A tester running 0.9.3 reported that older commit in
their log, which sent an investigation hunting for a regression among upstream
ISO changes their build did not contain, and very nearly had a fix built and
sent for a version they were not running. A build that misreports itself makes
every report from it ambiguous.
stamp-git-version.sh writes the header from HEAD and is called by both build
entry points before anything compiles. It only writes when the contents differ,
so an unchanged HEAD does not force a rebuild, and ninja rebuilds just the
translation units that include it.
The script bundled whatever core happened to be sitting in jniLibs, and
build-variants.sh overwrites that path once per variant, so the file left there
is simply whichever ran last. Running the variants and then rebuilding the
bundle would have shipped an armv8.2 core inside a minSdk 30 bundle:
installable on devices that cannot execute it, failing at dlopen, with nothing
in the failure to point at the cause.
Play serves one bundle to every device, so the ISA floor has to be the lowest
ARMSX3 supports. The legacy core is now stripped into place by this script
every time, and it refuses to run if that core has not been built.
The bundle already shipped was correct -- the legacy core was staged by hand
before it was built -- but only by hand, which is the part worth removing.
The bundle verifier reported libarmsx3-core.so missing from a bundle that
plainly contained it, at base/lib/arm64-v8a/libarmsx3-core.so.
Under `set -o pipefail`, `unzip -l "$AAB" | grep -q x` fails whenever x IS
present: grep exits at the first match, closes the pipe, and unzip takes
SIGPIPE, so the pipeline reports failure. Every check for something that must
be there was inverted, and every check for something that must be absent passed
for the wrong reason -- grep found nothing, read to the end, and no signal was
raised.
Capturing the listing first and piping printf into grep instead only moved
which process took the signal. The listing is matched with `case` now, which
has no subprocess to kill.
Worth stating plainly: this was a verifier that would have reported a clean
bundle whatever went wrong, which is the failure mode the script exists to
prevent. It only surfaced because the one check that should have passed was the
one that failed.
Checking that keystore.properties exists was not enough. A template with the
placeholders still in it passed the check and went on to build, failing much
later inside signing with an error that says nothing about the cause. It now
rejects the placeholders too, so the message arrives before the fifteen minutes
rather than after them.
Also locate a JDK. The script assumed the calling shell had one on PATH, and
without it gradle fails with "Unable to locate a Java Runtime" several steps in,
which points at Java rather than at the environment. Android Studio's bundled
JDK is used when JAVA_HOME is unset, with /usr/libexec/java_home as a fallback
and a clear failure if neither exists.
Two things stood between the flavor split and an uploadable bundle.
The release build type was debug-signed, with a comment saying to swap it
before any public build. It now uses an "upload" signing config when
keystore.properties exists and falls back to the debug key when it does not, so
GitHub alphas stay sideloadable without the key present while Play gets a
properly signed bundle. The keystore and the properties file are both already
gitignored, and nothing reads or echoes them outside the build.
build-play-aab.sh refuses to start without that file and prints the keytool
command to create one, rather than spending fifteen minutes producing a bundle
Play will reject.
Minify is off for the Play bundle and on everywhere else. This is not a
preference: AGP 9.2.1's R8 writes its mapping as mapping.prt, a compressed
per-class archive, while packageBundle still requires a plain mapping.txt, so
an AAB cannot be built with R8 enabled at all. ARMSX2 ships its Play build with
minify off entirely, so this matches existing practice rather than introducing
a compromise, and a 94 MB native core dominates a 76 MB APK -- shrinking the
Kotlin was never where the size is.
Driven by a gradle property, matching how armsx3.minSdk is already threaded
through by build-variants.sh, rather than by the variant API.
Verified: the github release APK still builds at 76.5 MB with R8 running.
An AAB is a ZIP and its entries are compressed, so grepping the archive finds
nothing and reports a clean bundle whatever is inside it. The first version of
this script did exactly that. It passed all five exclusion checks and then
failed to find the applicationId it was supposed to find, which is the only
reason the method got questioned at all -- every check that mattered was a
false pass.
It extracts the bundle now and reads base/manifest/AndroidManifest.xml for the
permissions, and the archive listing for the native libraries. Verified against
a real bundle: com.armsx3.play present, REQUEST_INSTALL_PACKAGES,
MANAGE_EXTERNAL_STORAGE, RECORD_AUDIO and the update provider all absent, and
libarmsx3_lsfg.so not among the shipped .so files.
The manifest is protobuf rather than text, so LC_ALL=C and grep -a stay: the
NUL bytes in it would otherwise make grep declare the file binary and print
nothing, which reads the same as a pass.
Play will not take the build ARMSX3 ships today, and the two runtime flags that
were already here could never have made it acceptable. The manifest said so:
it is the declared PERMISSION in the bundle that gets rejected, not the code
path behind a boolean. So the split is by source set, which is the only thing
that actually removes anything.
github keeps what a sideloaded build should have: the in-app updater, all-files
storage for an arbitrary data folder, and frame generation. play has none of
them, and its applicationId is com.armsx3.play so the two install side by side
instead of over each other.
Moved rather than flagged: com.armsx2.update and its FileProvider resource into
src/github, with no-op composables in src/play so that source set still
compiles. REQUEST_INSTALL_PACKAGES and MANAGE_EXTERNAL_STORAGE now live in the
github manifest, so the play bundle never declares them.
Frame generation is excluded by moving libarmsx3_lsfg.so into
src/github/jniLibs. A packaging block inside a productFlavor is NOT honoured --
it applied to both and dropped the library from the github build too, which the
build caught. Excluding the file is the whole exclusion: the shim is dlopen'd
by name and the core already reports the feature unavailable when it is absent,
which is the same path a device that cannot run it takes.
build-play-aab.sh builds the bundle and refuses to emit one that fails any of
five checks, because a flavor split is only worth as much as the thing that
notices when it stops working. It greps the bundle with LC_ALL=C and -a on
purpose: a NUL byte makes plain grep declare the file binary and say nothing,
which reads exactly like a pass.
build-variants.sh follows the rename to assembleGithubRelease; there is no
flavorless release variant any more.
Both failures in the savedata fixup were logged as fatal while the code
carried straight on -- the loop continues, the emulator starts, and one save
is left as it was. On Android a fatal line goes to logcat at ANDROID_LOG_FATAL,
so it reads like a crash, and it was reported as one.
It is also permanent rather than transient when it happens. Android storage can
refuse a directory rename outright with EPERM, and nothing about launching
again changes that, so it fired on every single launch and looked like a fault
that was getting worse. A user hitting it has no way to tell that their games
are unaffected.
Both are errors now, and both say what the consequence is and what to delete if
it keeps happening, rather than repeating an alarm about something the emulator
has already decided to continue past.
Continuing with the modules that did compile is correct -- the dispatcher
entry for an uncompiled function interprets, nothing runs garbage -- but it is
per-instruction dispatch and it is slower than the interpreter outright. Saint
Seiya measured 6fps against 23 with most of its modules missing.
A game at that speed looks broken, and it gets reported as broken, when the
real answer is one restart away. An honest stop is better than a degraded run
that invites the wrong bug report.
So this ends the boot the way running out of memory always did. What is
different from before is the reason: the message names it and says what to do,
and it stays on screen because the overlay is drawn by the RSX thread rather
than the one this stops.
The fallback itself stays in place for the ordinary case of a single module
failing for some other reason, which is upstream's design and is worth keeping
-- one bad module costing its own functions is a fair trade. Running out of
memory is not that case: it takes most of the executable with it.
Both changes that were brought back are out again. Neither recovered the frame
rate after switching frame generation off, and both broke rendering.
That is every attempt at this reverted. What remains is the code as it stood
before any of it: frame generation is slow, and switching it off does not
release what it allocated.
The GLES DSA shim is kept, because it belongs to the upstream merge and not to
this work -- it landed in the wrong commit and reverting that commit took it
with it, which broke the build for a reason unrelated to frame generation.
The findings from the three audits are still correct as descriptions of what
the code does. Acting on them, one at a time and with the reasoning written
down each time, still made the result worse on every attempt. So the fault is
not in the individual fixes but in something about this subsystem that reading
it has not revealed, and the next attempt should start from a measurement on
the device rather than from another reading of the source.
This is the change that stopped turning frame generation off leaving the
frame rate tanked -- confirmed by which build that was reported on, and by the
SUBOPTIMAL acquire fix not reproducing it on its own.
It replaces the per-frame vkDeviceWaitIdle on framegen's device with a poll on
a sync fd exported from a fence framegen already creates. Reasons it is safe
to bring back while the other three stay reverted: it only runs when frame
generation is producing frames, every failure path falls back to the device
wait, and it changes no layout, no ownership and no barrier -- which is where
the rendering damage came from.
Still out, and staying out: the cross-device ownership barrier, the
generated-frame acquire semaphore with its teardown, and the deferred-present
guard in frame_context_cleanup.
Swept up in last night's blanket revert of the frame-generation work, and it
should not have been. It is the one change of the five that cannot affect
rendering: it corrects error handling and touches nothing about layouts,
ownership or synchronisation.
VK_SUBOPTIMAL_KHR is a success code and the image IS acquired.
present_generated_frame treated it as a failure and returned without
presenting, so the image was never handed back -- one swapchain image lost per
generated frame, permanently, on a driver that reports SUBOPTIMAL as a
standing condition. Once the pool empties the real frame's acquire blocks its
whole 100ms timeout every frame, which is ten frames a second, and only a
swapchain rebuild recovers it. That is why turning frame generation off never
gave the frame rate back.
The generated present's result is handled for the same reason: OUT_OF_DATE and
SURFACE_LOST also leave the image unpresented, and none of the recovery flags
were being raised because generated frames never pass through present().
The other four stay reverted. This one is restored on its own so that if
anything is still wrong, the cause is a single small change rather than five
entangled ones.
Catches up 98 commits from RPCS3/rpcs3. Seven files conflicted:
PPUTranslator.cpp -- upstream fixed the ARM64 float-to-int saturation
inversion independently, and the two fixes are the same fix. Took upstream's
ordering so it stops re-conflicting, kept one line of the reasoning.
rpcn_types.h, rpcn_client.cpp -- our hand-rolled protocol 31 bump was a
stopgap to stop the server refusing us. Upstream's trophy sync (cb175278b) is
the real implementation, so it replaces ours outright.
nv0039.cpp -- upstream refactored the strided copy and extracted
validate_buffer_notify; ours was the older code plus a profiler include. Took
upstream's, re-added the include.
VKQueryPool.cpp -- both sides had real changes. Upstream added a lock around
the pool cache; ours has the bounded occlusion-query wait and the render-pass
fix for the Adreno device loss. Kept ours, re-applied their lock.
VKResourceManager.cpp -- upstream moved GC completion onto a
driver_manager_thread, which supersedes our flush parameter and offloader
dispatch entirely. Took theirs; no callers passed the second argument.
VKGSRender.cpp -- our flush-site counter and their driver-manager drain are
independent. Both kept.
Upstream also began attaching 3D and array levels through DSA, which
EXT_direct_state_access has no NamedFramebufferTextureLayer for, so the GLES
shim gains one that binds and uses the non-DSA entry point.
Reverted in the same commit, because they are what is on the tester's device
and it is broken: the sync-fd handoff, the SUBOPTIMAL acquire fix, the
cross-device ownership barrier, the generated-frame acquire semaphore, and the
deferred-present guard in frame_context_cleanup. Each was defensible on its
own reading of the code and the result was worse every time -- slower, judder
described as nauseating, and rendering faults that outlast switching frame
generation off. Five attempts is enough to stop treating the next theory as
better than the last.
What stays fixed is everything outside the present path. What goes back is the
state before tonight: frame generation is slow and does not release its memory
when switched off.
Two things frame generation was getting away with rather than doing.
The acquire for a generated frame passed VK_NULL_HANDLE for both the semaphore
and the fence. The specification forbids that outright, and the practical
consequence is that nothing made the blit wait for the presentation engine to
finish with the image -- the UNDEFINED old layout on the barrier discards
contents, it does not order anything, so the blit could overwrite an image
still being scanned out. It acquires with a semaphore now and the submit waits
on it.
A ring of eight rather than a single semaphore, because a binary semaphore may
not be waited again before it has been signalled again and up to three
generated frames are acquired per flip. Eight is more than two flips of slack,
so a slot is never revisited before the submit that waits on it has retired.
The other is a crash rather than a hazard. Frame generation's images are file
-scope globals created on the renderer's device, and nothing released them, so
stopping a game and starting another at the same resolution skipped the
rebuild -- capture_presented_frame only rebuilds when the dimensions change --
and recorded images belonging to a destroyed device into a live command
buffer. They are released in ~VKGSRender now, after the vkDeviceWaitIdle it
already does and before the swapchain goes.
That location is the point. An earlier attempt at the same teardown ran from
inside the capture hook, which executes while the frame's primary command
buffer is still being built, and it destroyed rendering outright. A
device-idle point the renderer owns is where this always belonged.
The blit that puts a generated frame on screen named
VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL for an image that was never in that
layout, and read it without ever taking ownership from the device that wrote
it. framegen releases its outputs with newLayout = VK_IMAGE_LAYOUT_GENERAL and
dstQueueFamilyIndex = VK_QUEUE_FAMILY_EXTERNAL, so this was a layout mismatch
and a missing queue-family acquire at the same time.
Undefined by the specification either way, and on a tiler the practical result
is reading whatever survived in cache instead of what framegen wrote -- which
is the corrupted rendering that appears whenever frame generation is switched
on.
The image is acquired from VK_QUEUE_FAMILY_EXTERNAL into the graphics family
and transitioned GENERAL -> TRANSFER_SRC_OPTIMAL before the blit, then
released back the same way afterwards, because framegen's own acquire barrier
on the next generation assumes it finds the image exactly as it left it.
This is about visibility and ownership, not timing: generate() already does
not return until framegen's submission has completed.
present_generated_frame treated VK_SUBOPTIMAL_KHR as a failure. It is a
success code and the image IS acquired, so the early return left it held and
never presented -- one swapchain image gone per generated frame, permanently.
This driver reports SUBOPTIMAL as a standing condition rather than a one-off,
which present() already accounts for a few lines below, so the pool emptied
within a few frames of turning frame generation on.
Once it is empty the real frame's acquire in flip() blocks its full 100ms
timeout every frame. 100ms is ten frames a second, which is the hard lock that
was reported, and it explains the part that made no sense: turning frame
generation back off changed nothing, because the images are gone from the
swapchain and only a rebuild -- a resize or a restart -- brings them back.
The present result was discarded for the same frames. OUT_OF_DATE and
SURFACE_LOST also leave the image unpresented, and none of the recovery flags
were raised, because generated frames never pass through present() where that
handling lives. Both are now routed the same way.
Also submit with flush before presenting. The present runs on this thread
while multithreaded RSX would still have the submit queued on the offloader,
which presents a swapchain image before the blit that fills it.
Every generated frame ended with vkDeviceWaitIdle on framegen's device. It is
the hardest sync primitive Vulkan has and it ran once per frame, which is a
large part of why frame generation cost more than it returned.
It was there because the two devices share no semaphore. The upstream
semaphore path cannot supply one on Android: framegen's device deliberately
does not enable VK_KHR_external_semaphore_fd -- it shares through
AHardwareBuffer instead -- so vkImportSemaphoreFdKHR resolves to nullptr there
and every semaphore handed across is -1. That is not a flag that can be
flipped.
A fence can be exported where a semaphore cannot. framegen already creates one
per generation pass, so this adds VK_KHR_external_fence_fd, exports a sync fd
after the passes are submitted, and hands it back through a new
armsx3_lsfg_present_fenced. Our side polls that fd instead of idling a device.
Three things this deliberately does not do:
The existing completion fence is not the one exported. SYNC_FD export has copy
transference, which resets the source fence, so exporting it would leave the
slot-reuse wait at the top of present() blocking on a fence with no pending
signal -- a hang traded for a stall. A dedicated fence signalled by a
zero-batch submit is used instead, which signals once all previously submitted
queue work completes.
The extension is probed, not required. framegen's required-extension list
throws on a miss, so a driver without it would fail vkCreateDevice and lose
frame generation altogether -- a regression rather than a degradation.
The device wait stays as the fallback. A null entry point, a failed export, a
poll timeout or a poll error all fall back to it, and the warning for that is
emitted once per session rather than once per frame.
armsx3_lsfg_present keeps its signature and forwards to the new path with a
null fd, so the two bodies cannot drift. ABI goes to 3; the version check is
left exact rather than loosened.
The serialised path submits the frame, then blocks on that whole submission
before framegen may read the capture, then blocks again on framegen's device.
Two full stalls per frame, one of them a complete frame of GPU work on the
critical path. That is the reason frame generation costs more than it gives
back on this hardware.
Pipelining exists to remove the first of those: it runs a frame behind and
interpolates the pair ending at the PREVIOUS frame, which the game has had a
whole frame to finish, so the wait is normally already satisfied when it is
reached.
It was disabled because the held-back frame could be recycled underneath it.
That is fixed at the choke point in the commit before this one, so this is
the switch and nothing else.
Kept as its own commit deliberately: if the pipelined path misbehaves on
device, reverting this alone returns to the serialised path with the reclaim
fix still in place.
Frame generation's pipelined path holds one frame back by a present, and the
guard that stopped that frame being recycled underneath it lived in
advance_queued_frames. There are two functions that walk m_queued_frames and
call frame_context_cleanup, and that was one of them.
check_present_status() is the other. It had no check at all, and
flush_command_queue calls it unconditionally on the way out -- so any of the
twenty or so flushes in a frame could retire the held-back frame, and
flush_command_queue(true) drains the queue outright, which made it certain
rather than likely. flip()'s own acquire-retry spin calls it too, exactly when
swapchain images are scarce, which is the condition pipelining creates by
design. That is the "already retired" that fired twice on device and kept
pipelining switched off.
The guard moves into frame_context_cleanup itself, which is the single point
every reclaim path goes through, so a seventh path added later cannot miss it.
Also stop a dropped present leaving a stale present_image behind. Leaving it
set is not harmless: the slot rotates back around and trips
ensure(present_image == umax) in flip(), and reinitialize_swapchain selects
contexts by that field and then calls frame_context_cleanup, whose first line
asserts a command buffer the context no longer has. Dropping a present should
cost a frame on screen, not a fatal two frames later.
No behaviour change on its own: nothing sets m_deferred_present_frame while
pipelining is disabled.
Releasing framegen's resources when the setting went to off broke rendering.
Arkham City lost its character models, then almost everything stopped drawing.
Both halves are reverted: the release_shared_images() call and the shutdown()
that followed it. Adding a vkDeviceWaitIdle on our device before the release
did not fix it, which rules out the in-flight capture blit being the whole
story -- the damage reaches past the shared images, and I do not yet know how
far. Guessing again on a renderer that draws nothing is not worth the trade.
What the revert restores is the known-good behaviour and the known bug with
it: switching frame generation off keeps every byte it allocated, so the frame
rate does not recover until the game is restarted. That is worse than it
should be and better than a black screen.
The teardown itself is still the right thing to do. It has to happen somewhere
the present path is not mid-flight -- a device-idle point owned by the
renderer -- rather than from inside the capture hook, which runs while the
frame's primary command buffer is still being built and submitted.
Shader compilation "dips" are not the renderer waiting on a pipeline. Both
async shader modes are built so nothing ever stalls for one -- the interpreter
draws it or the draw is dropped. The dip is the compile work itself competing
for cores the emulator is already short of.
The pipeline compiler workers never set an affinity at all. PPU, SPU and RSX
all do, and the big.LITTLE policy deliberately fences the SPUs off the prime
core so RSX can have it -- and then a shader burst was free to land on exactly
that core anyway. They are pinned to thread_class::general now, which is the
existing policy for helper threads and already resolves to the little cluster
on big.LITTLE and to every core on a uniform machine, so desktop is unchanged.
The pool was also sized to the wrong thing. The auto heuristic counts host
threads, so an 8-thread phone asked for 2 workers -- a number chosen for cores
those workers are now not allowed on, while its 3 little cores sat idle
through every burst. It sizes to the cluster it is pinned to instead, which
raises compile throughput without taking anything back from emulation: the
cores it adds are the ones nothing else wanted.
Not touched: the driver's compiler itself, which is Qualcomm's and is the
actual cost. This only stops it fighting the emulator for the same cores.
jpolo1224