Skip to content

Commit

Permalink
Engine: fix overlays or cursor crash software render if sprite unloads
Browse files Browse the repository at this point in the history
This problem with overlays might be dating back to the late 3.6.0 development, and is affecting mouse cursors probably since 3.6.1 (it may be difficult to specify now).
But at some point I have missed this fact that if we do not clone a sprite assigned to a game object, and such sprite gets unloaded when freeing space in a sprite cache, then the Software renderer might crash when presenting visible objects on screen. Because in its current implementation Software renderer's pseudo-textures do not take ownership of a sprite's image, but only have a reference of it.
For Characters and Room objects Software renderer already clones their sprites, primarily because it has to cache various visual effects done on them.
But it no longer does for: Screen Overlays, and mouse cursor.

This is very easy to test (with Software renderer):
- create a screen overlay with non-dynamic sprite on it, and/or simply have a mouse cursor with non-dynamic sprite.
- start loading very big regular sprites into memory.
- as soon as sprite cache reaches the limit, it will free some sprites, and with very high chance overlay and/or cursor will cause a crash.

Anyway, a proper long-term solution for this would perhaps be to lock the sprites that are in current use on visible game objects. This would require a precise control over locked state, and a sprite lock counter (instead of just a flag).
For now I have to make a quick fix, which would simply test if a sprite is loaded, and tell to reupdate the texture if it's not.
  • Loading branch information
ivan-mogilko committed Aug 26, 2024
1 parent 4837ebb commit 203a76d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 5 deletions.
7 changes: 7 additions & 0 deletions Common/ac/spritecache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ bool SpriteCache::IsSpriteLoaded(sprkey_t index) const
return ResourceCache::Exists(index);
}

bool SpriteCache::IsAssetUnloaded(sprkey_t index) const
{
return index >= 0 && (size_t)index < _spriteData.size() && // in the valid range
_spriteData[index].IsAssetSprite() && // found in the game resources
!ResourceCache::Exists(index);
}

void SpriteCache::Reset()
{
_file.Close();
Expand Down
5 changes: 5 additions & 0 deletions Common/ac/spritecache.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class SpriteCache :
bool IsAssetSprite(sprkey_t index) const;
// Tells if the sprite is loaded into the memory (either from asset file, or assigned directly)
bool IsSpriteLoaded(sprkey_t index) const;
// Tells if the given slot represents an "asset sprite", which is currently unloaded.
// This is a helper method that lets distinguish an asset missing from memory,
// because IsSpriteLoaded() also reports true for external sprites, and false for
// any non-occupied sprite slots.
bool IsAssetUnloaded(sprkey_t index) const;
// Loads sprite using SpriteFile if such index is known,
// frees the space if cache size reaches the limit
void PrecacheSprite(sprkey_t index);
Expand Down
10 changes: 7 additions & 3 deletions Engine/ac/draw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2681,6 +2681,9 @@ static void construct_overlays()
overcache[i].X = pos.X; overcache[i].Y = pos.Y;
}

// Test if must recache the unloaded sprite in software mode
has_changed |= (is_software_mode && spriteset.IsAssetUnloaded(over.GetSpriteNum()));

if (has_changed || overtx.IsChangeNotified())
{
overtx.SpriteID = over.GetSpriteNum();
Expand Down Expand Up @@ -2783,10 +2786,11 @@ void construct_game_screen_overlay(bool draw_mouse)
}

// Mouse cursor
if ((play.screen_is_faded_out == 0) &&
draw_mouse && !play.mouse_cursor_hidden)
if ((play.screen_is_faded_out == 0) && draw_mouse && !play.mouse_cursor_hidden)
{
if (cursor_gstate.HasChanged() || cursor_tx.IsChangeNotified())
if (cursor_gstate.HasChanged() || cursor_tx.IsChangeNotified() ||
// Test if must recache the unloaded sprite in software mode
(drawstate.SoftwareRender && (cursor_gstate.GetSpriteNum() >= 0) && spriteset.IsAssetUnloaded(cursor_gstate.GetSpriteNum())))
{
cursor_tx.SpriteID = cursor_gstate.GetSpriteNum();
if (cursor_tx.SpriteID != UINT32_MAX)
Expand Down
4 changes: 2 additions & 2 deletions Engine/ac/screenoverlay.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ struct ScreenOverlay
}
// Gets actual overlay's image, whether owned by overlay or by a sprite reference
Common::Bitmap *GetImage() const;
// Get sprite id, or 0 if none set
int GetSpriteNum() const { return _sprnum; }
// Get this overlay's sprite id
int GetSpriteNum() const { return _sprnum; }
Size GetGraphicSize() const;
// Assigns an exclusive image to this overlay; the image will be stored as a dynamic sprite
// in a sprite cache, but owned by this overlay and therefore disposed at its disposal
Expand Down

0 comments on commit 203a76d

Please sign in to comment.