Skip to content

hold bitmap references for replacement and script-set textures - #7763

Open
Goober5000 wants to merge 1 commit into
scp-fs2open:masterfrom
Goober5000:fix/texture_management
Open

Goober5000 wants to merge 1 commit into
scp-fs2open:masterfrom
Goober5000:fix/texture_management

Conversation

@Goober5000

@Goober5000 Goober5000 commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Model instance replacement arrays and script-set model textures stored raw
bitmap handles without taking a load-count reference. A script doing
ship.Textures["foo"] = gr.loadTexture("bar") therefore left the ship pointing
at a bitmap slot that was freed as soon as Lua collected the temporary
texture handle, and reused by whatever bitmap loaded next.

  • Add bm_add_ref() / bm_release_ref() to bmpman so callers no longer need
    BMPMAN_INTERNAL to take a reference. Both count on the first frame of an
    animation, so any frame may be passed and the release always hits the
    same entry as the add; neither counts render targets, since bm_release()
    will not release those anyway. Use them in the scripting texture handle
    and the librocket rendering interface, which previously counted on
    whatever frame they were handed.
  • Add bm_page_out() (bm_unload() with a new keep_reference option) for
    dropping a bitmap's data without giving up the caller's reference, and use
    it when paging out model textures and glow bank bitmaps. The plain
    bm_unload() call used before consumed one reference without freeing when
    another holder remained, which was harmless while nothing ever released
    but would now leave the model with a dangling handle once the other
    holder did.
  • Make model_texture_replace own its entries: the array is now private, is
    written only through adopt() / reference() / clear(), and releases every
    handle it holds when destroyed. Convert every writer (mission and SEXP
    replacements, cockpit displays, scripting, qtFRED). Render targets are
    the documented exception; the cockpit display code no longer pretends to
    release its target separately.
  • Release the references that mission parsing and the texture replacement
    SEXPs take on replacement bitmaps when the parse objects are discarded;
    ships created from those objects hold their own. Previously those
    references were never released.
  • Add model_instance_load_replacement_textures() to load a table's
    replacement list into an instance by filename, and use it from
    ship_model_change, the lab, FRED, and qtFRED, which each had their own
    copy of that loop (and which each leaked the loader's references).
  • Give texture_info an optional held reference so script-set model textures
    stay alive until reset, replaced, or paged out. PageOut() now pages out
    or releases the texture the model loaded rather than whatever is
    currently drawn, which also stops the debris species swap from releasing
    a texture it does not own.
  • Have the cached UI render instances carry their ship class's replacement
    textures, loaded once in model_set_up_techroom_instance(), and add
    model_get_cached_ui_render_instance_for_class() so that the tech room,
    ship and weapon select, loadout icons, and tech model rendering fetch the
    instance and its replacements with one call instead of each building a
    fresh array with bm_load() every frame, which leaked one load count per
    frame. The instance cache is now keyed by ship class as well, since
    classes sharing a model may differ in replacement textures. As a side
    effect, those screens and the briefing closeup now load table replacement
    textures the same way missions do, so "invisible" and animated
    replacements are honored in previews where they were previously ignored.
  • Delete the briefing closeup's model instance when its icon is set up
    again or the briefing closes; previously every revisited icon leaked its
    instance, which now also pinned its replacement bitmaps.
  • Skip a cockpit display whose texture is not on the cockpit model, with a
    warning; previously this indexed the replacement array with -1.
  • Guard bmpman against static destructors that release bitmaps after an
    exit that skipped bm_close().
  • Document the texture slot layout of the "textures" and
    "modelinstancetextures" Lua handles, fix the stale TM_NUM_TYPES comment,
    and correct the gr.loadTexture docs on texture lifetime.
  • Glow bank bitmaps are now released when a model is unloaded, and support
    ships receive their class's replacement textures.

Depends on #7761; in draft until that is merged.

@Goober5000 Goober5000 added this to the Release 26.2 milestone Sep 5, 2026
@Goober5000 Goober5000 added fix A fix for bugs, not-a-bugs, and/or regressions. graphics A feature or issue related to graphics (2d and 3d) labels Sep 5, 2026
@Goober5000
Goober5000 force-pushed the fix/texture_management branch 3 times, most recently from 451cfe2 to 64dfacf Compare September 5, 2026 04:03
@Goober5000
Goober5000 marked this pull request as draft September 5, 2026 04:05
@Goober5000 Goober5000 modified the milestones: Release 26.2, Release 27.0 Sep 5, 2026
@Goober5000
Goober5000 force-pushed the fix/texture_management branch 2 times, most recently from 05c79ad to ca985b6 Compare September 5, 2026 05:08
@Goober5000
Goober5000 force-pushed the fix/texture_management branch 4 times, most recently from b7b221c to fec2170 Compare September 6, 2026 06:00
Model instance replacement arrays and script-set model textures stored raw
bitmap handles without taking a load-count reference.  A script doing
ship.Textures["foo"] = gr.loadTexture("bar") therefore left the ship pointing
at a bitmap slot that was freed as soon as Lua collected the temporary
texture handle, and reused by whatever bitmap loaded next.

- Add bm_add_ref() / bm_release_ref() to bmpman so callers no longer need
  BMPMAN_INTERNAL to take a reference.  Both count on the first frame of an
  animation, so any frame may be passed and the release always hits the
  same entry as the add; neither counts render targets, since bm_release()
  will not release those anyway.  Use them in the scripting texture handle
  and the librocket rendering interface, which previously counted on
  whatever frame they were handed.
- Add bm_page_out() (bm_unload() with a new keep_reference option) for
  dropping a bitmap's data without giving up the caller's reference, and use
  it when paging out model textures and glow bank bitmaps.  The plain
  bm_unload() call used before consumed one reference without freeing when
  another holder remained, which was harmless while nothing ever released
  but would now leave the model with a dangling handle once the other
  holder did.
- Make model_texture_replace own its entries: the array is now private, is
  written only through adopt() / reference() / clear(), and releases every
  handle it holds when destroyed.  Convert every writer (mission and SEXP
  replacements, cockpit displays, scripting, qtFRED).  Render targets are
  the documented exception; the cockpit display code no longer pretends to
  release its target separately.
- Release the references that mission parsing and the texture replacement
  SEXPs take on replacement bitmaps when the parse objects are discarded;
  ships created from those objects hold their own.  Previously those
  references were never released.
- Add model_instance_load_replacement_textures() to load a table's
  replacement list into an instance by filename, and use it from
  ship_model_change, the lab, FRED, and qtFRED, which each had their own
  copy of that loop (and which each leaked the loader's references).
- Give texture_info an optional held reference so script-set model textures
  stay alive until reset, replaced, or paged out.  PageOut() now pages out
  or releases the texture the model loaded rather than whatever is
  currently drawn, which also stops the debris species swap from releasing
  a texture it does not own.
- Have the cached UI render instances carry their ship class's replacement
  textures, loaded once in model_set_up_techroom_instance(), and add
  model_get_cached_ui_render_instance_for_class() so that the tech room,
  ship and weapon select, loadout icons, and tech model rendering fetch the
  instance and its replacements with one call instead of each building a
  fresh array with bm_load() every frame, which leaked one load count per
  frame.  The instance cache is now keyed by ship class as well, since
  classes sharing a model may differ in replacement textures.  As a side
  effect, those screens and the briefing closeup now load table replacement
  textures the same way missions do, so "invisible" and animated
  replacements are honored in previews where they were previously ignored.
- Delete the briefing closeup's model instance when its icon is set up
  again or the briefing closes; previously every revisited icon leaked its
  instance, which now also pinned its replacement bitmaps.
- Skip a cockpit display whose texture is not on the cockpit model, with a
  warning; previously this indexed the replacement array with -1.
- Guard bmpman against static destructors that release bitmaps after an
  exit that skipped bm_close().
- Document the texture slot layout of the "textures" and
  "modelinstancetextures" Lua handles, fix the stale TM_NUM_TYPES comment,
  and correct the gr.loadTexture docs on texture lifetime.
- Glow bank bitmaps are now released when a model is unloaded, and support
  ships receive their class's replacement textures.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Goober5000
Goober5000 marked this pull request as ready for review September 8, 2026 16:58
@Goober5000
Goober5000 force-pushed the fix/texture_management branch from fec2170 to a05ab5b Compare September 8, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix for bugs, not-a-bugs, and/or regressions. graphics A feature or issue related to graphics (2d and 3d)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant