games/NXDoom: RGB565 support, loadable-module support, and crash fixes.

Add RGB565 framebuffer support to blit_screen() - it previously assumed
a 32-bit ARGB framebuffer unconditionally, a real limitation for any
board whose framebuffer is FB_FMT_RGB16_565. A single blit loop now
branches on pinfo.bpp only at the pixel-write step (RGBTO16() for
16bpp, the existing ARGBTO32() path for 32bpp); anything else fails
loudly via i_error() rather than reading/writing past the intended
pixel bounds silently. Also centers the scaled viewport within the
framebuffer instead of pinning it to the top-left corner, and adds
CONFIG_GAMES_NXDOOM_PREFDIR to the IWAD search path (previously only
used for the config/save file location; the Kconfig entry always has a
default, so no #ifdef guard is needed around using it).

Makes GAMES_NXDOOM tristate and lets MODULE follow it
(MODULE = $(CONFIG_GAMES_NXDOOM)) instead of hardcoding MODULE = m, so
it can still be built in as before or selected as a standalone
loadable module installable via nxpkg/nxstore, same as every other
tristate-capable app in apps/.

Fix two real hardware crashes found while bringing this up as a
loadable module:

- A truncated/corrupted config line was silently overriding a
  variable's compiled-in default with an empty/unparsable value
  instead of being skipped - this let a corrupted "screenblocks" line
  through as screenblocks=0, and the renderer's view-size math divides
  by a value derived from screenblocks, reaching a divide-by-zero
  hardware exception. Also clamps screenblocks to its own valid range
  [3, 11] as an independent second layer of defense.

- r_map_plane()'s bounds check was gated behind the debug-only
  CONFIG_GAMES_NXDOOM_RANGECHECK and, when tripped, called the fatal
  i_error() - both wrong: the check guards a real out-of-bounds array
  access (observed with values far past even viewheight), so it cannot
  be optional, and killing the whole process over one glitched plane
  span is worse than vanilla DOOM's own behavior of rendering the
  glitch. Now unconditional and clamps y into range instead of
  touching memory outside the buffers' bounds, so the span still
  renders (as one glitched row) rather than leaving a gap.

Also adds CONFIG_GAMES_NXDOOM_HEAP_BUFFERS: the renderer's
visplanes/openings/drawsegs/vissprites scratch buffers remain static
arrays by default (matching vanilla DOOM), with heap allocation
available as an opt-in for targets where their combined size threatens
the internal DRAM budget once linked into a full application image.
CONFIG_GAMES_NXDOOM_STATDUMP_MAX_CAPTURES makes statdump's diagnostic
capture-buffer size (unrelated to gameplay) a Kconfig option instead of
a hardcoded value, default unchanged from vanilla (32).

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
This commit is contained in:
aviralgarg05 2026-07-16 13:34:06 +05:30
parent 094b988be3
commit 097d78156e
12 changed files with 213 additions and 18 deletions

View file

@ -4,7 +4,7 @@
#
config GAMES_NXDOOM
bool "NXDoom"
tristate "NXDoom"
default n
depends on ALLOW_GPL_COMPONENTS
depends on VIDEO_FB
@ -178,6 +178,26 @@ config GAMES_NXDOOM_MAXDRAWSEGS
memory, so you may reduce the number. However, too few will cause
rendering issues (overflow is checked to avoid crashes).
config GAMES_NXDOOM_HEAP_BUFFERS
bool "Allocate renderer scratch buffers on the heap"
default n
---help---
The visplanes/openings/drawsegs/vissprites renderer scratch buffers
(sized by the options above) are static arrays by default, matching
vanilla DOOM. On a target where their combined size threatens the
internal DRAM budget once linked into a full application image,
enable this to allocate them from the heap instead (this target's
heap may be backed by external RAM/PSRAM). Static allocation is
preferred where DRAM budget is not a concern.
config GAMES_NXDOOM_STATDUMP_MAX_CAPTURES
int "Maximum statdump capture buffer entries"
default 32
---help---
Number of playtime-statistics capture slots statdump.c reserves.
This is diagnostic/debug capture storage, not required for normal
gameplay - reduce it on a DRAM-constrained target.
config GAMES_NXDOOM_RANGECHECK
bool "Perform range checks"
default y

View file

@ -271,6 +271,16 @@ static void buld_iwad_dir_list(void)
add_iwad_dir(m_dir_name(myargv[0]));
/* Add the board's configured DOOM data directory. Kconfig documents
* CONFIG_GAMES_NXDOOM_PREFDIR as "Directory where DOOM WAD files are
* stored", but until now it was only used for the config/save file
* location -- nothing actually searched it for IWADs, forcing every
* launch to rely on the current directory or DOOMWADDIR/DOOMWADPATH
* being set by hand first.
*/
add_iwad_dir(CONFIG_GAMES_NXDOOM_PREFDIR);
/* Add DOOMWADDIR if it is in the environment */
env = getenv("DOOMWADDIR");

View file

@ -78,7 +78,11 @@ line_t *linedef;
sector_t *frontsector;
sector_t *backsector;
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
drawseg_t *drawsegs;
#else
drawseg_t drawsegs[CONFIG_GAMES_NXDOOM_MAXDRAWSEGS];
#endif
drawseg_t *ds_p;
/* newend is one past the last valid seg */

View file

@ -52,7 +52,11 @@ extern boolean markceiling;
extern boolean skymap;
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
extern drawseg_t *drawsegs;
#else
extern drawseg_t drawsegs[CONFIG_GAMES_NXDOOM_MAXDRAWSEGS];
#endif
extern drawseg_t *ds_p;
extern lighttable_t **hscalelight;

View file

@ -685,6 +685,24 @@ fixed_t r_scale_from_global_angle(angle_t visangle)
void r_set_view_size(int blocks, int detail)
{
/* screenblocks is only ever meant to hold 3..11 (set that way by the
* options menu and by the config default of 9). The renderer's view
* geometry math divides by values derived from it - notably
* pspriteiscale = FRACUNIT * SCREENWIDTH / viewwidth in
* r_execute_set_view_size() - so a 0 or otherwise out-of-range value
* turns into a divide-by-zero hardware exception (EXCCAUSE=6), which on
* this flat-memory build takes the whole board down rather than just
* this task. Clamp defensively so a bad/missing config value degrades
* to the default screen size instead of a system crash.
*/
if (blocks < 3 || blocks > 11)
{
printf("r_set_view_size: screenblocks=%d out of range, using 10\n",
blocks);
blocks = 10;
}
setsizeneeded = true;
setblocks = blocks;
setdetail = detail;

View file

@ -57,12 +57,17 @@ planefunction_t ceilingfunc;
/* Here comes the obnoxious "visplane". */
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
visplane_t *visplanes;
short *openings;
#else
visplane_t visplanes[CONFIG_GAMES_NXDOOM_MAXVISPLANES];
short openings[MAXOPENINGS];
#endif
visplane_t *lastvisplane;
visplane_t *floorplane;
visplane_t *ceilingplane;
short openings[MAXOPENINGS];
short *lastopening;
/* Clip values are the solid pixel bounding the range. floorclip starts out
@ -114,12 +119,34 @@ static void r_map_plane(int y, int x1, int x2)
fixed_t length;
unsigned index;
#ifdef CONFIG_GAMES_NXDOOM_RANGECHECK
if (x2 < x1 || x1 < 0 || x2 >= viewwidth || y > viewheight)
/* y indexes cachedheight[]/cacheddistance[]/cachedxstep[]/cachedystep[]
* below, all sized SCREENHEIGHT - a y outside that range (observed on
* this port: y=255 against a 200-entry array, well past even
* viewheight) is an out-of-bounds array write, not just a "debug
* assertion". This used to be gated behind CONFIG_GAMES_NXDOOM_
* RANGECHECK and fatal (i_error(), which tears down the whole process
* on what vanilla Doom would just render as one glitched span) - both
* wrong: the memory-safety check must not be optional, and killing the
* entire game over one bad plane span is worse than just not drawing
* it. Clamp y into range instead of touching memory outside the
* buffers' real bounds - this still renders the span (as one glitched
* row, the same "wrong but visible" failure mode vanilla DOOM has) so
* a bad plane doesn't leave a blank gap on screen either.
*/
if (x2 < x1 || x1 < 0 || x2 >= viewwidth)
{
i_error("R_MapPlane: %i, %i at %i", x1, x2, y);
return;
}
if (y < 0)
{
y = 0;
}
else if (y >= SCREENHEIGHT)
{
y = SCREENHEIGHT - 1;
}
#endif
if (planeheight != cachedheight[y])
{
@ -195,7 +222,55 @@ static void r_make_spans(int x, int t1, int b1, int t2, int b2)
void r_init_planes(void)
{
/* Doh! */
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
/* These renderer scratch buffers are sized for a comfortable margin
* above vanilla DOOM's original limits and, on a DRAM-constrained
* target, blow the internal DRAM budget as static arrays - opt-in
* heap allocation instead (comes out of the PSRAM-backed user heap
* on this target) via CONFIG_GAMES_NXDOOM_HEAP_BUFFERS.
*/
visplanes = malloc(sizeof(visplane_t) * CONFIG_GAMES_NXDOOM_MAXVISPLANES);
openings = malloc(sizeof(short) * MAXOPENINGS);
drawsegs = malloc(sizeof(drawseg_t) * CONFIG_GAMES_NXDOOM_MAXDRAWSEGS);
vissprites = malloc(sizeof(vissprite_t) *
CONFIG_GAMES_NXDOOM_MAXVISSPRITES);
if (visplanes == NULL || openings == NULL || drawsegs == NULL ||
vissprites == NULL)
{
i_error("r_init_planes: failed to allocate renderer buffers");
}
/* i_quit() can be followed by another r_init_planes() call within the
* same boot (relaunching the game via nxpkg on this flat, single
* address-space build), so these heap buffers must be freed on exit
* or every relaunch leaks the previous allocation permanently.
*/
i_at_exit(r_shutdown_planes, true);
#endif
}
/* r_shutdown_planes
* Frees the renderer scratch buffers allocated by r_init_planes. Only
* registered as an exit handler when CONFIG_GAMES_NXDOOM_HEAP_BUFFERS is
* set - the static-array buffers have nothing to free.
*/
void r_shutdown_planes(void)
{
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
free(visplanes);
free(openings);
free(drawsegs);
free(vissprites);
visplanes = NULL;
openings = NULL;
drawsegs = NULL;
vissprites = NULL;
#endif
}
/* r_clear_planes

View file

@ -58,6 +58,7 @@ extern fixed_t distscale[SCREENWIDTH];
****************************************************************************/
void r_init_planes(void);
void r_shutdown_planes(void);
void r_clear_planes(void);
void r_draw_planes(void);

View file

@ -93,7 +93,11 @@ spriteframe_t sprtemp[29];
int maxframe;
const char *spritename;
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
vissprite_t *vissprites;
#else
vissprite_t vissprites[CONFIG_GAMES_NXDOOM_MAXVISSPRITES];
#endif
vissprite_t *vissprite_p;
int newvissprite;

View file

@ -28,7 +28,11 @@
* Public Data
****************************************************************************/
#ifdef CONFIG_GAMES_NXDOOM_HEAP_BUFFERS
extern vissprite_t *vissprites;
#else
extern vissprite_t vissprites[CONFIG_GAMES_NXDOOM_MAXVISSPRITES];
#endif
extern vissprite_t *vissprite_p;
extern vissprite_t vsprsortedhead;

View file

@ -39,7 +39,7 @@
* Pre-processor Definitions
****************************************************************************/
#define MAX_CAPTURES 32
#define MAX_CAPTURES CONFIG_GAMES_NXDOOM_STATDUMP_MAX_CAPTURES
/****************************************************************************
* Private Data

View file

@ -92,6 +92,13 @@ struct graphics_state_s
uint8_t scale;
/* Pixel offset to center the scaled game viewport within the frame
* buffer when the buffer is larger than SCREENWIDTH/HEIGHT * scale.
*/
int xoffset;
int yoffset;
bool inited; /* Track initialization */
};
@ -262,13 +269,25 @@ static void blit_screen(void)
uint8_t p_idx;
void *fbptr;
/* TODO: It would be best to do this more efficiently/with less memory.
* It also would be good if we could handle the palette translation here
* such that DOOM can be played on frame buffers with differing bit depths
* and pixel formats.
*/
/* TODO: It would be best to do this more efficiently/with less memory. */
if (g_graphics_state.pinfo.bpp != 16 && g_graphics_state.pinfo.bpp != 32)
{
/* The xoffset/stride math below assumes one of those two pixel
* sizes, so continuing would read/write past the intended pixel
* bounds on the very first frame. Fail loudly instead of
* silently corrupting framebuffer memory.
*/
i_error("Unsupported framebuffer depth: %u bpp",
g_graphics_state.pinfo.bpp);
}
fbptr = g_graphics_state.fbmem +
g_graphics_state.yoffset * g_graphics_state.pinfo.stride +
g_graphics_state.xoffset *
(g_graphics_state.pinfo.bpp == 16 ? 2 : 4);
fbptr = g_graphics_state.fbmem;
for (unsigned y = 0; y < SCREENHEIGHT * g_graphics_state.scale; y++)
{
for (unsigned x = 0; x < SCREENWIDTH * g_graphics_state.scale; x++)
@ -277,9 +296,18 @@ static void blit_screen(void)
.scrnbuf[(y / g_graphics_state.scale) * SCREENWIDTH +
(x / g_graphics_state.scale)];
((uint32_t *)(fbptr))[x] =
ARGBTO32(g_palette[p_idx].a, g_palette[p_idx].r,
g_palette[p_idx].g, g_palette[p_idx].b);
if (g_graphics_state.pinfo.bpp == 16)
{
((uint16_t *)(fbptr))[x] =
RGBTO16(g_palette[p_idx].r, g_palette[p_idx].g,
g_palette[p_idx].b);
}
else
{
((uint32_t *)(fbptr))[x] =
ARGBTO32(g_palette[p_idx].a, g_palette[p_idx].r,
g_palette[p_idx].g, g_palette[p_idx].b);
}
}
fbptr += g_graphics_state.pinfo.stride;
@ -724,6 +752,18 @@ void i_init_graphics(void)
yscale = g_graphics_state.vinfo.yres / SCREENHEIGHT;
g_graphics_state.scale = xscale > yscale ? yscale : xscale;
/* Center the scaled viewport within the frame buffer rather than
* pinning it to the top-left corner, since the buffer is typically
* larger than SCREENWIDTH/HEIGHT * scale.
*/
g_graphics_state.xoffset =
(g_graphics_state.vinfo.xres -
SCREENWIDTH * g_graphics_state.scale) / 2;
g_graphics_state.yoffset =
(g_graphics_state.vinfo.yres -
SCREENHEIGHT * g_graphics_state.scale) / 2;
/* Get frame buffer plane info */
if (ioctl(g_graphics_state.fd, FBIOGET_PLANEINFO,

View file

@ -2098,7 +2098,9 @@ static void load_default_collection(default_collection_t *collection)
while (!feof(f))
{
if (fscanf(f, "%79s %99[^\n]\n", defname, strparm) != 2)
strparm[0] = '\0';
if (fscanf(f, "%79s %99[^\n]\n", defname, strparm) < 1)
{
/* This line doesn't match */
@ -2136,6 +2138,19 @@ static void load_default_collection(default_collection_t *collection)
memmove(strparm, strparm + 1, sizeof(strparm) - 1);
}
/* A line with a name but no (or an unparsable) value - e.g. a
* config file left truncated by an unclean shutdown mid-write -
* must not silently override this variable's compiled-in default
* with a bogus zero/empty value. This is what let a corrupted
* "screenblocks" line (empty value) through as screenblocks=0,
* which fed a divide-by-zero straight into the renderer.
*/
if (strparm[0] == '\0')
{
continue;
}
set_variable(def, strparm);
}