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

aviralgarg05 · aviralgarg05 · commit 6fe600843764 · 2026-07-16T13:34:06.000+05:30
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. Branches on pinfo.bpp:
16bpp uses RGBTO16(), 32bpp keeps the existing ARGBTO32() path, and
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). Builds as a
standalone loadable module (MODULE = m) unconditionally so it can be
installed/launched via nxpkg/nxstore.

Fix three 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
  write (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, checks against the true array bound
  (SCREENHEIGHT), and skips the draw instead of touching memory or
  aborting. Also converts visplanes/openings/drawsegs/vissprites from
  static arrays to heap allocation (freed via i_at_exit so cleanup also
  runs on the i_error() fatal path) to relieve DRAM pressure once a
  full application is linked into the same image.

- myargv was under-allocated: sized for argc pointers, but this
  codebase's own argument handling assumes argv is NULL-terminated at
  argv[argc] (standard C/exec convention). Root-caused via a real crash
  dump and symbol resolution against the built ELF; now allocates
  argc + 1 pointers and explicitly NULL-terminates.

Signed-off-by: aviralgarg05 &lt;gargaviral99@gmail.com&gt;
diff --git a/games/NXDoom/Makefile b/games/NXDoom/Makefile
@@ -2,7 +2,7 @@ include $(APPDIR)/Make.defs
 
 # Program options
 
-MODULE    = $(CONFIG_GAMES_NXDOOM)
+MODULE    = m
 PRIORITY  = $(CONFIG_GAMES_NXDOOM_PRIORITY)
 STACKSIZE = $(CONFIG_GAMES_NXDOOM_STACKSIZE)
 PROGNAME  = nxdoom
diff --git a/games/NXDoom/src/d_iwad.c b/games/NXDoom/src/d_iwad.c
@@ -271,6 +271,18 @@ 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.
+   */
+
+#ifdef CONFIG_GAMES_NXDOOM_PREFDIR
+  add_iwad_dir(CONFIG_GAMES_NXDOOM_PREFDIR);
+#endif
+
   /* Add DOOMWADDIR if it is in the environment */
 
   env = getenv("DOOMWADDIR");
diff --git a/games/NXDoom/src/doom/r_bsp.c b/games/NXDoom/src/doom/r_bsp.c
@@ -78,7 +78,7 @@ line_t *linedef;
 sector_t *frontsector;
 sector_t *backsector;
 
-drawseg_t drawsegs[CONFIG_GAMES_NXDOOM_MAXDRAWSEGS];
+drawseg_t *drawsegs;
 drawseg_t *ds_p;
 
 /* newend is one past the last valid seg */
diff --git a/games/NXDoom/src/doom/r_bsp.h b/games/NXDoom/src/doom/r_bsp.h
@@ -52,7 +52,7 @@ extern boolean markceiling;
 
 extern boolean skymap;
 
-extern drawseg_t drawsegs[CONFIG_GAMES_NXDOOM_MAXDRAWSEGS];
+extern drawseg_t *drawsegs;
 extern drawseg_t *ds_p;
 
 extern lighttable_t **hscalelight;
diff --git a/games/NXDoom/src/doom/r_main.c b/games/NXDoom/src/doom/r_main.c
@@ -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;
diff --git a/games/NXDoom/src/doom/r_plane.c b/games/NXDoom/src/doom/r_plane.c
@@ -57,12 +57,12 @@ planefunction_t ceilingfunc;
 
 /* Here comes the obnoxious "visplane". */
 
-visplane_t visplanes[CONFIG_GAMES_NXDOOM_MAXVISPLANES];
+visplane_t *visplanes;
 visplane_t *lastvisplane;
 visplane_t *floorplane;
 visplane_t *ceilingplane;
 
-short openings[MAXOPENINGS];
+short *openings;
 short *lastopening;
 
 /* Clip values are the solid pixel bounding the range. floorclip starts out
@@ -114,12 +114,23 @@ 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.  Skip the draw instead of touching memory or the process outside
+   * the buffers' real bounds.
+   */
+
+  if (x2 < x1 || x1 < 0 || x2 >= viewwidth || y < 0 || y >= SCREENHEIGHT)
     {
-      i_error("R_MapPlane: %i, %i at %i", x1, x2, y);
+      return;
     }
-#endif
 
   if (planeheight != cachedheight[y])
     {
@@ -195,7 +206,48 @@ static void r_make_spans(int x, int t1, int b1, int t2, int b2)
 
 void r_init_planes(void)
 {
-  /* Doh! */
+  /* These renderer scratch buffers are sized for a comfortable margin
+   * above vanilla DOOM's original limits and would blow the platform's
+   * internal DRAM budget as static arrays, so they're heap-allocated
+   * instead (comes out of the PSRAM-backed user heap on this target).
+   */
+
+  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);
+}
+
+/* r_shutdown_planes
+ * Frees the renderer scratch buffers allocated by r_init_planes.
+ */
+
+void r_shutdown_planes(void)
+{
+  free(visplanes);
+  free(openings);
+  free(drawsegs);
+  free(vissprites);
+
+  visplanes = NULL;
+  openings = NULL;
+  drawsegs = NULL;
+  vissprites = NULL;
 }
 
 /* r_clear_planes
diff --git a/games/NXDoom/src/doom/r_plane.h b/games/NXDoom/src/doom/r_plane.h
@@ -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);
diff --git a/games/NXDoom/src/doom/r_things.c b/games/NXDoom/src/doom/r_things.c
@@ -93,7 +93,7 @@ spriteframe_t sprtemp[29];
 int maxframe;
 const char *spritename;
 
-vissprite_t vissprites[CONFIG_GAMES_NXDOOM_MAXVISSPRITES];
+vissprite_t *vissprites;
 vissprite_t *vissprite_p;
 int newvissprite;
 
diff --git a/games/NXDoom/src/doom/r_things.h b/games/NXDoom/src/doom/r_things.h
@@ -28,7 +28,7 @@
  * Public Data
  ****************************************************************************/
 
-extern vissprite_t vissprites[CONFIG_GAMES_NXDOOM_MAXVISSPRITES];
+extern vissprite_t *vissprites;
 extern vissprite_t *vissprite_p;
 extern vissprite_t vsprsortedhead;
 
diff --git a/games/NXDoom/src/doom/statdump.c b/games/NXDoom/src/doom/statdump.c
@@ -39,7 +39,7 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#define MAX_CAPTURES 32
+#define MAX_CAPTURES 4
 
 /****************************************************************************
  * Private Data
diff --git a/games/NXDoom/src/i_main.c b/games/NXDoom/src/i_main.c
@@ -57,17 +57,28 @@ void d_doom_main(void);
 
 int main(int argc, char **argv)
 {
-  /* save arguments */
+  /* save arguments
+   *
+   * +1 and an explicit NULL terminator: argv is conventionally
+   * NULL-terminated at argv[argc] (this is what the OS/exec path
+   * guarantees for the `argv` parameter above), and some of this
+   * codebase's own argument handling was written assuming that holds
+   * for myargv too - an under-sized allocation here leaves myargv[argc]
+   * pointing at whatever the allocator happens to return next, which
+   * only reads as "probably zero" by chance depending on heap layout.
+   */
 
   myargc = argc;
-  myargv = malloc(argc * sizeof(char *));
+  myargv = malloc((argc + 1) * sizeof(char *));
   assert(myargv != NULL);
 
   for (int i = 0; i < argc; i++)
     {
       myargv[i] = m_string_duplicate(argv[i]);
     }
 
+  myargv[argc] = NULL;
+
   /* Print the program version and exit. */
 
   if (m_parm_exists("-version") || m_parm_exists("--version"))
diff --git a/games/NXDoom/src/i_video.c b/games/NXDoom/src/i_video.c
@@ -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,27 +269,61 @@ 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. */
 
-  fbptr = g_graphics_state.fbmem;
-  for (unsigned y = 0; y < SCREENHEIGHT * g_graphics_state.scale; y++)
+  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);
+
+  if (g_graphics_state.pinfo.bpp == 16)
     {
-      for (unsigned x = 0; x < SCREENWIDTH * g_graphics_state.scale; x++)
+      for (unsigned y = 0; y < SCREENHEIGHT * g_graphics_state.scale; y++)
         {
-          p_idx = g_graphics_state
-                      .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);
+          for (unsigned x = 0; x < SCREENWIDTH * g_graphics_state.scale; x++)
+            {
+              p_idx = g_graphics_state
+                          .scrnbuf[(y / g_graphics_state.scale) *
+                                       SCREENWIDTH +
+                                   (x / g_graphics_state.scale)];
+
+              ((uint16_t *)(fbptr))[x] =
+                  RGBTO16(g_palette[p_idx].r, g_palette[p_idx].g,
+                          g_palette[p_idx].b);
+            }
+
+          fbptr += g_graphics_state.pinfo.stride;
+        }
+    }
+  else if (g_graphics_state.pinfo.bpp == 32)
+    {
+      for (unsigned y = 0; y < SCREENHEIGHT * g_graphics_state.scale; y++)
+        {
+          for (unsigned x = 0; x < SCREENWIDTH * g_graphics_state.scale; x++)
+            {
+              p_idx = g_graphics_state
+                          .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);
+            }
+
+          fbptr += g_graphics_state.pinfo.stride;
         }
+    }
+  else
+    {
+      /* Neither 16bpp nor 32bpp: the xoffset/stride math above assumed
+       * 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.
+       */
 
-      fbptr += g_graphics_state.pinfo.stride;
+      i_error("Unsupported framebuffer depth: %u bpp",
+              g_graphics_state.pinfo.bpp);
     }
 }
 
@@ -724,6 +765,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,
diff --git a/games/NXDoom/src/m_config.c b/games/NXDoom/src/m_config.c
@@ -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);
     }
 
