[Scummvm-tracker] [ScummVM :: Bugs] #17280: BACKENDS/MADS: Nebular: Background rendering issues with surfacesdl (SDL3)

[ScummVM :: Bugs]

#17280: BACKENDS/MADS: Nebular: Background rendering issues with surfacesdl (SDL3)
---------------------+--------------------------
Reporter:  dwatteau  |       Owner:  (none)
    Type:  defect    |      Status:  new
Priority:  high      |   Component:  Graphics
 Version:            |  Resolution:
Keywords:            |        Game:  Rex Nebular
---------------------+--------------------------
Comment (by dwatteau):

 I'm moving this to the `BACKENDS` graphics area, because after some Claude
 analysis it looks like the MADS engine may just be hitting a backend issue
 with SDL3.

 In the `SDL_SetColors()` path, SDL3 builds it were always calling
 `SDL_CreateSurfacePalette()` again. Claude produced this fix, then, which
 fixes the bug for me.

 {{{
 BACKENDS: SURFACESDL: Fix partial palette updates with SDL3 (Trac#17280)

 SDL_CreateSurfacePalette() replaces any existing surface palette with a
 new one where all entries are white [1], so each SDL_SetColors() call
 wiped every color outside the updated range.  Games doing partial palette
 updates, such as MADS with its palette cycling, got white backgrounds.
 Reuse the existing palette instead.

 Also, when saving a screenshot, just read the existing palette instead
 of replacing it, for consistency.

 [1]: https://wiki.libsdl.org/SDL3/SDL_CreateSurfacePalette#remarks

 Assisted-by: Claude:claude-opus-5-5

 diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 index ad7bc1a82fd..50abed5e0cd 100644
 --- a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 +++ b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 @@ -1620,7 +1620,7 @@ bool SurfaceSdlGraphicsManager::saveScreenshot(const
 Common::Path &filename) con
         bool success;

  #if SDL_VERSION_ATLEAST(3, 0, 0)
 -       SDL_Palette *sdlPalette = SDL_CreateSurfacePalette(_hwScreen);
 +       SDL_Palette *sdlPalette = SDL_GetSurfacePalette(_hwScreen);
  #else
         SDL_Palette *sdlPalette = _hwScreen->format->palette;
  #endif
 @@ -3202,7 +3202,9 @@ void
 SurfaceSdlGraphicsManager::SDL_UpdateRects(SDL_Surface *screen, int
 numrect

  int SurfaceSdlGraphicsManager::SDL_SetColors(SDL_Surface *surface,
 SDL_Color *colors, int firstcolor, int ncolors) {
  #if SDL_VERSION_ATLEAST(3, 0, 0)
 -       SDL_Palette *palette = SDL_CreateSurfacePalette(surface);
 +       SDL_Palette *palette = SDL_GetSurfacePalette(surface);
 +       if (!palette)
 +               palette = SDL_CreateSurfacePalette(surface);
         if (palette) {
                 return !SDL_SetPaletteColors(palette, colors, firstcolor,
 ncolors) ? 1 : 0;
         }
 }}}

 and then, while iterating through this, I was getting random
 `SDL_BlitSurface failed: Parameter 'src' is invalid!` warning windows from
 SDL. It then suggested the following change as well:

 {{{
 BACKENDS: SURFACESDL: Properly initialize `_numDirtyRects`

 The first screen update of a newly created manager could blit garbage
 rects from a screen surface that wasn't allocated yet, causing random
 "SDL_BlitSurface failed: Parameter 'src' is invalid!" startup errors.

 Assisted-by: Claude:claude-opus-5-5

 diff --git a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 index 50abed5e0cd..7ed441a6d42 100644
 --- a/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 +++ b/backends/graphics/surfacesdl/surfacesdl-graphics.cpp
 @@ -190,7 +190,7 @@
 SurfaceSdlGraphicsManager::SurfaceSdlGraphicsManager(SdlEventSource
 *sdlEventSou
  #endif
         _transactionMode(kTransactionNone),
         _scalerPlugins(ScalerMan.getPlugins()), _scalerPlugin(nullptr),
 _scaler(nullptr),
 -       _needRestoreAfterOverlay(false), _isInOverlayPalette(false),
 _isDoubleBuf(false), _prevForceRedraw(false), _numPrevDirtyRects(0),
 +       _needRestoreAfterOverlay(false), _isInOverlayPalette(false),
 _isDoubleBuf(false), _prevForceRedraw(false), _numDirtyRects(0),
 _numPrevDirtyRects(0),
         _prevCursorNeedsRedraw(false),
         _mouseKeyColor(0), _disableMouseKeyColor(false) {

 }}}

 I'm including this here for investigation purposes, but graphics area
 clearly is a place where I'd rather not submit any change myself, be it
 AI-assisted or not :D
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/17280#comment:3>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM