From 2ea3470cef489e659b074c36528791d7756e94e7 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:09:24 +0200 Subject: [PATCH 1/8] fix(rtg): restore dirty-page tracking for RTG display updates Switching between RTG and native modes left the display stale until the GUI was opened (fixes #27). The RTG dirty-page refresh optimization added in 5.9.0 (ported from Amiberry) only ever flushed full refreshes on non-Windows builds: - dirty_page_map was never allocated, so picasso_getwritewatch() always returned -1 and partial flushes copied zero pages; mark_dirty() from the P96 trap handlers was a no-op. - plain CPU stores to VRAM never marked pages dirty; only P96 API ops (FillRect, BlitRect, ...) did, and even those were lost to the unallocated map. Workbench renders mostly via direct CPU writes, so after a mode switch the RTG screen stayed grey/frozen until something forced a full refresh (e.g. opening and dismissing the GUI). Port the follow-up fixes from the main Amiberry repository: - allocate dirty_page_map in picasso_allocatewritewatch and track min/max dirty page bounds so picasso_getwritewatch scans only the touched range (amiberry 875c4198) - give the gfxmem banks custom put handlers (lput/wput/bput) that call mark_dirty() after every store, and flag the banks with S_WRITE so JIT-routed writes also go through the handlers (amiberry 63a74239) - clamp the unlockscr dirty rectangle to the current surface so a stale width_allocated after a mode switch cannot feed SDL_UpdateTexture an out-of-bounds region (amiberry 756ebf1e) --- src/osdep/amiberry_gfx.cpp | 27 +++++++--- src/osdep/picasso96.cpp | 106 +++++++++++++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 17 deletions(-) diff --git a/src/osdep/amiberry_gfx.cpp b/src/osdep/amiberry_gfx.cpp index 31520bd66..317e3f863 100644 --- a/src/osdep/amiberry_gfx.cpp +++ b/src/osdep/amiberry_gfx.cpp @@ -1422,13 +1422,28 @@ void unlockscr(struct vidbuffer* vb, int y_start, int y_end) // Record the dirty rectangle if y_start and y_end are valid. if (y_start >= 0 && y_end >= y_start) { AmigaMonitor* mon = &AMonitors[vb->monitor_id]; - SDL_Rect dirty_rect; - dirty_rect.x = 0; - dirty_rect.y = y_start; - dirty_rect.w = vb->width_allocated; - dirty_rect.h = y_end - y_start + 1; - add_dirty_rect(mon, dirty_rect); + // Clamp to the current surface: during a mode switch (native <-> RTG) + // width_allocated/height_allocated may still reflect the old, larger + // buffer until it is reallocated. An unclamped rect feeds + // SDL_UpdateTexture a region that reads past the new surface's pixel + // buffer and fails, leaving the display frozen. + int clamped_w = vb->width_allocated; + if (amiga_surface && clamped_w > amiga_surface->w) + clamped_w = amiga_surface->w; + int clamped_y_end = y_end; + if (amiga_surface && clamped_y_end >= amiga_surface->h) + clamped_y_end = amiga_surface->h - 1; + + if (clamped_w > 0 && clamped_y_end >= y_start) { + SDL_Rect dirty_rect; + dirty_rect.x = 0; + dirty_rect.y = y_start; + dirty_rect.w = clamped_w; + dirty_rect.h = clamped_y_end - y_start + 1; + + add_dirty_rect(mon, dirty_rect); + } } } diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index 0eb5c141f..cad00aea1 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -101,6 +101,8 @@ void mman_ResetWatch (PVOID lpBaseAddress, SIZE_T dwRegionSize); #else static bool* dirty_page_map[MAX_RTG_BOARDS]; static int dirty_page_map_size[MAX_RTG_BOARDS]; +static int min_dirty_page_index[MAX_RTG_BOARDS]; +static int max_dirty_page_index[MAX_RTG_BOARDS]; #endif static void picasso_flushpixels(int index, uae_u8 *src, int offset, bool render); @@ -442,8 +444,13 @@ static void mark_dirty(int index, uae_u8* addr, int size) if (start_page < 0) start_page = 0; if (end_page >= dirty_page_map_size[index]) end_page = dirty_page_map_size[index] - 1; - for (int i = start_page; i <= end_page; ++i) { - dirty_page_map[index][i] = true; + if (start_page < min_dirty_page_index[index]) min_dirty_page_index[index] = start_page; + if (end_page > max_dirty_page_index[index]) max_dirty_page_index[index] = end_page; + + if (start_page <= end_page) { + for (int i = start_page; i <= end_page; ++i) { + dirty_page_map[index][i] = true; + } } } #endif @@ -2745,6 +2752,15 @@ void picasso_allocatewritewatch (int index, int gfxmemsize) gwwbufsize[index] = gfxmemsize / gwwpagesize[index] + 1; gwwpagemask[index] = gwwpagesize[index] - 1; gwwbuf[index] = xmalloc (void*, gwwbufsize[index]); + + delete[] dirty_page_map[index]; + const int pages = gwwbufsize[index]; + dirty_page_map[index] = new bool[pages]; + dirty_page_map_size[index] = pages; + // Initialize min/max to the "empty" state + min_dirty_page_index[index] = pages; + max_dirty_page_index[index] = -1; + memset(dirty_page_map[index], 0, pages * sizeof(bool)); #endif } @@ -2781,7 +2797,18 @@ int picasso_getwritewatch (int index, int offset, uae_u8 ***gwwbufp, uae_u8 **st const int page_size = gwwpagesize[index]; int count = 0; - for (int i = 0; i < dirty_page_map_size[index]; ++i) { + int start = min_dirty_page_index[index]; + int end = max_dirty_page_index[index]; + + if (start > end) { + return 0; + } + + // Reset bounds immediately for next frame accumulation + min_dirty_page_index[index] = dirty_page_map_size[index]; + max_dirty_page_index[index] = -1; + + for (int i = start; i <= end; ++i) { if (dirty_page_map[index][i]) { if (count < gwwbufsize[index]) { gwwbuf[index][count++] = const_cast(base) + i * page_size; @@ -6200,41 +6227,98 @@ static int render_thread(void *v) return 0; } +// RTG memory banks: use MEMORY_FUNCTIONS for the read/check/xlate halves but +// provide custom put functions that also call mark_dirty(). Without this, +// direct CPU writes to VRAM (software renderers, apps bypassing the P96 API) +// are invisible to picasso_getwritewatch and the RTG display never updates. +// On WinUAE-native Windows builds the write-watch is provided by the OS +// (GetWriteWatch) and mark_dirty is not defined, so fall back to the stock +// MEMORY_FUNCTIONS in that case. +#ifndef _WIN32 +#define GFXMEM_PUT_FUNCTIONS(name, index) \ +static void REGPARAM3 name ## _lput (uaecptr, uae_u32) REGPARAM; \ +static void REGPARAM2 name ## _lput (uaecptr addr, uae_u32 l) \ +{ \ + uae_u8 *m; \ + addr -= name ## _bank.startaccessmask; \ + addr &= name ## _bank.mask; \ + m = name ## _bank.baseaddr + addr; \ + do_put_mem_long ((uae_u32 *)m, l); \ + mark_dirty((index), m, 4); \ +} \ +static void REGPARAM3 name ## _wput (uaecptr, uae_u32) REGPARAM; \ +static void REGPARAM2 name ## _wput (uaecptr addr, uae_u32 w) \ +{ \ + uae_u8 *m; \ + addr -= name ## _bank.startaccessmask; \ + addr &= name ## _bank.mask; \ + m = name ## _bank.baseaddr + addr; \ + do_put_mem_word ((uae_u16 *)m, w); \ + mark_dirty((index), m, 2); \ +} \ +static void REGPARAM3 name ## _bput (uaecptr, uae_u32) REGPARAM; \ +static void REGPARAM2 name ## _bput (uaecptr addr, uae_u32 b) \ +{ \ + addr -= name ## _bank.startaccessmask; \ + addr &= name ## _bank.mask; \ + name ## _bank.baseaddr[addr] = b; \ + mark_dirty((index), name ## _bank.baseaddr + addr, 1); \ +} + +#define GFXMEM_MEMORY_FUNCTIONS(name, index) \ +MEMORY_LGET(name); \ +MEMORY_WGET(name); \ +MEMORY_BGET(name); \ +GFXMEM_PUT_FUNCTIONS(name, index) \ +MEMORY_CHECK(name); \ +MEMORY_XLATE(name); + +// Force JIT to route writes through the bank's *_put handlers (by flagging +// the bank as special for writes via S_WRITE) so mark_dirty() runs on every +// CPU poke to VRAM. WinUAE relies on GetWriteWatch() for this instead. +// Without S_WRITE, JIT blocks that write directly to natmem never mark the +// touched pages dirty and picasso_flushpixels uploads nothing. +#define GFXMEM_JIT_WRITE_FLAG S_WRITE +#else +#define GFXMEM_MEMORY_FUNCTIONS(name, index) MEMORY_FUNCTIONS(name) +#define GFXMEM_JIT_WRITE_FLAG 0 +#endif + extern addrbank gfxmem_bank; -MEMORY_FUNCTIONS(gfxmem); +GFXMEM_MEMORY_FUNCTIONS(gfxmem, 0) addrbank gfxmem_bank = { gfxmem_lget, gfxmem_wget, gfxmem_bget, gfxmem_lput, gfxmem_wput, gfxmem_bput, gfxmem_xlate, gfxmem_check, nullptr, nullptr, _T("RTG RAM"), dummy_lgeti, dummy_wgeti, - ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, 0 + ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, GFXMEM_JIT_WRITE_FLAG }; extern addrbank gfxmem2_bank; -MEMORY_FUNCTIONS(gfxmem2); +GFXMEM_MEMORY_FUNCTIONS(gfxmem2, 1) addrbank gfxmem2_bank = { gfxmem2_lget, gfxmem2_wget, gfxmem2_bget, gfxmem2_lput, gfxmem2_wput, gfxmem2_bput, gfxmem2_xlate, gfxmem2_check, nullptr, nullptr, _T("RTG RAM #2"), dummy_lgeti, dummy_wgeti, - ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, 0 + ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, GFXMEM_JIT_WRITE_FLAG }; extern addrbank gfxmem3_bank; -MEMORY_FUNCTIONS(gfxmem3); +GFXMEM_MEMORY_FUNCTIONS(gfxmem3, 2) addrbank gfxmem3_bank = { gfxmem3_lget, gfxmem3_wget, gfxmem3_bget, gfxmem3_lput, gfxmem3_wput, gfxmem3_bput, gfxmem3_xlate, gfxmem3_check, nullptr, nullptr, _T("RTG RAM #3"), dummy_lgeti, dummy_wgeti, - ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, 0 + ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, GFXMEM_JIT_WRITE_FLAG }; extern addrbank gfxmem4_bank; -MEMORY_FUNCTIONS(gfxmem4); +GFXMEM_MEMORY_FUNCTIONS(gfxmem4, 3) addrbank gfxmem4_bank = { gfxmem4_lget, gfxmem4_wget, gfxmem4_bget, gfxmem4_lput, gfxmem4_wput, gfxmem4_bput, gfxmem4_xlate, gfxmem4_check, nullptr, nullptr, _T("RTG RAM #4"), dummy_lgeti, dummy_wgeti, - ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, 0 + ABFLAG_RAM | ABFLAG_RTG | ABFLAG_DIRECTACCESS, 0, GFXMEM_JIT_WRITE_FLAG }; addrbank *gfxmem_banks[MAX_RTG_BOARDS]; From ef58cf9a977207ac304592fb985df591151c26b5 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:19:38 +0200 Subject: [PATCH 2/8] fix(rtg): address review findings in dirty-page tracking - picasso_getwritewatch(): with VGA screen splitting active the getter was called once per source region but drained the whole dirty map on the first call, so pages belonging to the wrapped lower region were dropped by the first pass's range filter and missing from the second. Drain once per frame and reuse the page list for both regions. - picasso_getwritewatch(): return the region base as board base plus the screen offset, matching the Windows GetWriteWatch semantics. Returning the bare board base let the caller's range filter accept pages below the visible screen (offscreen bitmaps, split region). - mark_dirty()/picasso_getwritewatch(): synchronize the dirty bounds with the RTG render thread. With gfxcard_multithread enabled, mark_dirty() runs from the CPU write handlers while render_thread() drains the map, racing on the bounds (and allowing a drain to reset the bounds between a writer's compare and its bit-set, losing the page until another write covers it). The map and bounds are now std::atomic, and bounds are published only after the bits are set so a concurrent drain always re-covers marked pages on its next pass. --- src/osdep/picasso96.cpp | 78 +++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index cad00aea1..04f8d9c77 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -35,6 +35,7 @@ #include #include +#include #include "uae.h" @@ -99,10 +100,12 @@ static int picasso96_PCT = PCT_Unknown; int mman_GetWriteWatch (PVOID lpBaseAddress, SIZE_T dwRegionSize, PVOID *lpAddresses, PULONG_PTR lpdwCount, PULONG lpdwGranularity); void mman_ResetWatch (PVOID lpBaseAddress, SIZE_T dwRegionSize); #else -static bool* dirty_page_map[MAX_RTG_BOARDS]; +static std::atomic* dirty_page_map[MAX_RTG_BOARDS]; static int dirty_page_map_size[MAX_RTG_BOARDS]; -static int min_dirty_page_index[MAX_RTG_BOARDS]; -static int max_dirty_page_index[MAX_RTG_BOARDS]; +// The bounds are atomics: mark_dirty() runs in the CPU write handlers while +// picasso_getwritewatch() drains from the RTG render thread. +static std::atomic min_dirty_page_index[MAX_RTG_BOARDS]; +static std::atomic max_dirty_page_index[MAX_RTG_BOARDS]; #endif static void picasso_flushpixels(int index, uae_u8 *src, int offset, bool render); @@ -444,13 +447,25 @@ static void mark_dirty(int index, uae_u8* addr, int size) if (start_page < 0) start_page = 0; if (end_page >= dirty_page_map_size[index]) end_page = dirty_page_map_size[index] - 1; - if (start_page < min_dirty_page_index[index]) min_dirty_page_index[index] = start_page; - if (end_page > max_dirty_page_index[index]) max_dirty_page_index[index] = end_page; + if (start_page > end_page) { + return; + } + for (int i = start_page; i <= end_page; ++i) { + dirty_page_map[index][i].store(true, std::memory_order_relaxed); + } - if (start_page <= end_page) { - for (int i = start_page; i <= end_page; ++i) { - dirty_page_map[index][i] = true; - } + // Publish the bounds only after the bits are set. picasso_getwritewatch() + // resets the bounds before scanning, so publishing first would allow a + // drain to reset the bounds and then miss our already-set bits until some + // later write happens to expand the range over them. The CAS loops keep + // the bounds monotonic when several writers race. + int cur = min_dirty_page_index[index].load(); + while (start_page < cur && + !min_dirty_page_index[index].compare_exchange_weak(cur, start_page)) { + } + cur = max_dirty_page_index[index].load(); + while (end_page > cur && + !max_dirty_page_index[index].compare_exchange_weak(cur, end_page)) { } } #endif @@ -2755,12 +2770,14 @@ void picasso_allocatewritewatch (int index, int gfxmemsize) delete[] dirty_page_map[index]; const int pages = gwwbufsize[index]; - dirty_page_map[index] = new bool[pages]; + dirty_page_map[index] = new std::atomic[pages]; dirty_page_map_size[index] = pages; // Initialize min/max to the "empty" state - min_dirty_page_index[index] = pages; - max_dirty_page_index[index] = -1; - memset(dirty_page_map[index], 0, pages * sizeof(bool)); + min_dirty_page_index[index].store(pages); + max_dirty_page_index[index].store(-1); + for (int i = 0; i < pages; i++) { + dirty_page_map[index][i].store(false, std::memory_order_relaxed); + } #endif } @@ -2797,30 +2814,36 @@ int picasso_getwritewatch (int index, int offset, uae_u8 ***gwwbufp, uae_u8 **st const int page_size = gwwpagesize[index]; int count = 0; - int start = min_dirty_page_index[index]; - int end = max_dirty_page_index[index]; + int start = min_dirty_page_index[index].load(); + int end = max_dirty_page_index[index].load(); if (start > end) { return 0; } - // Reset bounds immediately for next frame accumulation - min_dirty_page_index[index] = dirty_page_map_size[index]; - max_dirty_page_index[index] = -1; + // Reset bounds immediately for next frame accumulation. mark_dirty() + // sets the page bits before publishing the bounds, so any page marked + // while we are scanning is covered by a bound updated after our reset. + min_dirty_page_index[index].store(dirty_page_map_size[index]); + max_dirty_page_index[index].store(-1); for (int i = start; i <= end; ++i) { - if (dirty_page_map[index][i]) { + if (dirty_page_map[index][i].load(std::memory_order_relaxed)) { if (count < gwwbufsize[index]) { gwwbuf[index][count++] = const_cast(base) + i * page_size; } - dirty_page_map[index][i] = false; // Reset after reading + dirty_page_map[index][i].store(false, std::memory_order_relaxed); // Reset after reading } } if (gwwbufp) *gwwbufp = (uae_u8**)gwwbuf[index]; if (startp) { - *startp = const_cast(base); + // Match the Windows semantics: the region base is the board base + // plus the screen offset, not the bare board base. Returning the + // bare base would widen the caller's range filter to pages below + // the visible screen (e.g. offscreen bitmaps, the split region). + *startp = const_cast(base) + offset; } return count; #endif @@ -2868,7 +2891,7 @@ bool picasso_is_vram_dirty (int index, uaecptr addr, int size) if (end_page >= dirty_page_map_size[index]) end_page = dirty_page_map_size[index] - 1; for (int i = start_page; i <= end_page; ++i) { - if (dirty_page_map[index][i]) { return true; } + if (dirty_page_map[index][i].load(std::memory_order_relaxed)) { return true; } } return false; #endif @@ -5978,6 +6001,7 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) int maxy = -1; int miny = pheight - 1; int flushlines = 0, matchcount = 0; + int partial_gwwcnt = -1; // dirty pages drained once, reused for both split regions struct picasso_vidbuf_description *vidinfo = &picasso_vidinfo[monid]; bool overlay_updated = false; @@ -6039,7 +6063,15 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) if (mman_GetWriteWatch(src_start[split], regionsize, gwwbuf[index], &gwwcnt, &ps)) continue; #else - gwwcnt = picasso_getwritewatch(index, off, (uae_u8***)&gwwbuf[index], &src_start[split]); + // The emulated write-watch drains the whole dirty map, so it + // must only be drained on the first region; the second + // (split) region reuses the same page list. Draining per + // region would clear pages that belong to the other region + // and leave it stale. + if (split == 0) { + partial_gwwcnt = picasso_getwritewatch(index, off, (uae_u8***)&gwwbuf[index], &src_start[split]); + } + gwwcnt = partial_gwwcnt; #endif } From 942eeed35f19b3fab66a72eedc0894e4987c8fb8 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:27:20 +0200 Subject: [PATCH 3/8] fix(rtg): page-align the dirty-page region base picasso_getwritewatch() returns page-aligned addresses, but the region base was returned as the unaligned base + screen offset. With a panned (SetPanning) screen offset that is not page-aligned, the page holding the top-left of the visible screen compares below the returned base, so the range check in picasso_flushpixels() rejects it after its dirty bit was already cleared, leaving updates near the top-left stale. Round the base down to the page boundary (offset & ~gwwpagemask), matching how picasso_flushpixels() itself initializes src_start[0]. --- src/osdep/picasso96.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index 04f8d9c77..cd828b82e 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -2843,7 +2843,12 @@ int picasso_getwritewatch (int index, int offset, uae_u8 ***gwwbufp, uae_u8 **st // plus the screen offset, not the bare board base. Returning the // bare base would widen the caller's range filter to pages below // the visible screen (e.g. offscreen bitmaps, the split region). - *startp = const_cast(base) + offset; + // The returned page list is page-aligned, so round the base down + // too: with a panned (SetPanning) screen offset that is not + // page-aligned, an unaligned base would reject the page holding + // the top-left of the visible screen after its dirty bit was + // already cleared, leaving it stale. + *startp = const_cast(base) + (offset & ~gwwpagemask[index]); } return count; #endif From 65daf7d5b8f1f2b4e2751c131dc8a5fdc3c039e4 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:33:19 +0200 Subject: [PATCH 4/8] fix(rtg): claim the dirty range with a single compare_exchange Review follow-up: making the two bound variables atomic does not make the drain's two-store reset atomic with respect to mark_dirty(). A writer can publish a page between the drain's load and its reset, so the reset then erases the freshly published bound: the page stays set but outside any published range and is not rendered until a later write happens to republish a range. Pack the bounds into a single word, (min_page << 32) | (max_page + 1): mark_dirty() widens the packed value with a compare_exchange loop after setting the page bits, and picasso_getwritewatch() claims the whole range with one compare_exchange to the empty state, retrying against the wider range if a concurrent widening made the exchange fail. A claimed range therefore always includes every page published before the claim, and pages marked after the claim are published anew for the next frame. --- src/osdep/picasso96.cpp | 81 +++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index cd828b82e..121b4d709 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -102,10 +102,12 @@ void mman_ResetWatch (PVOID lpBaseAddress, SIZE_T dwRegionSize); #else static std::atomic* dirty_page_map[MAX_RTG_BOARDS]; static int dirty_page_map_size[MAX_RTG_BOARDS]; -// The bounds are atomics: mark_dirty() runs in the CPU write handlers while -// picasso_getwritewatch() drains from the RTG render thread. -static std::atomic min_dirty_page_index[MAX_RTG_BOARDS]; -static std::atomic max_dirty_page_index[MAX_RTG_BOARDS]; +// Dirty-page bounds packed into a single word, (min_page << 32) | (max_page + 1): +// mark_dirty() runs in the CPU write handlers while picasso_getwritewatch() +// drains from the RTG render thread, and a single word lets the drain claim +// the whole range with one compare_exchange so a concurrent writer can never +// have its freshly published range overwritten by a reset. +static std::atomic dirty_bounds[MAX_RTG_BOARDS]; #endif static void picasso_flushpixels(int index, uae_u8 *src, int offset, bool render); @@ -430,6 +432,26 @@ static int gwwbufsize[MAX_RTG_BOARDS], gwwpagesize[MAX_RTG_BOARDS], gwwpagemask[ //extern uae_u8* natmem_offset; #ifndef _WIN32 +// Widen the published dirty range. Called from the CPU write handlers after +// the page bits have been set. +static void dirty_bounds_widen(int index, int start_page, int end_page) +{ + uae_u64 cur = dirty_bounds[index].load(); + for (;;) { + const int cur_min = static_cast(cur >> 32); + const int cur_max = static_cast(static_cast(cur)) - 1; + const int new_min = start_page < cur_min ? start_page : cur_min; + const int new_max = end_page > cur_max ? end_page : cur_max; + if (new_min == cur_min && new_max == cur_max) { + return; + } + const uae_u64 next = (static_cast(new_min) << 32) | static_cast(new_max + 1); + if (dirty_bounds[index].compare_exchange_weak(cur, next)) { + return; + } + } +} + static void mark_dirty(int index, uae_u8* addr, int size) { if (index < 0 || !dirty_page_map[index]) @@ -454,19 +476,11 @@ static void mark_dirty(int index, uae_u8* addr, int size) dirty_page_map[index][i].store(true, std::memory_order_relaxed); } - // Publish the bounds only after the bits are set. picasso_getwritewatch() - // resets the bounds before scanning, so publishing first would allow a - // drain to reset the bounds and then miss our already-set bits until some - // later write happens to expand the range over them. The CAS loops keep - // the bounds monotonic when several writers race. - int cur = min_dirty_page_index[index].load(); - while (start_page < cur && - !min_dirty_page_index[index].compare_exchange_weak(cur, start_page)) { - } - cur = max_dirty_page_index[index].load(); - while (end_page > cur && - !max_dirty_page_index[index].compare_exchange_weak(cur, end_page)) { - } + // Publish the widened bounds only after the bits are set. The bounds are + // a single word so picasso_getwritewatch() can claim the whole range with + // one compare_exchange; two separate words would let a drain reset one + // half after a writer published, losing the range until a later write. + dirty_bounds_widen(index, start_page, end_page); } #endif @@ -2772,9 +2786,8 @@ void picasso_allocatewritewatch (int index, int gfxmemsize) const int pages = gwwbufsize[index]; dirty_page_map[index] = new std::atomic[pages]; dirty_page_map_size[index] = pages; - // Initialize min/max to the "empty" state - min_dirty_page_index[index].store(pages); - max_dirty_page_index[index].store(-1); + // Initialize the bounds to the "empty" state (min = pages, max = -1) + dirty_bounds[index].store(static_cast(pages) << 32); for (int i = 0; i < pages; i++) { dirty_page_map[index][i].store(false, std::memory_order_relaxed); } @@ -2814,19 +2827,25 @@ int picasso_getwritewatch (int index, int offset, uae_u8 ***gwwbufp, uae_u8 **st const int page_size = gwwpagesize[index]; int count = 0; - int start = min_dirty_page_index[index].load(); - int end = max_dirty_page_index[index].load(); - - if (start > end) { - return 0; + // Claim the whole dirty range with a single compare_exchange. If a + // writer widened the bounds between our load and the exchange, the + // exchange fails and we retry against the wider range instead of + // resetting over the freshly published pages and losing them. + const uae_u64 empty = static_cast(dirty_page_map_size[index]) << 32; + uae_u64 cur = dirty_bounds[index].load(); + int start; + int end; + for (;;) { + start = static_cast(cur >> 32); + end = static_cast(static_cast(cur)) - 1; + if (start > end) { + return 0; + } + if (dirty_bounds[index].compare_exchange_weak(cur, empty)) { + break; + } } - // Reset bounds immediately for next frame accumulation. mark_dirty() - // sets the page bits before publishing the bounds, so any page marked - // while we are scanning is covered by a bound updated after our reset. - min_dirty_page_index[index].store(dirty_page_map_size[index]); - max_dirty_page_index[index].store(-1); - for (int i = start; i <= end; ++i) { if (dirty_page_map[index][i].load(std::memory_order_relaxed)) { if (count < gwwbufsize[index]) { From 67c58b985018de05432cec04dcfdf147d29584a7 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:40:39 +0200 Subject: [PATCH 5/8] fix(rtg): filter the reused dirty-page count per split region Review follow-up: the dirty page list drained once per frame spans both split regions (and can contain pages outside the visible screen, e.g. offscreen bitmaps), but the full-copy heuristic compared that total against each region's own size. A busy first region could push the second region over the 80% threshold and trigger a needless full copy. Count only the pages that fall inside the current region, matching the region-scoped Windows GetWriteWatch, and skip the region entirely when none apply. --- src/osdep/picasso96.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index 121b4d709..73a262093 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -6099,12 +6099,25 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) #endif } - matchcount += (int)gwwcnt; + // The reused page list spans both split regions (and may contain + // pages outside the visible screen, e.g. offscreen bitmaps), so + // filter it down to this region before deciding between a full + // copy and partial rows. Windows' region-scoped GetWriteWatch + // never sees foreign pages here. + int region_gwwcnt = 0; + for (int i = 0; i < gwwcnt; i++) { + const uae_u8* p = static_cast(gwwbuf[index][i]); + if (p >= src_start[split] && p < src_end[split]) { + region_gwwcnt++; + } + } + + matchcount += region_gwwcnt; - if (gwwcnt == 0) { + if (region_gwwcnt == 0) { continue; } - dofull = gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; + dofull = region_gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; if (!dstp) { dstp = gfx_lock_picasso(monid, dofull); From 0ef99a21770bee176fa7a5813c68765cba695d09 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:47:30 +0200 Subject: [PATCH 6/8] fix(rtg): keep forced full refreshes out of the region filter Review follow-up: the per-region page filter must only apply to the drained dirty list. In the forced full-refresh path (full_refresh < 0 after a mode change, flash or RTG clear) the page count is synthetic and gwwbuf is filled with generated region pages, so filtering there could only ever discard required work. Move the filter into the drain branch and count the synthetic list directly. --- src/osdep/picasso96.cpp | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index 73a262093..b1c63d4a1 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -6080,12 +6080,14 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) for (int i = 0; i < gwwcnt; i++) gwwbuf[index][i] = src_start[split] + i * gwwpagesize[index]; + matchcount += (int)gwwcnt; } else { #ifdef _WIN32 ULONG ps; gwwcnt = gwwbufsize[index]; if (mman_GetWriteWatch(src_start[split], regionsize, gwwbuf[index], &gwwcnt, &ps)) continue; + matchcount += (int)gwwcnt; #else // The emulated write-watch drains the whole dirty map, so it // must only be drained on the first region; the second @@ -6096,28 +6098,28 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) partial_gwwcnt = picasso_getwritewatch(index, off, (uae_u8***)&gwwbuf[index], &src_start[split]); } gwwcnt = partial_gwwcnt; -#endif - } - // The reused page list spans both split regions (and may contain - // pages outside the visible screen, e.g. offscreen bitmaps), so - // filter it down to this region before deciding between a full - // copy and partial rows. Windows' region-scoped GetWriteWatch - // never sees foreign pages here. - int region_gwwcnt = 0; - for (int i = 0; i < gwwcnt; i++) { - const uae_u8* p = static_cast(gwwbuf[index][i]); - if (p >= src_start[split] && p < src_end[split]) { - region_gwwcnt++; + // The reused page list spans both split regions (and may + // contain pages outside the visible screen, e.g. offscreen + // bitmaps), so filter it down to this region before deciding + // between a full copy and partial rows. Windows' region-scoped + // GetWriteWatch never sees foreign pages here. + int region_gwwcnt = 0; + for (int i = 0; i < gwwcnt; i++) { + const uae_u8* p = static_cast(gwwbuf[index][i]); + if (p >= src_start[split] && p < src_end[split]) { + region_gwwcnt++; + } } + matchcount += region_gwwcnt; + gwwcnt = region_gwwcnt; +#endif } - matchcount += region_gwwcnt; - - if (region_gwwcnt == 0) { + if (gwwcnt == 0) { continue; } - dofull = region_gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; + dofull = gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; if (!dstp) { dstp = gfx_lock_picasso(monid, dofull); From a085efa10b89c68bb7d66ebe80c019bd8966a43f Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Tue, 15 Sep 2026 23:53:22 +0200 Subject: [PATCH 7/8] fix(rtg): iterate the full dirty list when copying regions Review follow-up: shortening gwwcnt to the region-filtered count made the copy loop examine only a prefix of the page-ordered list, but foreign pages (lower split region, offscreen bitmaps) can occupy that prefix, so matching pages later in the list were skipped after their dirty bits had already been cleared by the drain. Keep gwwcnt untruncated for iteration - the copy loop range-checks each entry itself - and use the filtered count only for the skip decision and the full-copy heuristic. Compacting the list in place instead would destroy the entries the second split region still has to examine. --- src/osdep/picasso96.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index b1c63d4a1..f5b0a4452 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -6081,6 +6081,11 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) for (int i = 0; i < gwwcnt; i++) gwwbuf[index][i] = src_start[split] + i * gwwpagesize[index]; matchcount += (int)gwwcnt; + + if (gwwcnt == 0) { + continue; + } + dofull = gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; } else { #ifdef _WIN32 ULONG ps; @@ -6088,6 +6093,11 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) if (mman_GetWriteWatch(src_start[split], regionsize, gwwbuf[index], &gwwcnt, &ps)) continue; matchcount += (int)gwwcnt; + + if (gwwcnt == 0) { + continue; + } + dofull = gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; #else // The emulated write-watch drains the whole dirty map, so it // must only be drained on the first region; the second @@ -6101,9 +6111,13 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) // The reused page list spans both split regions (and may // contain pages outside the visible screen, e.g. offscreen - // bitmaps), so filter it down to this region before deciding - // between a full copy and partial rows. Windows' region-scoped - // GetWriteWatch never sees foreign pages here. + // bitmaps), so filter it down to this region when deciding + // between a full copy and partial rows. The copy loop below + // must keep iterating the FULL list: it range-checks each + // entry itself, and truncating the count here would hide + // matching pages that sit behind foreign (lower-split or + // offscreen) pages in the page-ordered list after their + // dirty bits have already been cleared. int region_gwwcnt = 0; for (int i = 0; i < gwwcnt; i++) { const uae_u8* p = static_cast(gwwbuf[index][i]); @@ -6112,14 +6126,13 @@ static void picasso_flushpixels(int index, uae_u8 *src, int off, bool render) } } matchcount += region_gwwcnt; - gwwcnt = region_gwwcnt; -#endif - } - if (gwwcnt == 0) { - continue; + if (region_gwwcnt == 0) { + continue; + } + dofull = region_gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; +#endif } - dofull = gwwcnt >= (regionsize / gwwpagesize[index]) * 80 / 100; if (!dstp) { dstp = gfx_lock_picasso(monid, dofull); From 4a43d52a36d89c9e6fba8715737b93951fa83e24 Mon Sep 17 00:00:00 2001 From: Dimitris Panokostas Date: Wed, 16 Sep 2026 06:35:49 +0200 Subject: [PATCH 8/8] fix(rtg): clear dirty bits with an atomic exchange Review follow-up: draining a page with a separate load and store could lose a concurrent write - the writer sets the bit and republishes the bounds between the drain's load and its unconditional clear, so the next drain claims the republished bounds but finds no set bit and never uploads the new pixels. Use exchange(false) so the test and the clear are one read-modify-write: a mark made before the exchange is consumed by this drain (the copy reads VRAM after it), and a mark made after it stays set for the next drain. --- src/osdep/picasso96.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/osdep/picasso96.cpp b/src/osdep/picasso96.cpp index f5b0a4452..5b08124ab 100644 --- a/src/osdep/picasso96.cpp +++ b/src/osdep/picasso96.cpp @@ -2846,12 +2846,14 @@ int picasso_getwritewatch (int index, int offset, uae_u8 ***gwwbufp, uae_u8 **st } } + // Clear with a single read-modify-write: a plain load/store pair could + // let a concurrent writer set the bit between our load and our clear, + // erasing its mark even though it republished the bounds covering it. for (int i = start; i <= end; ++i) { - if (dirty_page_map[index][i].load(std::memory_order_relaxed)) { + if (dirty_page_map[index][i].exchange(false, std::memory_order_relaxed)) { if (count < gwwbufsize[index]) { gwwbuf[index][count++] = const_cast(base) + i * page_size; } - dirty_page_map[index][i].store(false, std::memory_order_relaxed); // Reset after reading } }