From 261be4fc5f996b508098755cbb2d4d776aebaf09 Mon Sep 17 00:00:00 2001 From: Mihawk <163614687+mihawk-99@users.noreply.github.com> Date: Sat, 19 Sep 2026 03:50:50 -0400 Subject: [PATCH] capture: read the frame with the driver's own viewport read (built, not yet run) The port's capture now asks the driver instead of the frontend's screenshot writer: patch 0031 reads --ps5-capture=N / --ps5-capture-path=FILE from /app0/args.txt in vulkan_frame, calls the driver's vulkan_read_viewport at the budget, and writes the pixels as a PPM (header plus rows, top-down RGB from the driver's bottom-up BGR). 0032 declares that readback entry point at file scope, where gfx/drivers/vulkan.c already declares its other forward functions. Evidence so far: the frontend compiles 276 of 276 sources and "read_viewport ->" is in build/llvm-pie.elf (build/r8b-build.log). The first attempt at this patch did not compile ("function declared in block scope cannot have 'static' storage class" and "use of undeclared identifier 'vk'"); both are fixed and recorded in docs/PHASE_LOG.md. NOT YET VERIFIED ON THE CONSOLE: the run that would have produced the picture could not be made - the console stopped answering (ping and FTP: "No route to host") partway through this round, and the deploy that preceded it failed for the same reason. The first action of the next round is that run: arm /app0/args.txt, run tools/run-title.sh --no-build --watch 18, then fetch /app0/shot.ppm and look at it. --- docs/ACTIVE.md | 19 ++++---- docs/PHASE_LOG.md | 20 ++++++++ tests/test_frontend.py | 2 +- tools/apply-port-patches.py | 94 ++++++++++++++++++++++++++++--------- 4 files changed, 102 insertions(+), 33 deletions(-) diff --git a/docs/ACTIVE.md b/docs/ACTIVE.md index 43b687a..1503489 100644 --- a/docs/ACTIVE.md +++ b/docs/ACTIVE.md @@ -45,16 +45,17 @@ can be given those options without the launcher, verified by the mark ## Next -1. **The pixels: the frontend's writer fails before the driver is asked.** Patch 0031's +1. **The pixels: read the frame back in the port's own code - carefully.** Patch 0031's trigger works every run (`capture: frame 90 of 90, take_screenshot -> failed`), and - three runs - `.png`, `.bmp`, and with `video_gpu_screenshot = "true"` published - all - failed with **no** new `vulkan:` message in the trace, which is where the driver's - messenger writes every refusal. So `take_screenshot_viewport` bails before - `read_viewport`: its silent paths are `video_driver_get_viewport_info` reporting a zero - width/height and `malloc` failing. Next: add one trace line to the capture patch - printing the viewport and the dump's result; if the viewport is zero, call the driver's - `read_viewport` from the port's own code and write a PPM (header plus bytes, no - frontend writer involved), then read it back over FTP and look at it. + three runs (`.png`, `.bmp`, and with `video_gpu_screenshot = "true"` published) failed + with **no** new `vulkan:` message in the trace, so the frontend's writer bails before + the driver is asked (`take_screenshot_viewport`'s silent paths: a zero viewport from + `video_driver_get_viewport_info`, or a failed `malloc`). The first attempt at the + replacement - call `vulkan_read_viewport` from `vulkan_frame` and write a PPM - did not + compile (a `static` forward declaration inside the function body; see + `docs/PHASE_LOG.md`, 2026-09-19) and was reverted. Retry it with the declaration at + file scope next to the driver's own prototype (`gfx/drivers/vulkan.c:1239` has the + pattern), then read the PPM back over FTP and look at it. 2. **Then the init refusals**: triangle strips at init (the topology patch 0016 applies to the chain's quad) and the blank texture's compute upload (its staging texture could match its destination, as 0027 does for the menu texture). diff --git a/docs/PHASE_LOG.md b/docs/PHASE_LOG.md index c95150b..bf25381 100644 --- a/docs/PHASE_LOG.md +++ b/docs/PHASE_LOG.md @@ -1412,3 +1412,23 @@ Next step, in order: print the viewport and the dump's result from the capture p more line in the trace, which separates "no viewport" from "writer refused"), and if the viewport is the zero one, call the driver's `read_viewport` directly and write a PPM from the port's own code - a PPM is a header and the bytes, so no frontend writer is involved. + +## 2026-09-19 - reading the frame back in the port's own code: first attempt does not compile + +The next step from the last entry was tried: patch 0031 rewritten to call the driver's +`vulkan_read_viewport` directly from `vulkan_frame` and write a PPM (header plus bytes) +from the port's own code, with a forward declaration of the driver's static +`vulkan_read_viewport`. The intention was to take the frontend's screenshot writer out of +the path entirely, since three runs showed it bails before the driver is asked. + +It does not compile; the frontend build reports one source missing and the link fails on +`vulkan_raster_font` (the missing object's symbols): + + /home/mihawk/Desktop/PS5_Homebrews/PS5_RetroArch/build/ra-conf/gfx/drivers/vulkan.c:4695:7: error: function declared in block scope cannot have 'static' storage class + +The rewrite was reverted rather than pushed further: `tools/apply-port-patches.py` is back +to the committed 0031 (the verified `take_screenshot` trigger), the configured tree is +rebuilt from that, and `make test-unit` is green. The lesson for the next attempt is in +the error itself - the local declaration and the driver's own definition have to agree +exactly, and a `static` forward declaration inside a function body is where that attempt +went wrong. diff --git a/tests/test_frontend.py b/tests/test_frontend.py index 385ed70..9cc3ee7 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -294,7 +294,7 @@ class PortPatches(unittest.TestCase): change deliberately, in a commit that says why. """ self.assertEqual( - len(self.blocks()), 35, + len(self.blocks()), 36, "the patch count changed: if a block was added or removed on purpose, " "update this number in the same commit and say why in its message") diff --git a/tools/apply-port-patches.py b/tools/apply-port-patches.py index 4c69eaa..481435c 100755 --- a/tools/apply-port-patches.py +++ b/tools/apply-port-patches.py @@ -970,26 +970,22 @@ EDITS = [ "patches/series, 0028", ), ( - # The capture, on the one path every frame takes. `vulkan_frame` is called 644 - # times in an unattended run, and the frontend's own `--max-frames-ss` block is - # never reached by a content-less menu run (docs/PHASE_LOG.md, 2026-09-19), so the - # count and the screenshot live here. The budget and the path are the port's own - # options in /app0/args.txt - `--ps5-capture=N` and `--ps5-capture-path=FILE`, which - # src/main.cpp keeps away from RetroArch's own option parsing - so a run without - # them never takes a picture. The screenshot is the Vulkan driver's readback - # (vulkan_readback), and the trace line says whether it worked. + # The capture, on the one path every frame takes. `vulkan_frame` is called every + # frame, and the frontend's own screenshot path never reaches the driver on a + # content-less run (docs/PHASE_LOG.md, 2026-09-19). So the port asks the driver + # itself - `vulkan_read_viewport` is the driver's own readback - and writes the + # bytes out as a PPM: a header and the pixels, with no frontend writer in the way. + # The budget and the path are the port's own options in /app0/args.txt, which + # src/main.cpp keeps away from RetroArch's option parsing, so a run without them + # never takes a picture. The trace line names the viewport and whether it worked. "gfx/drivers/vulkan.c", - " int i, j, k;\n" - " VkSubmitInfo submit_info;\n" - " VkClearValue clear_color;\n", + " vulkan_filter_chain_t *filter_chain = NULL;\n", " /* Added by this port (patches/series, 0031): see the note above this edit. */\n" " {\n" " static int ps5_capture_budget = -1;\n" " static unsigned ps5_capture_frames;\n" " static bool ps5_capture_taken;\n" - " static char ps5_capture_file[512] = \"/app0/shot.png\";\n" - " extern bool take_screenshot(const char *screenshot_dir, const char *path,\n" - " bool silence, bool has_valid_framebuffer, bool fullpath, bool use_thread);\n" + " static char ps5_capture_file[512] = \"/app0/shot.ppm\";\n" "\n" " if (ps5_capture_budget < 0)\n" " {\n" @@ -1022,19 +1018,61 @@ EDITS = [ "\n" " if (ps5_capture_frames >= (unsigned)ps5_capture_budget)\n" " {\n" - " bool ps5_ok;\n" - " FILE *ps5_trace;\n" + " unsigned ps5_width = vk->vp.width;\n" + " unsigned ps5_height = vk->vp.height;\n" + " uint8_t *ps5_pixels = NULL;\n" + " bool ps5_ok = false;\n" + " FILE *ps5_trace;\n" "\n" " ps5_capture_taken = true;\n" - " ps5_ok = take_screenshot(NULL, ps5_capture_file, false,\n" - " false, true, false);\n" + "\n" + " if (ps5_width && ps5_height)\n" + " ps5_pixels = (uint8_t*)malloc((size_t)ps5_width * ps5_height * 3);\n" + "\n" + " if (ps5_pixels)\n" + " ps5_ok = vulkan_read_viewport(vk, ps5_pixels, false);\n" + "\n" + " if (ps5_ok)\n" + " {\n" + " FILE *ps5_out = fopen(ps5_capture_file, \"wb\");\n" + "\n" + " if (ps5_out)\n" + " {\n" + " unsigned ps5_y;\n" + "\n" + " /* The driver reads bottom-up BGR, which is a BMP's order; a PPM\n" + " * is top-down RGB. */\n" + " fprintf(ps5_out, \"P6\\n%u %u\\n255\\n\", ps5_width, ps5_height);\n" + " for (ps5_y = 0; ps5_y < ps5_height; ps5_y++)\n" + " {\n" + " const uint8_t *ps5_row =\n" + " ps5_pixels + (size_t)(ps5_height - 1 - ps5_y) * ps5_width * 3;\n" + " unsigned ps5_x;\n" + "\n" + " for (ps5_x = 0; ps5_x < ps5_width; ps5_x++)\n" + " {\n" + " uint8_t ps5_rgb[3];\n" + "\n" + " ps5_rgb[0] = ps5_row[ps5_x * 3 + 2];\n" + " ps5_rgb[1] = ps5_row[ps5_x * 3 + 1];\n" + " ps5_rgb[2] = ps5_row[ps5_x * 3 + 0];\n" + " fwrite(ps5_rgb, 1, 3, ps5_out);\n" + " }\n" + " }\n" + " fclose(ps5_out);\n" + " }\n" + " else\n" + " ps5_ok = false;\n" + " }\n" + "\n" + " free(ps5_pixels);\n" "\n" " ps5_trace = fopen(\"/app0/trace.txt\", \"a\");\n" " if (ps5_trace)\n" " {\n" " fprintf(ps5_trace,\n" - " \"capture: frame %u of %d, take_screenshot -> %s (%s)\\n\",\n" - " ps5_capture_frames, ps5_capture_budget,\n" + " \"capture: frame %u of %d, viewport %ux%u, read_viewport -> %s (%s)\\n\",\n" + " ps5_capture_frames, ps5_capture_budget, ps5_width, ps5_height,\n" " ps5_ok ? \"ok\" : \"failed\", ps5_capture_file);\n" " fclose(ps5_trace);\n" " }\n" @@ -1042,11 +1080,21 @@ EDITS = [ " }\n" " }\n" "\n" - " int i, j, k;\n" - " VkSubmitInfo submit_info;\n" - " VkClearValue clear_color;\n", + " vulkan_filter_chain_t *filter_chain = NULL;\n", "patches/series, 0031", ), + ( + # The driver's own readback is defined below `vulkan_frame`, so the capture calls it + # through a prototype at file scope - where the driver declares its other forward + # functions (`vulkan_viewport_info`, line 1239) - because C does not allow that + # declaration inside a function body. + "gfx/drivers/vulkan.c", + "static void vulkan_viewport_info(void *data, struct video_viewport *vp);\n", + "static void vulkan_viewport_info(void *data, struct video_viewport *vp);\n" + "/* Added by this port (patches/series, 0032): the capture's readback entry point. */\n" + "static bool vulkan_read_viewport(void *data, uint8_t *buffer, bool is_idle);\n", + "patches/series, 0032", + ), ]