hold bitmap references for replacement and script-set textures - #7763
Open
Goober5000 wants to merge 1 commit into
Open
Goober5000 wants to merge 1 commit into
Goober5000 wants to merge 1 commit into
Conversation
Goober5000
force-pushed
the
fix/texture_management
branch
3 times, most recently
from
September 5, 2026 04:03
451cfe2 to
64dfacf
Compare
Goober5000
marked this pull request as draft
September 5, 2026 04:05
Goober5000
force-pushed
the
fix/texture_management
branch
2 times, most recently
from
September 5, 2026 05:08
05c79ad to
ca985b6
Compare
Goober5000
force-pushed
the
fix/texture_management
branch
4 times, most recently
from
September 6, 2026 06:00
b7b221c to
fec2170
Compare
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
marked this pull request as ready for review
September 8, 2026 16:58
Goober5000
force-pushed
the
fix/texture_management
branch
from
September 8, 2026 17:04
fec2170 to
a05ab5b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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.
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.
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.
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.
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).
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.
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.
again or the briefing closes; previously every revisited icon leaked its
instance, which now also pinned its replacement bitmaps.
warning; previously this indexed the replacement array with -1.
exit that skipped bm_close().
"modelinstancetextures" Lua handles, fix the stale TM_NUM_TYPES comment,
and correct the gr.loadTexture docs on texture lifetime.
ships receive their class's replacement textures.
Depends on #7761; in draft until that is merged.