FosterFramework / Foster

A small C# game framework
MIT License
422 stars 37 forks source link

SpriteFontBlitModes.Streaming deteriorates to spin lock in BlitModule.Update #91

Closed MrBrixican closed 2 months ago

MrBrixican commented 2 months ago

SpriteFontBlitModes.Streaming can lead to spin lock behavior in BlitModule.Update.

Specifically this code in SpriteFont.cs:

public override void Update()
{
    uploadTimer.Restart();

    // find all finished tasks
    while (runningTasks.TryDequeue(out var task))
    {
        if (!task.IsCompleted)
        {
            runningTasks.Enqueue(task);
            continue;
        }
        else if (uploadTimer.ElapsedMilliseconds > MaximumMillisecondsPerFrame)
        {
            runningTasks.Enqueue(task);
            break;
        }
        else
        {
            ...

The first if will be hit if there are any incomplete tasks. The else if will never be hit until all tasks are complete, essentially ignoring the purpose of MaximumMillisecondsPerFrame. Granted, this is only an issue if a character takes longer than 3 ms to blit, which is relatively unlikely in practice on powerful hardware, but can be simulated by adding a Thread.Sleep in the BlitTask.

A simple swap of the first and second conditions should solve the issue.

Also, as an aside, shouldn't it be >= MaximumMillisecondsPerFrame since ElapsedMilliseconds is a long so the actual maximum being enforced is currently MaximumMillisecondsPerFrame + 1.

NoelFB commented 2 months ago

Oh good call, I missed this, thanks!

In general I think the whole thing could still be cleaned up a lot ... once SDL GPU is implemented, allowing texture writes in threads, all of this code will be a lot simpler.