Repository navigation
Prevent Castle Town guard-house crashes with a larger render-target pool #1202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,8 +202,10 @@ NV(nvnDeviceGetImageHandle) { RET((uint64_t)I(1)); } | |
| /* combined handle = texture id (20 bits) | sampler id << 20: the Maxwell layout, which the shaders' handle reads expect */ | ||
| NV(nvnDeviceGetTextureHandle) { | ||
| const int t = I(1), s = I(2); | ||
| if (t < 0 || t >= (1 << 20) || s < 0 || s >= (1 << 12)) die("texture handle ids %d/%d", t, s); | ||
| RET((uint64_t)t | (uint64_t)s << 20); | ||
| /* The SDK sign-extends both ids before combining them, and texture 0 always means no handle. | ||
| A missing texture can have id -1 (Castle Town's guard house): it stays the all-ones sentinel, | ||
| rather than stopping the game here. Keep the signed ids, including their upper bits. */ | ||
| RET(t ? (uint64_t)t | (uint64_t)s << 20 : 0); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Matching the SDK makes sense. One small thing: the old |
||
| } | ||
| NV(nvnDeviceGetTexelFetchHandle) { RET((uint64_t)I(1)); } | ||
| NV(nvnDeviceGetSeparateTextureHandle) { RET((uint64_t)I(1)); } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| """Exercise the production handle constructor with SDK-observed boundary values. | ||
|
|
||
| Castle Town's guard house passes texture -1 and sampler 264. The SDK returns | ||
| UINT64_MAX; the former range check stopped the game. Texture zero is special: | ||
| it returns zero even with a nonzero or negative sampler. Fixtures are authored | ||
| constants; this test contains no game code and needs no dump or GPU. | ||
| """ | ||
| import argparse | ||
| import importlib.util | ||
| from pathlib import Path | ||
| import shutil | ||
| import subprocess | ||
| import tempfile | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--cc", help="C compiler (otherwise clang, gcc or cc from PATH)") | ||
| parser.add_argument("--source", type=Path, default=ROOT / "host/nvn_native.c") | ||
| args = parser.parse_args() | ||
| spec = importlib.util.spec_from_file_location("null_pool", ROOT / "tests/test_nvn_null_pool.py") | ||
| helpers = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(helpers) | ||
| body = helpers.block(args.source.read_text(), "NV(nvnDeviceGetTextureHandle)") | ||
| fixture = r'''#include <stdint.h> | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> | ||
| typedef struct { uint64_t x[32]; } Ctx; | ||
| #define I(i) ((int)c->x[i]) | ||
| #define RET(v) (c->x[0] = (uint64_t)(v)) | ||
| #define NV(name) static void n_##name(Ctx *c, void *unused) | ||
| static void die(const char *fmt, ...) { (void)fmt; exit(86); } | ||
| ''' + body + r''' | ||
| static const struct { uint64_t texture, sampler, expected; } cases[] = { | ||
| {0xffffffff, 264, UINT64_MAX}, | ||
| {0, 264, 0}, | ||
| {0, 0xffffffff, 0}, | ||
| {1, 264, 0x10800001}, | ||
| {0xfffff, 0xfff, 0xffffffff}, | ||
| {0x100000, 264, 0x10900000}, | ||
| {0xfffffffe, 264, UINT64_MAX - 1}, | ||
| {1, 0xffffffff, UINT64_C(0xfffffffffff00001)}, | ||
| {0x80000000, 0, UINT64_C(0xffffffff80000000)}, | ||
| {1, 0x80000000, UINT64_C(0xfff8000000000001)}, | ||
| {0xfeedface00000000, 264, 0}, | ||
| {0x12345678ffffffff, 264, UINT64_MAX}, | ||
| {1, 0xfeedface00000108, 0x10800001}, | ||
| }; | ||
| int main(int argc, char **argv) { | ||
| if (argc != 2) return 2; | ||
| unsigned i = (unsigned)atoi(argv[1]); | ||
| if (i >= sizeof cases / sizeof *cases) return 2; | ||
| Ctx c, before; | ||
| memset(&c, 0xa5, sizeof c); | ||
| c.x[1] = cases[i].texture; | ||
| c.x[2] = cases[i].sampler; | ||
| before = c; | ||
| n_nvnDeviceGetTextureHandle(&c, NULL); | ||
| if (c.x[0] != cases[i].expected) { | ||
| fprintf(stderr, "case %u: got %016llx, expected %016llx\n", i, | ||
| (unsigned long long)c.x[0], (unsigned long long)cases[i].expected); | ||
| return 1; | ||
| } | ||
| if (memcmp(c.x + 1, before.x + 1, sizeof c.x - sizeof c.x[0])) return 1; | ||
| return 0; | ||
| } | ||
| ''' | ||
| compiler = shutil.which(args.cc) if args.cc else next( | ||
| (path for name in ("clang", "gcc", "cc") if (path := shutil.which(name))), None) | ||
| if not compiler: | ||
| raise SystemExit("No C compiler found (pass --cc)") | ||
| with tempfile.TemporaryDirectory(prefix="nvn-texture-handle-") as directory: | ||
| path = Path(directory) | ||
| source, binary = path / "fixture.c", path / "fixture.exe" | ||
| source.write_text(fixture) | ||
| subprocess.run([compiler, "-std=c11", "-O2", str(source), "-o", str(binary)], check=True) | ||
| failed = [] | ||
| for case in range(13): | ||
| result = subprocess.run([str(binary), str(case)], capture_output=True, text=True, timeout=5) | ||
| if result.returncode: | ||
| failed.append((case, result.returncode, result.stderr.strip())) | ||
| if failed: | ||
| raise SystemExit("FAIL: " + repr(failed)) | ||
| print("PASS: guard-house sentinel, empty handles, signed ids and 32-bit arguments (13 cases)") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| """Compile the production render-target pool sizing function without game data. | ||
| The 305 MiB default crashed in Castle Town's guard house; 512 MiB resolved the | ||
| reported conversation/camera reproduction. Keep existing 4K/ultrawide growth | ||
| and explicit HOS_RT_POOL_MB overrides, including overrides below the floor. | ||
| """ | ||
| import argparse | ||
| import importlib.util | ||
| from pathlib import Path | ||
| import shutil | ||
| import subprocess | ||
| import tempfile | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--cc", help="C compiler (otherwise clang, gcc or cc from PATH)") | ||
| parser.add_argument("--source", type=Path, default=ROOT / "host/nn_applet.c") | ||
| args = parser.parse_args() | ||
| spec = importlib.util.spec_from_file_location("null_pool", ROOT / "tests/test_nvn_null_pool.py") | ||
| helpers = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(helpers) | ||
| body = helpers.block(args.source.read_text(), "uint64_t hos_rt_pool_size(uint64_t game)") | ||
| fixture = r'''#include <stdint.h> | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> | ||
| #define MIB (UINT64_C(1) << 20) | ||
| volatile uint64_t hos_wide_out; | ||
| static unsigned output_width, output_height; | ||
| static const char *override; | ||
| static uint32_t hos_output_size(int height) { return height ? output_height : output_width; } | ||
| static void hos_log(const char *fmt, ...) { (void)fmt; } | ||
| static const char *fixture_env(const char *name) { | ||
| if (strcmp(name, "HOS_RT_POOL_MB")) exit(2); | ||
| return override; | ||
| } | ||
| #define getenv fixture_env | ||
| ''' + body + r''' | ||
| #define WIDE(w, h) ((uint64_t)(h) << 32 | (w)) | ||
| static const struct { | ||
| unsigned width, height; | ||
| uint64_t wide, game_mib; | ||
| const char *env; | ||
| uint64_t expected_mib; | ||
| } cases[] = { | ||
| {2560, 1440, 0, 305, NULL, 512}, | ||
| {3840, 2160, 0, 305, NULL, 687}, | ||
| {2560, 1440, WIDE(3440, 1440), 305, NULL, 512}, | ||
| {2560, 1440, WIDE(5120, 1440), 305, NULL, 610}, | ||
| {3840, 2160, WIDE(7680, 2160), 305, NULL, 1373}, | ||
| {2560, 1440, WIDE(1920, 1080), 305, NULL, 512}, | ||
| {2560, 1440, 0, 305, "256", 256}, | ||
| {3840, 2160, WIDE(7680, 2160), 305, "768", 768}, | ||
| {2560, 1440, 0, 305, "0x200", 512}, | ||
| {2560, 1440, 0, 305, "0", 0}, | ||
| {2560, 1440, 0, 768, NULL, 768}, | ||
| {3840, 2160, 0, 768, NULL, 1728}, | ||
| }; | ||
| int main(void) { | ||
| int failed = 0; | ||
| for (unsigned i = 0; i < sizeof cases / sizeof *cases; i++) { | ||
| output_width = cases[i].width; output_height = cases[i].height; | ||
| hos_wide_out = cases[i].wide; override = cases[i].env; | ||
| uint64_t got = hos_rt_pool_size(cases[i].game_mib * MIB); | ||
| if (got != cases[i].expected_mib * MIB) { | ||
| fprintf(stderr, "case %u: got %llu bytes, expected %llu MiB\n", i, | ||
| (unsigned long long)got, (unsigned long long)cases[i].expected_mib); | ||
| failed = 1; | ||
| } | ||
| } | ||
| return failed; | ||
| } | ||
| ''' | ||
| compiler = shutil.which(args.cc) if args.cc else next( | ||
| (path for name in ("clang", "gcc", "cc") if (path := shutil.which(name))), None) | ||
| if not compiler: | ||
| raise SystemExit("No C compiler found (pass --cc)") | ||
| with tempfile.TemporaryDirectory(prefix="rt-pool-minimum-") as directory: | ||
| path = Path(directory) | ||
| source, binary = path / "fixture.c", path / "fixture.exe" | ||
| source.write_text(fixture) | ||
| subprocess.run([compiler, "-std=c11", "-O2", str(source), "-o", str(binary)], check=True) | ||
| subprocess.run([str(binary)], check=True, timeout=5) | ||
| print("PASS: 512 MiB floor, 4K/ultrawide growth and explicit overrides (12 cases)") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to consider: the floor is applied after scaling. If the guard house needs more than 305 MB at 1440p because of render targets, it would need roughly 2.25x that at 4K, but 4K still gets 687 MB (305 scaled). Applying the floor to the base first (
game < minimum ? minimum : game, then scale) would give 4K the same relative headroom, at about +465 MB at 4K. Not tested at 4K either way, so I'm fine leaving it as is for now. It's just worth knowing if a 4K report of the same room comes in.