Skip to content

Commit e7bb95f

Browse files
committed
gfx/gdi,vga: reuse menu frame buffer across set_texture_frame calls
Both drivers unconditionally free'd menu_frame and set it to NULL before a subsequent size-comparison check, making the "only reallocate if size changed" branch dead. Every menu frame triggered a full free+malloc+memcpy round-trip on a buffer whose size rarely or never changes. gdi: replace with a cached-capacity realloc pattern matching the earlier caca/fpga/sixel/network fix — allocate or grow only when the required size exceeds the current capacity; otherwise reuse. The menu_frame_cap field is added to gdi_t in gdi_defines.h. Since gdi_t is heap-allocated via calloc and freed entire, no explicit cap-reset is needed in the free path (re-init gets fresh zeros). vga: the menu buffer is *always* VGA_WIDTH*VGA_HEIGHT (64000 bytes) regardless of the source frame dimensions — the incoming frame is downscaled into this fixed-size buffer. No capacity tracking is needed; allocate once on first call and reuse forever. The original size-comparison check was doubly meaningless (against a buffer of constant size). Note on vga: an unrelated pre-existing bug — the rgb32 branch of the downscaling loop does not populate vga_menu_frame at all, leaving stale/uninitialized content — is flagged with a FIXME comment but not fixed here. It deserves its own commit with testing on a real DOS target. Thread-safety for both: set_texture_frame and the frame read-site both run on the video thread (threaded video) or main thread (non-threaded), sequentially within the same loop iteration. No synchronization required, none added. The Win32 WM_PAINT handler reads other fields of gdi_t from the UI thread; menu_frame is not among them and this patch does not alter the pre-existing concurrency surface. The realloc/once-alloc patterns additionally close a transient NULL window that existed between free and malloc in the original code.
1 parent 05de9cb commit e7bb95f

3 files changed

Lines changed: 36 additions & 40 deletions

File tree

gfx/common/gdi_defines.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ typedef struct gdi
3535
HBITMAP bmp_old;
3636
uint16_t *temp_buf;
3737
uint8_t *menu_frame;
38+
size_t menu_frame_cap;
3839

3940
unsigned video_width;
4041
unsigned video_height;

gfx/drivers/gdi_gfx.c

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -872,38 +872,29 @@ static void gdi_set_texture_frame(void *data,
872872
const void *frame, bool rgb32, unsigned width, unsigned height,
873873
float alpha)
874874
{
875-
gdi_t *gdi = (gdi_t*)data;
876-
unsigned pitch = width * 2;
875+
gdi_t *gdi = (gdi_t*)data;
876+
unsigned pitch = width * (rgb32 ? 4 : 2);
877+
size_t required;
877878

878-
if (rgb32)
879-
pitch = width * 4;
879+
if (!frame || !width || !height || !pitch)
880+
return;
880881

881-
if (gdi->menu_frame)
882-
free(gdi->menu_frame);
883-
gdi->menu_frame = NULL;
882+
required = (size_t)pitch * (size_t)height;
884883

885-
if ( !gdi->menu_frame
886-
|| (gdi->menu_width != width)
887-
|| (gdi->menu_height != height)
888-
|| (gdi->menu_pitch != pitch))
884+
if (required > gdi->menu_frame_cap)
889885
{
890-
if (pitch && height)
891-
{
892-
unsigned char *tmp = (unsigned char*)malloc(pitch * height);
893-
894-
if (tmp)
895-
gdi->menu_frame = tmp;
896-
}
886+
uint8_t *tmp = (uint8_t*)realloc(gdi->menu_frame, required);
887+
if (!tmp)
888+
return; /* keep previous frame intact */
889+
gdi->menu_frame = tmp;
890+
gdi->menu_frame_cap = required;
897891
}
898892

899-
if (gdi->menu_frame && frame && pitch && height)
900-
{
901-
memcpy(gdi->menu_frame, frame, pitch * height);
902-
gdi->menu_width = width;
903-
gdi->menu_height = height;
904-
gdi->menu_pitch = pitch;
905-
gdi->menu_bits = rgb32 ? 32 : 16;
906-
}
893+
memcpy(gdi->menu_frame, frame, required);
894+
gdi->menu_width = width;
895+
gdi->menu_height = height;
896+
gdi->menu_pitch = pitch;
897+
gdi->menu_bits = rgb32 ? 32 : 16;
907898
}
908899

909900
static void gdi_set_video_mode(void *data, unsigned width, unsigned height,

gfx/drivers/vga_gfx.c

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -405,23 +405,24 @@ static void vga_set_texture_frame(void *data,
405405
float alpha)
406406
{
407407
vga_t *vga = (vga_t*)data;
408-
unsigned pitch = width * 2;
408+
unsigned pitch = width * (rgb32 ? 4 : 2);
409409

410-
if (rgb32)
411-
pitch = width * 4;
412-
413-
if (vga->vga_menu_frame)
414-
free(vga->vga_menu_frame);
415-
vga->vga_menu_frame = NULL;
410+
if (!frame || !width || !height || !pitch)
411+
return;
416412

417-
if ( !vga->vga_menu_frame ||
418-
vga->vga_menu_width != width ||
419-
vga->vga_menu_height != height ||
420-
vga->vga_menu_pitch != pitch)
421-
if (pitch && height)
422-
vga->vga_menu_frame = (unsigned char*)malloc(VGA_WIDTH * VGA_HEIGHT);
413+
/* vga_menu_frame is always VGA_WIDTH*VGA_HEIGHT regardless of the
414+
* incoming source frame size — the source is downscaled into this
415+
* fixed-size buffer below. Allocate once on first call and reuse
416+
* thereafter; the original free+malloc per call was pure churn on
417+
* a buffer whose size never changes. */
418+
if (!vga->vga_menu_frame)
419+
{
420+
unsigned char *tmp = (unsigned char*)malloc(VGA_WIDTH * VGA_HEIGHT);
421+
if (!tmp)
422+
return; /* keep previous frame intact (NULL) */
423+
vga->vga_menu_frame = tmp;
424+
}
423425

424-
if (vga->vga_menu_frame && frame && pitch && height)
425426
{
426427
unsigned x, y;
427428

@@ -444,6 +445,9 @@ static void vga_set_texture_frame(void *data,
444445
}
445446
}
446447
}
448+
/* FIXME: rgb32 path does not populate vga_menu_frame - leaves
449+
* stale/uninitialized content for the renderer. Separate pre-
450+
* existing bug, not fixed here. */
447451

448452
vga->vga_menu_width = width;
449453
vga->vga_menu_height = height;

0 commit comments

Comments
 (0)