SnowyMouse / chimera

The update to Halo PC that never was
https://chimera.opencarnage.net
GNU General Public License v3.0
137 stars 26 forks source link

Fixed crash due to rendering stale text entries #41

Closed surrealwaffle closed 3 years ago

surrealwaffle commented 3 years ago

This PR solves a crash related to text output for fullscreen instances of Halo with background_playback active, triggered after the user tabs back in from being tabbed out across a map load. This PR may address #24, as #39 solves the other half of the puzzle. However, I have been unable to crash from only being tabbed out across a map load - I have always needed to tab back in to trigger it; the crash reported may be of a different source.

Explanation of the Crash

With background_playback active, Halo attempts to render the frame even if the application does not have focus. If the video device could not be acquired, no rendering work takes place.

However, preframe and frame events fire on the attempt to draw the frame (before and after, respectively), while on_text() is called only when rendering work is being done for that frame. One can observe this behavior by logging when these events occur.

In a tabbed, fullscreen instance with background_playback active, the video device is lost (and is not reacquired until the instance regains focus due to #39), so no rendering work is performed. In this situation, preframe and frame events fire, but on_text() is not invoked. The custom chat system adds text to display for that frame as a preframe event, but that list of entries is only ever cleared when on_text() is called. This causes text entries to accumulate in text_list until Halo regains focus and on_text is called.

Entries in text_list store a TagID for the font tag used to display that text. When a user tabs back in through a map load to a different map, the tag IDs for some of these entries may be for a previous map and may result in a crash or abnormal behavior when Halo interprets the tag as a font tag.

This behavior does not present itself when background_playback is disabled, because preframe and frame events do not fire when tabbed out under the same conditions.

The Patch

A quick change to prevent entries from accumulating in text_list is to add a frame event that clears text_list. This is what I have opted for.

However, this issue brings into question if preframe and frame events should fire only when rendering work is performed, or if new event handlers should be created that meet that behavior.