Make Gizmos compatible with FixedUpdate

[SpecificProtagonist]

Superseded by #10973

Objective

Gizmos are immediate mode and thus last only for one frame. When drawing them during FixedUpdate, multiple frames may be drawn before the next fixed update, causing them to flicker.

Solution

Record fixed update gizmos separately and keep them around till the next update. This also works if there are multiple fixed updates in a single frame.

Also expose GizmoBuffer for any custom scheduling not covered by this.

Alternatives

#10973 achieves the same by adding a generic parameter with a default value to Gizmos. While technically less flexible for extension as users can only store gizmo buffers based on their type instead of as values, I think that's sufficient and is a solution I'd be happy with.

---

Changelog

Added

• GizmoBuffer for recording gizmo draw calls

Fixed

• fixed gizmos flickering is drawn in FixedUpdate

[aevyrie]

Wouldn't it make more sense to have a retained API for gizmos, e.g. Handle<Polyline>, for cases like this?

[Aceeri]

Wouldn't it make more sense to have a retained API for gizmos, e.g. Handle<Polyline>, for cases like this?

I would say no mostly because I use these things as quick debug tools. Retained mode would add a lot of boilerplate here. The goal is just to have gizmos function intuitively in the fixed update.

[rlidwka]

You are only fixing issue for fixed update, it wouldn't work for custom schedules, run_if conditions, and so on.

Perhaps gizmos should be retained {until next system run OR until 1 second is passed}? This solution would make gizmos work in all systems. But I'm not sure about performance implications of this approach.

[SpecificProtagonist]

Perhaps gizmos should be retained {until next system run OR until 1 second is passed}? This solution would make gizmos work in all systems. But I'm not sure about performance implications of this approach.

Putting aside any performance implications for now, that would mean gizmos persist after they're not wanted anymore (and would create an unintuitive distinction between not running the system and running the system but doing nothing inside).

You are only fixing issue for fixed update, it wouldn't work for custom schedules, run_if conditions, and so on.

To draw gizmos, you need to draw them once per 'frame' – this PR makes this work for both frame meaning "once per Render schedule" and "once per FixedUpdate schedule". If during a frame you use run_if to not run a system, e.g. debug_grid.run_if(in_state(DebugState::On), not persisting the gizmos rendered by that system is the correct choice and the same behavior as with an if inside the system. When writing custom schedules, if your concept of 'frame' is a fixed update, you'd run these schedule inside FixedUpdate. When these custom schedules implement something similar to FixedUpdate without building on top of it, they can manage the FixedUpdateScheduleIsCurrentlyRunning resource themself – I should probably document this better.

The only problem is when such schedules with their concept of 'frame' aren't build on FixedUpdate but FixedUpdate is also used. I think a good solution would be to give public access to building GizmosBuffers with the existing gizmos drawing APIs and then letting users enqueue said buffers when they want. This would be both flexible and easy to use.

[SpecificProtagonist]

public access to building GizmosBuffers with the existing gizmos drawing APIs

Implemented.

[SpecificProtagonist]

This PR is tagged for 0.11.1, but it might be worth just including the fix for FixedTime (4acb25a) and leaving the more general solution of exposing GizmoBuffer for 0.12.

Edit: Not anymore.

[mnmaita]

Leaving minor suggestions and typos on some doc comments.

[alice-i-cecile]

Wouldn't it make more sense to have a retained API for gizmos, e.g. Handle<Polyline>, for cases like this?

This is sort of the direction that I lean towards. I don't think you should be drawing things to the screen during FixedUpdate at all, especially with an immediate mode API.

[SpecificProtagonist]

I don't think you should be drawing things to the screen during FixedUpdate at all, especially with an immediate mode API.

Could you clarify why?

This is sort of the direction that I lean towards.

I don't like it that things work in some cases but not in others. Why should you be able to use Gizmos normally in a variable-time game, but if you decide to switch to fixed time have it stop working? Or do you mean there shouldn't be an immediate mode API at all? In that case I agree with Aceeri.
Say I want to add a debug indicator when an enemy fails to find a path. With immediate mode:

• Add Gizmos system parameter
• When pathfinding fails, draw the indicator

With retained mode with Handles:

• Add a PathingFailedIndicator component
• Add Gizmos system parameter
• When pathfinding fails, create a handle & store it with the component
• either
  • Remove this component from all entities at the end of each tick, effectively recreating immediate mode but with worse performance
  • Every tick, somehow try to find out which enemies don't try to pathfind (sleeping, too far away, …) and remove the component from them as well as from every enemy that successfully pathfinds. Depending on how positioning works, I might also need to update the position each tick.
• This is assuming all gizmos for which a handle is live are drawn, but that would be inconsistent with Bevy's existing retained mode API. If it instead needs a special component (which is how it works with sprites), this would necessitate also spawning and managing an extra entity with that component.

[aevyrie]

This is adding retained mode behavior to an immediate mode API. What if someone uses this expecting the gizmo to draw only on the frames the fixedupdate is running? That's how I would expect this to work, and this change would prevent that. Like, lets say I want to find the exact frame my updates are happening, visually, I would expect the gizmo would only display for the rendered frame the fixed update system is running. I could then record my screen to visually debug precisely when updates are happening.

You can make gizmos work the way you want without any changes:

// scheduled on a fixed update
fn update_pathfinding(mut pathfinding: ResMut<Pathfinding>) {}

// normally scheduled system
fn debug_pathfinding(mut gizmos: Gizmos, pathfinding: Res<Pathfinding>) {}

[Aceeri]

This is adding retained mode behavior to an immediate mode API. What if someone uses this expecting the gizmo to draw only on the frames the fixedupdate is running? That's how I would expect this to work, and this change would prevent that. Like, lets say I want to find the exact frame my updates are happening, visually, I would expect the gizmo would only display for the rendered frame the fixed update system is running.

The problem I have with this is that now you have boilerplate and friction to just debugging functionality. I can't just plop a line of code into my gameplay systems and debug it similar to a info!(...).

The way I see it is FixedUpdate is where basically all gameplay systems should be, this makes it significantly easier to debug interactions/have deterministic state/network/etc. Gizmos are, in my opinion, the most useful debugging tool in bevy as of right now. Not having it work well in FixedUpdate chips away at the usability of the system set, just as with Events and Time.

I could then record my screen to visually debug precisely when updates are happening.

I'm unsure of the point of that... Even in networking situations I am not using visual feedback.

[alice-i-cecile]

Ultimately, drawing things, especially in immediate mode, in FixedUpdate is fundamentally questionable. I think that your idea of "just use it like dbg" is good (and that "gameplay systems should be in FIxedUpdate by default"), but there are some real challenges here.

FixedUpdate's behavior is uniquely strange, and with good reason. It may run once, zero or many times for a single frame. Once works fine out of the box. Your solution handles zero times, at the cost of substantial complexity. But what about multiple times? Your solution (and any non-retained solution) will then render multiple copies of the gizmos on the screen simultaneously, if only for a second.

Ultimately, I think that the best solution here is to say "sorry, you can't use immediate mode drawing APIs within FixedUpdate", and then clearly document why and the workaround (split it out into another system that runs in Update).

[Aceeri]

Ultimately, drawing things, especially in immediate mode, in FixedUpdate is fundamentally questionable. I think that your idea of "just use it like dbg" is good (and that "gameplay systems should be in FIxedUpdate by default"), but there are some real challenges here.

FixedUpdate's behavior is uniquely strange, and with good reason. It may run once, zero or many times for a single frame. Once works fine out of the box. Your solution handles zero times, at the cost of substantial complexity. But what about multiple times? Your solution (and any non-retained solution) will then render multiple copies of the gizmos on the screen simultaneously, if only for a second.

This doesn't render multiple copies of gizmos, it would clear each FixedUpdate frame, just as normal gizmos get cleared each rendering frame.

Ultimately, I think that the best solution here is to say "sorry, you can't use immediate mode drawing APIs within FixedUpdate", and then clearly document why and the workaround (split it out into another system that runs in Update).

What if we just replicate the Time<Fixed> stuff here? I don't see why the workaround can't just be the solution here. I also see it as immediate mode doesn't need to be just restricted to rendering frames. Why not gameplay ticks as well.

[SpecificProtagonist]

This came up in Discord: #8964 (which Aceeri referred to above) is similar in that it abstracts over different clocks.

When that's merged, this PR could build upon it to do the same: Gizmos<Virtual> would render until the next variable-time update, Gizmos<Fixed> until the next fixed-time update (and so on for custom clocks), and using just Gizmos would mean the system works correctly regardless of whether it's in update or fixedupdate. This would fix the issue while also addressing the concerns raised here.