Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update system requirements #3058

Closed
Durman opened this issue Apr 11, 2020 · 17 comments · Fixed by #4164
Closed

Update system requirements #3058

Durman opened this issue Apr 11, 2020 · 17 comments · Fixed by #4164
Labels
core Architectural problems high priority
Milestone

Comments

@Durman
Copy link
Collaborator

Durman commented Apr 11, 2020

I'm going to create list of desirable features which new update system should include. What can be included in this list is not obvious and can be discussed.

  • Performance issue.:

    • Don't recalculate what already was calculated:

      • Nodes should keep results of their calculations.
      • All changes (in parameters, inputs, data from Blender scene ...) should be tracked.
      • Manually prohibition of recalculation (freezing mode)
    • Don't calculate if result is need for nobody:

      • Create output nodes which cause work of other nodes (viewers).
      • Mode for output nodes switch on/off of showing of calculation results
  • User should understand what's going on:

    • Use annotations for showing error and other messages.
    • Color system for showing error nodes and different modes or whatever.
    • Showing performance information.
  • Performance of workflow:

    • supporting of node subtrees
    • pass data without changes throu node (bypass mode. It is not possible for all nodes)

related: #2770 #2380 #1934 #1990

@Durman
Copy link
Collaborator Author

Durman commented May 5, 2020

  • User should understand what's going on:

node statistics

@vicdoval
Copy link
Collaborator

vicdoval commented May 5, 2020

Really nice!
No sure the recalculations number is that useful, I guess after 3 minutes tweaking the node-tree it must be a very high number...

@Durman
Copy link
Collaborator Author

Durman commented May 6, 2020

I have found that Viewer Draw nodes effect on each other performance. For example first node update time is 1s. Then another node will update faster if the first node will be disabled. I think there should be possibilities for performance improving.

BmeshViewer has similar performance but they does not effect on each other. In this way they are faster.

@enzyme69
Copy link
Collaborator

enzyme69 commented May 6, 2020 via email

@Durman
Copy link
Collaborator Author

Durman commented May 7, 2020

I'm thinking about workflow where error nodes returns their previous result so the rest of a tree still could do something.
ignore error

@Durman
Copy link
Collaborator Author

Durman commented May 7, 2020

Tool which can help to find nodes with slow performance. All nodes with update time more than given value will be dyed into a color.
slow nodes

Also it has mode of coloring last updated nodes. Probably more for debugging purposes.
updated nodes

@portnov
Copy link
Collaborator

portnov commented May 7, 2020

It seemed to me there already was such coloring... "heat map" or smth like that.

@Durman
Copy link
Collaborator Author

Durman commented May 7, 2020

Yes, I know.

@Durman Durman mentioned this issue May 7, 2020
29 tasks
@Durman
Copy link
Collaborator Author

Durman commented May 8, 2020

I have managed to connect to tree origin.
cool bug
https://developer.blender.org/T76538

@Durman
Copy link
Collaborator Author

Durman commented May 9, 2020

2020-05-09_08-58-06

@Durman
Copy link
Collaborator Author

Durman commented May 17, 2020

I'm thinking about show this layout button.
2020-05-17_11-59-38

What I would expect from the button is to switch on/off all what is drawing in 3d space by current node tree. I see two ways to implement this.

First is where draw nodes do not know about the property and update system make decisions about whether to draw mesh or not by itself. But in this case the nodes should grant extra information about themselves. All output nodes can be splitted at least into two categories which are drawing something in 3d scene and other which can draw in node tree or do other stuff. So only nodes which are drawing into 3d scene should be affected by the property. And in this case the nodes should have interface for switching them on/off so update system could use it.

Second variant is where draw nodes do know about the property. The decision about drawing in 3d space should made inside each draw node separately. It is good in case if a node should have some complex logic on this matter and bad because all nodes should be aware the draw property of a tree what can be easily be forgotten in new draw nodes. Also in this case the option should have its own handler which would search all draw in 3d nodes in a tree and make them switch off/on.

What I'm certain for sure is that update system should know about the draw property of a tree and the draw properties of output nodes so it could make decision about whether it worth to calculate tree for particular output node.

@Durman
Copy link
Collaborator Author

Durman commented May 17, 2020

I have came to the next decision. Most simple way will be to have two properties for nodes.

  • is_active_output is property which all nodes will have by default with value False. It should be overridden by outputs nodes. They can return true or false dependently their own condition. This property will help to update system recalculate tree more efficiently.
  • hide_viewport property will serve two purposes. It will let know to update system that purpose of a node with this property is to draw in viewport because only such nodes will have this property. And it will let to update system to toggle hide property of objects which are owned by nodes.

Whole process should looks next. Update system will get signal that the property of a tree was changed. If the property is hide it will find all nodes with hide_viewport property and toggle them. If the property is show it will find all nodes before output nodes and check their update status. If update status is outdated it will reevaluate the tree and then toggle hide_viewport property of output nodes.

@Durman
Copy link
Collaborator Author

Durman commented May 17, 2020

If calculate output is too expensive it will be possible to disable output node and set properties without delays.

updating

@nortikin nortikin added this to the core milestone Jul 15, 2020
@nortikin nortikin linked a pull request Jul 15, 2020 that will close this issue
29 tasks
@nortikin nortikin pinned this issue Jul 15, 2020
@Durman Durman removed a link to a pull request Oct 5, 2020
29 tasks
@Durman
Copy link
Collaborator Author

Durman commented Oct 16, 2020

This module would help if Blender will ever use Python 3.9
https://docs.python.org/3/library/graphlib.html

@portnov
Copy link
Collaborator

portnov commented Oct 16, 2020

At the moment we have sverchok.utils.topo module, which implements "stable topological sorting". As far as I remember, id does not raise exception in case of cycles, just "tears them apart" somehow.

@Durman
Copy link
Collaborator Author

Durman commented Oct 16, 2020

Would be nice if the code could have any documentation

@Durman
Copy link
Collaborator Author

Durman commented Oct 28, 2020

I think new update system should be as easy replaceable as possible according experience based on current update system.

class AbstractHandler(ABC):
    @abstractmethod
    def send(self, event): ...


class ConcreteHandler(AbstractHandler):
    def send(self, event):
        if event.type == 'TreeUpdate':
            update_tree()


class Tree:
    handler = ConcreteHandler()

    def update(self, context):
        handler.send(Event(type='TreeUpdate'))

In this case if someone decides to create another update system the doing next will be enough.

class AnotherConcreteHandler(AbstractHandler):
    def send(self, event):
        if event.type == 'TreeUpdate':
            update_tree_differently()


class Tree:
    handler = AnotherConcreteHandler()

It will be even possible to have several update systems if needed switching between them via addon properties for example.

@Durman Durman mentioned this issue Jun 12, 2021
16 tasks
@Durman Durman unpinned this issue Jul 12, 2021
@Durman Durman added the core Architectural problems label Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Architectural problems high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants