Skip to content

Commit e976983

Browse files
committed
gfx/vita2d: fix broken size-change check and replace per-pixel copy with per-row memcpy
vita2d_set_texture_frame had two separate bugs in its menu-texture update path: 1. Broken size-change check. The recreate condition was if ((width != vita->menu.width) && (height != vita->menu.height) && vita->menu.texture) with && between the dimension comparisons. That only recreates the texture when BOTH dimensions change. If a resize flips only one dimension (e.g. 480x272 -> 480x544 on a portrait-mode menu toggle, or a width-only change on menu-size setting updates), the old texture stays in place but the per-pixel writes below run with the new dimensions - indexing past the old allocation when the new size is larger, or leaving stale border pixels when it is smaller. Change the operator to || so either dimension change triggers a recreate. Also hoist the 'vita->menu.texture' truthiness check to the front of the expression so the short-circuit is: 'no texture to free anyway -> skip the whole branch', which reads more naturally than the old ordering. 2. Per-pixel copy. Both the rgb32 and rgb16 branches wrote the frame into the texture one pixel at a time inside a double loop: for (i = 0; i < height; i++) for (j = 0; j < width; j++) tex32[j + i*stride] = frame32[j + i*width]; The destination texture's stride can legitimately differ from the source pitch (width * bpp) for alignment reasons, so a single memcpy of the whole buffer is not correct, but a per-row memcpy handles that case and is straightforwardly faster than the per-pixel nested-loop form the compiler has to try hard to vectorise. Replace with for (i = 0; i < height; i++) memcpy(tex + i * stride, frame + i * width, rowlen); in both branches. Also clean up the loop-counter type: the original declared 'int i, j' but compared against 'unsigned height' / 'unsigned width'. Now just 'unsigned i'; j is no longer needed. Pre-existing not-addressed-here: if rgb32 flips between calls while the texture dimensions happen to stay the same, the new check (same as the old one in this respect) does not recreate the texture, so the texture format and the caller's data format diverge. In current practice the menu drivers that drive set_texture_frame always pass a consistent rgb32 for the duration of a session (rgui always passes false), so this is latent rather than live, and fixing it cleanly needs a stored 'was_rgb32' flag on vita_menu_t. Deferred. Thread-safety: unchanged. set_texture_frame and vita2d_frame both run on the video thread (threaded video) or main thread (non-threaded), sequentially within the same loop iteration. No synchronisation introduced or needed. The transient 'NULL texture between free and recreate' window is present in both the old and new code and is correctly guarded by the 'if (vita->menu.texture)' check at the reader in vita2d_frame.
1 parent d37d161 commit e976983

1 file changed

Lines changed: 28 additions & 16 deletions

File tree

gfx/drivers/vita2d_gfx.c

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,14 +1041,22 @@ static void vita2d_apply_state_changes(void *data)
10411041
static void vita2d_set_texture_frame(void *data, const void *frame, bool rgb32,
10421042
unsigned width, unsigned height, float alpha)
10431043
{
1044-
int i, j;
1044+
unsigned i;
10451045
void *tex_p;
10461046
unsigned int stride;
10471047
vita_video_t *vita = (vita_video_t*)data;
10481048

1049-
if ( (width != vita->menu.width)
1050-
&& (height != vita->menu.height)
1051-
&& vita->menu.texture)
1049+
/* Recreate when either dimension changes, not both. The original
1050+
* condition used && between the width/height comparisons, so a
1051+
* resize that only changed one dimension (e.g. 480x272 -> 480x544
1052+
* when the menu toggles portrait mode, or a width-only change on
1053+
* menu-size setting updates) would skip the destroy-recreate step;
1054+
* the subsequent per-pixel write below would then index past the
1055+
* old texture's allocation (new size > old) or leave stale border
1056+
* pixels (new size < old). */
1057+
if ( vita->menu.texture
1058+
&& ( width != (unsigned)vita->menu.width
1059+
|| height != (unsigned)vita->menu.height))
10521060
{
10531061
vita2d_wait_rendering_done();
10541062
vita2d_free_texture(vita->menu.texture);
@@ -1073,27 +1081,31 @@ static void vita2d_set_texture_frame(void *data, const void *frame, bool rgb32,
10731081
tex_p = vita2d_texture_get_datap(vita->menu.texture);
10741082
stride = vita2d_texture_get_stride(vita->menu.texture);
10751083

1084+
/* Copy row-by-row via memcpy rather than pixel-by-pixel. The
1085+
* texture stride (destination) can differ from the source pitch
1086+
* (width * bpp) for alignment reasons, so a single memcpy of the
1087+
* whole buffer isn't safe when stride != width*bpp, but a per-row
1088+
* memcpy handles both cases and is materially faster than the
1089+
* previous nested loop with per-pixel indexing. */
10761090
if (rgb32)
10771091
{
1078-
uint32_t *tex32 = tex_p;
1079-
const uint32_t *frame32 = frame;
1080-
1081-
stride /= 4;
1092+
uint32_t *tex32 = (uint32_t*)tex_p;
1093+
const uint32_t *frame32 = (const uint32_t*)frame;
1094+
size_t rowlen = (size_t)width * 4;
10821095

1096+
stride /= 4;
10831097
for (i = 0; i < height; i++)
1084-
for (j = 0; j < width; j++)
1085-
tex32[j + i*stride] = frame32[j + i*width];
1098+
memcpy(tex32 + i * stride, frame32 + i * width, rowlen);
10861099
}
10871100
else
10881101
{
1089-
uint16_t *tex16 = tex_p;
1090-
const uint16_t *frame16 = frame;
1091-
1092-
stride /= 2;
1102+
uint16_t *tex16 = (uint16_t*)tex_p;
1103+
const uint16_t *frame16 = (const uint16_t*)frame;
1104+
size_t rowlen = (size_t)width * 2;
10931105

1106+
stride /= 2;
10941107
for (i = 0; i < height; i++)
1095-
for (j = 0; j < width; j++)
1096-
tex16[j + i*stride] = frame16[j + i*width];
1108+
memcpy(tex16 + i * stride, frame16 + i * width, rowlen);
10971109
}
10981110
}
10991111

0 commit comments

Comments
 (0)