Page MenuHome

Depsgraph: display a dependency cycle warning in the UI.
Needs RevisionPublic

Authored by Alexander Gavrilov (angavrilov) on Wed, Apr 24, 12:55 PM.

Details

Summary

Cycles in the dependency graph are caused by a user error, so
they should be exposed in the UI rather than just in the console.

This commit stores information about cycles in the graph object,
exposes it through C and RNA API, and adds a notification button
to the timeline header when the view layer graph has cycles.

Pressing the button pops up a menu with information about the cycles.
In the future entries of the menu can potentially jump to objects and
bones, but currently they are passive labels.

Diff Detail

Repository
rB Blender
Branch
depsgraph-cycle-popup (branched from master)
Build Status
Buildable 3568
Build 3568: arc lint + arc unit

Event Timeline

So, the UI part is a bit of a hack (using a menu and all), and has some issues:

  • The cycle info is ugly, basically using the same text as the console.
  • The button is truncated with stupid "Cyclic Dependen.." appearance.

I'd appreciate suggestions how to fix, if anybody has ideas.

I don't think the timeline is the right place for this, it's not guaranteed to be visible and also not specifically related to the dependency graph.

What I recommend to do is:

  • Report a warning message (WM_report).
    • Loading a file with dependency cycles should not show a warning message, this seems too annoying.
    • If during editing a new cycle is detected, show a warning message.
  • In the info editor, add an additional view mode so we have Log and Dependency Graph (better names may be possible). We would like to add more later (Render, GPU, Memory Usage, ..).
    • Display the dependency cycle messages in this new view mode.
    • We could have a button to open a new window with the info editor set to this view mode. A button next to the warning message seems like a good place although those messages are not persistent, it would need some tweaking.

Agree with @Brecht Van Lommel (brecht). I think that's the way to go. We already have a system for reports, so it's better to use that and make improvements to that rather than adding a separate hack inside the Timeline.

I don't think the timeline is the right place for this, it's not guaranteed to be visible and also not specifically related to the dependency graph.

Timeline is directly related to animation, which is where cycles are most visible. It also has a lot of free space in the header.

  • Report a warning message (WM_report).
    • Loading a file with dependency cycles should not show a warning message, this seems too annoying.
    • If during editing a new cycle is detected, show a warning message.

No, I've already implemented the report idea, and didn't like it at all, because the reports disappear from the status line, and you can't easily tell there's a problem from the common workspaces without specifically looking for it.

No, I've already implemented the report idea, and didn't like it at all, because the reports disappear from the status line, and you can't easily tell there's a problem from the common workspaces without specifically looking for it.

We could have a type of persistent report.

If you don't notice warning reports for some reason, we really need to fix that rather than finding a different place.

We could have a type of persistent report.

If you don't notice warning reports for some reason, we really need to fix that rather than finding a different place.

Another thing is that existence of a cycle is not an event to be reported, it is a state. Reports are explicitly designed to report events, not persistent conditions. The whole mention of "new cycle", "annoying" etc is because reports are not designed for persistent conditions in core background systems.

Another thing is that existence of a cycle is not an event to be reported, it is a state. Reports are explicitly designed to report events, not persistent conditions. The whole mention of "new cycle", "annoying" etc is because reports are not designed for persistent conditions in core background systems.

In my experience, complex production files always have some kind of issue (not just Blender, other 3D apps too). Sometimes it's serious, sometimes it's harmless, and this is often impossible to determine automatically. And the person working with the files often is not in a position to fix these warnings. That means if you have a persistent message anywhere in the UI, for example for animators who did not make the rig and it's working fine for them despite the cycle, it will become annoying.

And so in my opinion it can show in a place like the info editor, where someone who is interested in it can find it but it's not always in your face. And it can also show when someone is editing the dependencies. This is not about the report system as it exists in Blender, but general UI design around warning messages.

The argument can be made that if you have these kinds of warnings it's good to annoy users so they fix them, but in my experience this is just not practical. I'm fine with having a strict mode where such warnings or errors are persistent and prominent for those who want to work that way, but it does not seem a good default.

  • Report a warning message (WM_report).
    • Loading a file with dependency cycles should not show a warning message, this seems too annoying.
    • If during editing a new cycle is detected, show a warning message.

Also, stuff like WM_report is relying on globals. If you try to do it properly by pulling ReportList through to depsgraph code like I did in my previous attempt, you realize how many places actually can call depsgraph construction. This is not something that can be easily managed to do stuff like "if new cycle is detected" and so on.

In my experience, complex production files always have some kind of issue (not just Blender, other 3D apps too). And the person working with them often is not in a position to fix them. That means if you have a persistent message anywhere in the UI, for example for animators who did not make the rig and it's working fine for them despite the cycle, it will become annoying.
The argument can be made that if you have these kinds of warnings it's good to annoy users so they fix them, but in my experience this is just not practical.

Consider that cycles can also cause crashes, because a lot of code assumes things are good, and killing a random link in the graph basically is undefined behavior - this has happened. Any bugs with a cycle in the file are invalid and unsupported.

And so in my opinion it can show in a place like the info editor, where someone who is interested in it can find it but it's not always in your face. And it can also show when someone is editing the dependencies. This is not about the report system as it exists in Blender, but general UI design around warning messages.

By this logic, the cycles are already displayed in the console. However people submit bugs with cycles in them, so obviously nobody is going to go looking unless properly prompted in the UI.

Also, stuff like WM_report is relying on globals. If you try to do it properly by pulling ReportList through to depsgraph code like I did in my previous attempt, you realize how many places actually can call depsgraph construction. This is not something that can be easily managed to do stuff like "if new cycle is detected" and so on.

It should be possible to add some flag in the dependency graph when a new cycle is detected, and pick that up in the main event loop. We don't need to show this warning for every dependency graph, just the one that matches what the user is currently editing. Otherwise it's not very clear anyway.

Consider that cycles can also cause crashes, because a lot of code assumes things are good, and killing a random link in the graph basically is undefined behavior - this has happened. Any bugs with a cycle in the file are invalid and unsupported.

Ideally cycles should never cause crashes, we can solve this better, but yes I agree it's important for users to see them.

By this logic, the cycles are already displayed in the console. However people submit bugs with cycles in them, so obviously nobody is going to go looking unless properly prompted in the UI.

No one checks the console, I understand that and I don't think showing them there is enough. What I am suggesting is to show prominent warnings the moment you create the dependency cycle, and when you link/append and asset with dependency cycles.

We could also eventually add a way to click one the warning in the Status Bar to open the Console which would then tell the user more about what the exact issue is.

It should be possible to add some flag in the dependency graph when a new cycle is detected, and pick that up in the main event loop. We don't need to show this warning for every dependency graph, just the one that matches what the user is currently editing. Otherwise it's not very clear anyway.

First, there's no "the" dependency cycle -- every view layer and renderer has one of its own. The dependency graph itself has no idea which one it is, and it should be of no concern to it. There is also no such thing as a "new" cycle, because the graph is completely wiped and rebuilt from scratch every time. Thus, as far as the graph is concerned, all cycles are always new every time it is rebuilt.

Therefore the data has to be simply exposed from the graph to the other code, which should decide what to do with it by polling, which is exactly what this patch does: list in the graph -> C API -> Python API -> Timeline header polls current view_layer graph and shows a warning button.

Choosing the Timeline is rather random, but it is less crowded than the status bar itself, and is present on most workspaces suitable for animation & rigging.

Moved the button to the status bar, because I agree that putting it in the Timeline is sort of random.

First, there's no "the" dependency cycle -- every view layer and renderer has one of its own. The dependency graph itself has no idea which one it is, and it should be of no concern to it.

There is usually one active view layer, maybe multiple if different windows have different view layers. I don't think it's an issue to show warnings only for those, the current patch already uses the active view layer dependency graph in the timeline.

There is also no such thing as a "new" cycle, because the graph is completely wiped and rebuilt from scratch every time. Thus, as far as the graph is concerned, all cycles are always new every time it is rebuilt.

The dependency graph data structure and CoW datablocks are persistent even when rebuilding relations. Either way this seems like an implementation detail.

The way I imagine it working is the first time you build relations for a dependency graph, it doesn't warnings any errors in the UI. Then if you rebuild it would detect if any new warnings were added to the list, and report if so.

Therefore the data has to be simply exposed from the graph to the other code, which should decide what to do with it by polling, which is exactly what this patch does: list in the graph -> C API -> Python API -> Timeline header polls current view_layer graph and shows a warning button.

Right, I'm proposing to do this polling in the main event loop and show it in the status bar instead.

Choosing the Timeline is rather random, but it is less crowded than the status bar itself, and is present on most workspaces suitable for animation & rigging.

Still, this kind of information is exactly what the status bar was added for.

  • As mentioned before, now the cycle list menu is in the status bar.
  • Fixed the width of menu buttons with icons to avoid '...'.
  • Exposed detailed information about the nodes involved in the cycle, and used it in python to make the items readable, and make them jump to objects and bones when clicked (using the Jump To Target code).

I think we should ideally tie this into the reports system, and not add a separate menu thing. But I also get that the reports system may need a larger overhaul then.

Currently it looks like this:

To me it seems probably ok. The feature is obviously very useful, it seems a little arbitrary that it's separate from the system we already have for reporting issues and warnings.

source/blender/editors/interface/interface_layout.c
306–315

Logic here gets a bit cryptic, suggest:

float margin = compact ? 1.25 : 1.50;
if (icon) {
  margin += compact ? 0.35 : 0.25;
}
return (UI_fontstyle_string_width(fstyle, name) + (unit_x * margin));

...with comments about why it's needed.

Rebased and added filtering of uninteresting lines to reduce noise
(i.e. sequences of jumps between operations inside the same bone)

Brecht Van Lommel (brecht) requested changes to this revision.Mon, May 13, 4:02 PM

While this is a useful feature, we discussed this in the UI meeting and think it should better integrate into the user interface before it can be committed. We don't have the time for that for 2.80, if you can do it that would be fine if it can be done this week, otherwise this moves to 2.81.

That means:

  • Display in the info editor as a separate tab. Displaying the information in the info editor might is not a big overhaul, if we change CTX_wm_reports(C) to a different ReportList depending on a view mode. Opening a window with such an editor is also possible without big changes.
  • Warnings should not be permanent at all. When a new warning is introduced it can report a message. Maybe showing a single icon somewhere in the statusbar without no coloring could be ok, but it should not grab attention.
This revision now requires changes to proceed.Mon, May 13, 4:02 PM

This could work with a separate ReportList for 'current dependency issues' that is wiped clean every time depsgraph is recalculated, and if reports could be enhanced to support multiple text lines, and ideally with each one optionally annotated with an object or bone reference for convenient jumping to it.

That separate report list could also be used to detect 'new' issues like you wanted, by comparing the newly computed list with the previous one before replacing it.

Dependency graph code can also potentially alert about other reference-related issues, like bad drivers or animation curves, so making it more generic may be useful.