Files
virtualjaguar-libretro/test/test_framebuffer_integrity.c
Joe MattielloandClaude Fable 5 94782ac7e0 fix(libretro): advertise a max_height that bounds what we emit (#208)
* fix(libretro): advertise a max_height that bounds what we emit

retro_get_system_av_info advertised geometry.max_height = 240 for NTSC,
but the core submits 326x241 frames on test/roms/yarc.j64.  libretro
requires every frame passed to video_cb to fit inside the advertised
max_width/max_height -- frontends size their texture from those values,
so an oversized frame can be clipped or dropped outright.

Root cause, measured rather than assumed.  Probing TOM's vertical
timing registers live while yarc runs:

  VP=523 VBB=500 VBE=24 VDB=25 VDE=507
  (VDE - VDB) / 2 = 482 / 2 = 241 exactly

So this is NOT an off-by-one in our derivation: VDB/VDE are
game-programmable, the game itself opens a 241-line window, and
TOMGetVideoModeHeight() reports it faithfully (the NTSC reset defaults,
VDB=38/VDE=518, give exactly 240).  The nominal 240 active NTSC lines
is a convention, not a hardware ceiling -- VP=523 means the raster
carries 262 lines, so a taller window is physically representable, and
TOMGetVideoModeHeight() already accepts anything up to 256.

Therefore the emission is correct and the advertised bound was wrong.
max_height becomes 256 for both regions, matching that clamp, so it
bounds every height the core can produce.  The nominal size is still
carried by base_height, which is what frontends use for scaling;
max_* is only the allocation bound, so raising it costs a marginally
larger texture and nothing else.

test/test_framebuffer_integrity.c gains frame_within_advertised_geometry,
asserting the emitted frame fits the advertised maximum, and the Makefile
now runs the test for NTSC and PAL so a region-specific overflow cannot
hide.  Verified: fails against the previous behaviour (emitted 326x241 vs
max 652x240), passes after (326x241 and PAL 326x256 vs max 652x256).

The new assertion uses its own detail buffer because check() stores the
detail pointer instead of copying it, so assertions sharing one buffer
all report whatever the last one wrote.

make TEST_EXPORTS=1 test exits 0; C89 lint passed.

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix(review): address Copilot feedback on PR #208

Four review points, all valid:

- Makefile: the NTSC and PAL invocations were separated by `;`, so an
  NTSC failure followed by a passing PAL run left the block exiting 0
  and the failure masked.  Chained with && instead.  Confirmed the bug
  was real: `if ...; then false; true; fi` exits 0, `false && true`
  exits 1.
- test_framebuffer_integrity.c: resolve retro_get_system_av_info by
  direct assignment, the way harness.c:260+ resolves the retro_* entry
  points, instead of casting through `void **`.
- test_framebuffer_integrity.c: include the vendored header by its
  relative path ("../libretro-common/include/libretro.h"), matching
  every other test in test/, rather than <libretro.h> which depends on
  -I ordering and could pick up a system copy.
- test_framebuffer_integrity.c: compare dimensions in the signed
  domain via int max_w/max_h locals instead of casting w/h to unsigned,
  which removes the sign conversion from the comparison.  w/h are
  already validated > 0 above and the maxima are far below INT_MAX.

Verified: builds clean under -Wall -Wextra with no sign-conversion or
pointer warnings; the assertion still PASSes for NTSC (326x241) and PAL
(326x256) against max 652x256; make TEST_EXPORTS=1 test exits 0 with
the assertion running twice; C89 lint passed.

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix(review): convert dlsym result via memcpy (PR #208)

Neither of the previous forms is defined by ISO C: casting through
void ** (first attempt) nor plain assignment of a void * to a function
pointer (second attempt, which matched harness.c's existing style).
Copy the representation with memcpy instead, the POSIX-recommended
idiom for dlsym results, so the conversion is well-defined.

harness.c:260+ still uses direct assignment in ~12 places; that is
pre-existing and out of scope here, but the same treatment would apply
if it is ever tightened.

Verified: -Wall -Wextra clean, assertion still PASSes for NTSC
(326x241) and PAL (326x256), make TEST_EXPORTS=1 test exits 0.

Co-Authored-By: Claude Fable 5 <[email protected]>

---------

Co-authored-by: Claude Fable 5 <[email protected]>
2026-07-27 00:27:04 -04:00

410 lines
15 KiB
C

/*
* test/test_framebuffer_integrity.c — Framebuffer integrity regression test.
*
* Catches two classes of bugs that slipped through the test suite:
* 1. Alpha/X-channel corruption: AvP showed "red alpha noise" on the map
* screen because the high byte of XRGB8888 pixels contained random
* garbage instead of a consistent value.
* 2. Screen position shifts: Battle Sphere had leftVisible drift causing
* the first N columns to be all-black border when they shouldn't be.
*
* Uses the shared harness to load the core, run 120 frames with yarc.j64
* (public homebrew), then inspect the framebuffer via dlsym'd videoBuffer.
*
* Exit codes: 0 = pass, 1 = fail, 77 = skip (ROM not found)
*
* Build: cc -O2 -Wall -std=c99 $(INCFLAGS) -o test/test_framebuffer_integrity \
* test/test_framebuffer_integrity.c test/harness/harness.c \
* $(if $(filter Linux,$(shell uname -s)),-ldl) -lm
*
* Usage: ./test/test_framebuffer_integrity [core.dylib] [rom.j64]
* Default ROM: test/roms/yarc.j64
*/
#include "harness/harness.h"
#include "../libretro-common/include/libretro.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
/* Suppress LeakSanitizer for the known-benign ROM buffer leak. */
#if defined(__SANITIZE_ADDRESS__)
#define FB_TEST_HAS_ASAN 1
#elif defined(__has_feature)
#if __has_feature(address_sanitizer)
#define FB_TEST_HAS_ASAN 1
#endif
#endif
#ifdef FB_TEST_HAS_ASAN
const char *__lsan_default_suppressions(void) {
return "leak:harness_load_rom\n";
}
#endif
#define MAX_RESULTS 8
#define DEFAULT_ROM "test/roms/yarc.j64"
#define DEFAULT_FRAMES 120
/* Threshold: at least this fraction of pixels must be non-black
* after running frames (ensures rendering is actually happening). */
#define MIN_NONBLACK_FRACTION 0.01
/* Maximum number of leading all-black columns before we flag a
* screen position shift. If rendering is active, the first content
* should appear within this many columns. */
#define MAX_LEADING_BLACK_COLS 10
/* Trailing side is more lenient: the framebuffer may be wider than
* the active rendering area (e.g. 326px wide for 320px content). */
#define MAX_TRAILING_BLACK_COLS 16
/* Alpha consistency: what fraction of non-black pixels must share
* the same X-channel value for the test to pass. */
#define ALPHA_CONSISTENCY_THRESHOLD 0.99
static int pass_count = 0;
static int fail_count = 0;
static void check(int cond, const char *name, const char *detail,
harness_result *results, unsigned *num)
{
if (*num >= MAX_RESULTS) return;
if (cond) {
results[*num] = (harness_result){"PASS", name, detail};
pass_count++;
} else {
results[*num] = (harness_result){"FAIL", name, detail};
fail_count++;
}
(*num)++;
}
int main(int argc, char **argv)
{
harness_config cfg = HARNESS_CONFIG_DEFAULT;
harness_result results[MAX_RESULTS];
unsigned num_results = 0;
uint32_t **fb_ptr;
int *width_ptr;
int *height_ptr;
uint32_t *fb;
int w, h;
void (*av_info_fn)(struct retro_system_av_info *);
int max_w, max_h;
char geom_detail_buf[128];
unsigned total_pixels, nonblack_count;
unsigned i, x, y;
double nonblack_frac;
char detail_buf[256];
/* Alpha analysis variables */
unsigned alpha_00_count, alpha_ff_count, alpha_other_count;
unsigned dominant_alpha_count;
uint8_t dominant_alpha;
double alpha_consistency;
/* Screen position analysis variables */
unsigned leading_black_cols;
int col_has_content;
cfg.frames = DEFAULT_FRAMES;
if (!harness_init_from_args(&cfg, argc, argv)) {
fprintf(stderr, "Failed to load core\n");
return 2;
}
/* Default ROM if none provided */
if (!cfg.rom_path)
cfg.rom_path = DEFAULT_ROM;
/* Check if ROM exists before attempting to load */
{
FILE *f = fopen(cfg.rom_path, "rb");
if (!f) {
fprintf(stderr, "SKIP: ROM '%s' not found\n", cfg.rom_path);
harness_shutdown(&cfg);
return 77;
}
fclose(f);
}
/* Resolve videoBuffer, game_width, game_height from the core */
fb_ptr = (uint32_t **)harness_dlsym(&cfg, "videoBuffer");
width_ptr = (int *)harness_dlsym(&cfg, "game_width");
height_ptr = (int *)harness_dlsym(&cfg, "game_height");
/* Object pointer -> function pointer via memcpy: the conversion is not
* defined by ISO C in either direction (neither a plain assignment nor a
* cast through void **), so copy the representation instead. This is the
* POSIX-recommended idiom for dlsym results. */
{
void *av_info_sym = harness_dlsym(&cfg, "retro_get_system_av_info");
memcpy(&av_info_fn, &av_info_sym, sizeof(av_info_fn));
}
if (!fb_ptr || !width_ptr || !height_ptr) {
fprintf(stderr, "Cannot resolve videoBuffer/game_width/game_height\n");
harness_shutdown(&cfg);
return 2;
}
if (!harness_load_rom(&cfg)) {
fprintf(stderr, "Failed to load ROM '%s'\n", cfg.rom_path);
harness_shutdown(&cfg);
return 2;
}
/* Run frames to let the ROM render */
harness_run(&cfg);
/* Read the framebuffer state */
fb = *fb_ptr;
w = *width_ptr;
h = *height_ptr;
if (!fb || w <= 0 || h <= 0) {
fprintf(stderr, "No framebuffer available (fb=%p, %dx%d)\n",
(void *)fb, w, h);
check(0, "framebuffer_available",
"videoBuffer is NULL or dimensions are zero",
results, &num_results);
harness_report(&cfg, results, num_results);
harness_shutdown(&cfg);
return 1;
}
total_pixels = (unsigned)(w * h);
/* ================================================================
* Test 0: the emitted frame must fit inside the advertised geometry
*
* libretro requires every frame passed to video_cb to be no larger
* than the max_width/max_height reported by retro_get_system_av_info;
* frontends size their texture from those values, and an oversized
* frame gets clipped or dropped. VDB/VDE are game-programmable, so
* this is not hypothetical: yarc programs VDB=25/VDE=507 = 241 lines,
* one more than the nominal 240 NTSC active lines. Runs in whichever
* region the harness was given, so the Makefile invokes the test for
* both NTSC and PAL.
* ================================================================ */
if (av_info_fn) {
struct retro_system_av_info av;
memset(&av, 0, sizeof(av));
av_info_fn(&av);
/* Own buffer, not the shared detail_buf: check() stores the pointer
* rather than copying, so a shared buffer would report whatever the
* last assertion wrote. */
/* Compare in the signed domain: w/h are ints already validated > 0
* above, and the advertised maxima are far below INT_MAX, so this
* avoids a sign conversion in the comparison. */
max_w = (int)av.geometry.max_width;
max_h = (int)av.geometry.max_height;
snprintf(geom_detail_buf, sizeof(geom_detail_buf),
"emitted %dx%d must fit advertised max %dx%d",
w, h, max_w, max_h);
check(w <= max_w && h <= max_h,
"frame_within_advertised_geometry", geom_detail_buf,
results, &num_results);
} else {
check(0, "frame_within_advertised_geometry",
"could not resolve retro_get_system_av_info",
results, &num_results);
}
/* ================================================================
* Test 1: Basic sanity — rendering is happening
* ================================================================ */
nonblack_count = 0;
for (i = 0; i < total_pixels; i++) {
/* A pixel is "non-black" if any of the RGB channels are non-zero.
* Mask off the X/alpha channel (high byte). */
if (fb[i] & 0x00FFFFFF)
nonblack_count++;
}
nonblack_frac = (double)nonblack_count / (double)total_pixels;
snprintf(detail_buf, sizeof(detail_buf),
"%.1f%% non-black (%u/%u pixels), need >%.0f%%",
nonblack_frac * 100.0, nonblack_count, total_pixels,
MIN_NONBLACK_FRACTION * 100.0);
check(nonblack_frac > MIN_NONBLACK_FRACTION,
"rendering_active", detail_buf, results, &num_results);
/* If nothing rendered, the other tests are meaningless — skip them */
if (nonblack_count == 0) {
check(0, "alpha_integrity",
"skipped: no rendered pixels to analyze",
results, &num_results);
check(0, "screen_position",
"skipped: no rendered pixels to analyze",
results, &num_results);
harness_report(&cfg, results, num_results);
harness_shutdown(&cfg);
return fail_count > 0 ? 1 : 0;
}
/* ================================================================
* Test 2: Alpha/X-channel integrity
*
* In XRGB8888, the high byte (X) should be a consistent value
* across all non-black pixels. The core may output 0x00 or 0xFF
* (both are valid), but it must NOT output random garbage.
*
* Count how many non-black pixels have X=0x00 vs X=0xFF vs other.
* If a single value dominates (>99%), the alpha is consistent.
* ================================================================ */
alpha_00_count = 0;
alpha_ff_count = 0;
alpha_other_count = 0;
for (i = 0; i < total_pixels; i++) {
uint32_t pixel = fb[i];
uint8_t alpha;
/* Only inspect non-black pixels (black may legitimately have any X) */
if (!(pixel & 0x00FFFFFF))
continue;
alpha = (uint8_t)(pixel >> 24);
if (alpha == 0x00)
alpha_00_count++;
else if (alpha == 0xFF)
alpha_ff_count++;
else
alpha_other_count++;
}
/* Determine the dominant alpha value */
if (alpha_00_count >= alpha_ff_count) {
dominant_alpha = 0x00;
dominant_alpha_count = alpha_00_count;
} else {
dominant_alpha = 0xFF;
dominant_alpha_count = alpha_ff_count;
}
alpha_consistency = (double)dominant_alpha_count / (double)nonblack_count;
snprintf(detail_buf, sizeof(detail_buf),
"X=0x%02X: %u, X=0x%02X: %u, other: %u (%.1f%% consistent, need >%.0f%%)",
dominant_alpha, dominant_alpha_count,
(dominant_alpha == 0x00) ? 0xFF : 0x00,
(dominant_alpha == 0x00) ? alpha_ff_count : alpha_00_count,
alpha_other_count,
alpha_consistency * 100.0,
ALPHA_CONSISTENCY_THRESHOLD * 100.0);
check(alpha_consistency >= ALPHA_CONSISTENCY_THRESHOLD,
"alpha_integrity", detail_buf, results, &num_results);
/* Additional check: if there are ANY "other" alpha values (not 0x00
* or 0xFF), that's very suspicious — flag it separately. */
if (alpha_other_count > 0) {
unsigned sample_idx = 0;
uint32_t sample_pixel = 0;
/* Find a sample pixel with weird alpha for diagnostics */
for (i = 0; i < total_pixels && sample_idx == 0; i++) {
uint32_t pixel = fb[i];
uint8_t a;
if (!(pixel & 0x00FFFFFF))
continue;
a = (uint8_t)(pixel >> 24);
if (a != 0x00 && a != 0xFF) {
sample_pixel = pixel;
sample_idx = i;
}
}
snprintf(detail_buf, sizeof(detail_buf),
"%u pixels with unexpected X values (sample: pixel[%u]=0x%08X)",
alpha_other_count, sample_idx, sample_pixel);
check(alpha_other_count == 0, "alpha_no_garbage",
detail_buf, results, &num_results);
} else {
check(1, "alpha_no_garbage",
"no pixels with unexpected X-channel values",
results, &num_results);
}
/* ================================================================
* Test 3: Screen position — no excessive left-side black border
*
* If leftVisible shifted right unexpectedly, the first N columns
* will be entirely black. For yarc.j64, content should start within
* the first few columns.
* ================================================================ */
leading_black_cols = 0;
for (x = 0; x < (unsigned)w && x < MAX_LEADING_BLACK_COLS + 5; x++) {
col_has_content = 0;
for (y = 0; y < (unsigned)h; y++) {
if (fb[y * w + x] & 0x00FFFFFF) {
col_has_content = 1;
break;
}
}
if (col_has_content)
break;
leading_black_cols++;
}
snprintf(detail_buf, sizeof(detail_buf),
"%u leading black columns (max allowed: %d)",
leading_black_cols, MAX_LEADING_BLACK_COLS);
check(leading_black_cols <= MAX_LEADING_BLACK_COLS,
"screen_position", detail_buf, results, &num_results);
/* ================================================================
* Test 4: No excessive right-side black border (symmetric check)
*
* If the screen width grew without rendering filling it, the right
* side will be all-black. Check last few columns.
* ================================================================ */
{
unsigned trailing_black_cols = 0;
for (x = (unsigned)w; x > 0 && ((unsigned)w - x) < MAX_LEADING_BLACK_COLS + 5; ) {
x--;
col_has_content = 0;
for (y = 0; y < (unsigned)h; y++) {
if (fb[y * w + x] & 0x00FFFFFF) {
col_has_content = 1;
break;
}
}
if (col_has_content)
break;
trailing_black_cols++;
}
snprintf(detail_buf, sizeof(detail_buf),
"%u trailing black columns (max allowed: %d)",
trailing_black_cols, MAX_TRAILING_BLACK_COLS);
check(trailing_black_cols <= MAX_TRAILING_BLACK_COLS,
"screen_position_right", detail_buf, results, &num_results);
}
/* ================================================================
* Report
* ================================================================ */
if (!cfg.quiet) {
printf("\n Framebuffer: %dx%d, %u total pixels\n", w, h, total_pixels);
printf(" Non-black: %u (%.1f%%)\n", nonblack_count, nonblack_frac * 100.0);
printf(" Alpha: X=0x00: %u, X=0xFF: %u, other: %u\n",
alpha_00_count, alpha_ff_count, alpha_other_count);
printf(" Position: %u leading black cols, ", leading_black_cols);
printf("rendering starts at column %u\n",
leading_black_cols < (unsigned)w ? leading_black_cols : (unsigned)w);
}
harness_report(&cfg, results, num_results);
harness_shutdown(&cfg);
return fail_count > 0 ? 1 : 0;
}