Rewrite Notifier System (2.8) #53308

New Issue

Campbell Barton · 2017-11-13T04:30:31+01:00

Campbell Barton commented

2017-11-13 04:30:31 +01:00

Currently notifiers types are hard coded, where listeners make assumptions about the content each region shows.

This fails when the contents of a window is defined by add-ons, or defined dynamically.

Typical cases are:

Add-ons expose buttons for properties not accessed by default for those regions.
Manipulators in the view-port may control properties which users will expect to be refreshed when changed elsewhere (2.8 only).

I'm proposing to update this system with a dynamic and more general system using the publish-subscribe pattern.

This task was prompted by manipulators not responding updates to changes elsewhere, currently there are many noticeable glitches.

Process Overview

As the interface is constricted (manipulators and properties), the region subscribes to changes from the properties.
Upon modification, changes to the property are published. (RNA property update)
Subscribers are notified of the change (their callbacks are executed).

Details

RNA properties will be one type of message,

we may want to use this system for listening to other kinds of changes
(listening to file-system message for example).

I expect RNA property messages cover most cases,
keeping option open to have other kinds of messages just helps to be future proof.

There is some overlap with bpy.app.handlers since it should be possible to listen to changes to

`Scene.frame_current` which is similar to `frame_change_post` handler.

The difference is this system is intended for refreshing the interface after a change - and they should never
make destructive changes,
also - unlike `bpy.app.handlers` you wont have pre/post conditions.

It's not well defined how granular publish/subscribe messages should be.
- Only publishing changes from ID data-blocks is too course.
- Publishing changes to individual vertices is too fine grained.

I'd suggest to make array-elements an exception,
where an (ID-data-block + mesh-vertex-property) pair could be used,
avoiding the issue of treating each vertex separately.

There is still some areas that aren't well defined:
how should we handle modifiers or fcurve properties for example
as a single unit, or handle each property separately.

This can be changed as we go, so I'm not too worried about this.

It may be useful for Python to be able to subscribe to messages too.

There is nothing to prevent this from being supported, however it's not part of this initial proposal.

Challenges

Performance: keeping the overhead of maintaining the messages low.

Each full-redraw will create a set of data to listen out for -
which need to be stored for fast matching in the case a change is made.

I'm mostly concerned with the overhead of allocation, hashing and de-duplicating.

This may not end up being slower overall than our current notifier system, which redraws often *just-incase*
a change elsewhere is needed.

Nevertheless there is a difference with the old notifier system,
where the inefficiencies happen when listening to changes instead of when drawing buttons and manipulators.

Migrating from the existing notifier system.

This will be kept in-place initially, we should be able to phase it out.

Exactly how the 3D view should subscribe to changes from mesh/lamp... etc.

We may end up needing to listen to changes on a per-datablock level in this case.

Even then - if there are 1000 objects in the view. Would this add 1000 listeners on each redraw?

Suggest in this case we don't attempt to track individual ID's
the draw-manager for each 3D view can listen to a fixed set of properties with `PointerRNA.id.data` set to NULL,
with the down-side that changes to an object will cause a redraw even if it's not in the view.

This is at least no worse than the current system.

As with the current notifier system, we'll need to make sure stale data is cleared.

Currently notifiers types are hard coded, where listeners make assumptions about the content each region shows. This fails when the contents of a window is defined by add-ons, or defined dynamically. Typical cases are: - Add-ons expose buttons for properties not accessed by default for those regions. - Manipulators in the view-port may control properties which users will expect to be refreshed when changed elsewhere (2.8 only). I'm proposing to update this system with a dynamic and more general system using the publish-subscribe pattern. This task was prompted by manipulators not responding updates to changes elsewhere, currently there are many noticeable glitches. ### Process Overview - As the interface is constricted (manipulators and properties), the region subscribes to changes from the properties. - Upon modification, changes to the property are published. (RNA property update) - Subscribers are notified of the change (their callbacks are executed). ### Details - RNA properties will be one type of message, ``` we may want to use this system for listening to other kinds of changes (listening to file-system message for example). ``` ``` I expect RNA property messages cover most cases, keeping option open to have other kinds of messages just helps to be future proof. ``` - There is some overlap with `bpy.app.handlers` since it should be possible to listen to changes to ``` `Scene.frame_current` which is similar to `frame_change_post` handler. ``` ``` The difference is this system is intended for refreshing the interface after a change - and they should never make destructive changes, also - unlike `bpy.app.handlers` you wont have pre/post conditions. ``` - It's not well defined how granular publish/subscribe messages should be. - Only publishing changes from ID data-blocks is too course. - Publishing changes to individual vertices is too fine grained. ``` I'd suggest to make array-elements an exception, where an (ID-data-block + mesh-vertex-property) pair could be used, avoiding the issue of treating each vertex separately. ``` ``` There is still some areas that aren't well defined: how should we handle modifiers or fcurve properties for example as a single unit, or handle each property separately. ``` ``` This can be changed as we go, so I'm not too worried about this. ``` - It may be useful for Python to be able to subscribe to messages too. ``` There is nothing to prevent this from being supported, however it's not part of this initial proposal. ``` ### Challenges - Performance: keeping the overhead of maintaining the messages low. ``` Each full-redraw will create a set of data to listen out for - which need to be stored for fast matching in the case a change is made. ``` ``` I'm mostly concerned with the overhead of allocation, hashing and de-duplicating. ``` ``` This may not end up being slower overall than our current notifier system, which redraws often *just-incase* a change elsewhere is needed. ``` ``` Nevertheless there is a difference with the old notifier system, where the inefficiencies happen when listening to changes instead of when drawing buttons and manipulators. ``` - Migrating from the existing notifier system. ``` This will be kept in-place initially, we should be able to phase it out. ``` - Exactly how the 3D view should subscribe to changes from mesh/lamp... etc. ``` We may end up needing to listen to changes on a per-datablock level in this case. ``` ``` Even then - if there are 1000 objects in the view. Would this add 1000 listeners on each redraw? ``` ``` Suggest in this case we don't attempt to track individual ID's the draw-manager for each 3D view can listen to a fixed set of properties with `PointerRNA.id.data` set to NULL, with the down-side that changes to an object will cause a redraw even if it's not in the view. ``` ``` This is at least no worse than the current system. ``` - As with the current notifier system, we'll need to make sure stale data is cleared.

Campbell Barton self-assigned this 2017-11-13 04:30:31 +01:00

Campbell Barton commented

2017-11-13 04:30:31 +01:00

Changed status to: 'Open'

Campbell Barton commented

2017-11-13 04:30:31 +01:00

Added subscribers: @ideasman42, @mont29, @dfelinto, @Sergey

Joshua Leung commented

2017-11-14 01:22:15 +01:00

Added subscriber: @JoshuaLeung

Brecht Van Lommel commented

2017-11-14 02:28:50 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2017-11-14 02:28:50 +01:00

Drivers can cause a change in one datablock to update a property of any other datablock, and only the depsgraph is aware of this relation. So I think it should be involved? Just the existing depsgraph is not sufficient to replace the notifier system, since it does not cover selection. The depsgraph tagging could include selection changes on datablocks, or the depsgraph could feed into the notifier system and selection changes would feed into it separately.

Personally I've always though the depsgraph should replace the non-UI part of the notifier system, especially now that is more granular than before. But I'm not sure @Sergey agrees with this.

Listening to changes from specific datablocks is tricky. You also need to listen to when that datablock changes to another datablock, and that depends on context, selection, layers, editor settings, etc. In principle you can get faster redraws if you have two editors displaying different datablocks of the same type, and redrawing just one. In practice the complexity of getting this right may not be worth it, and I think it would be better to just handle notifiers per datablock type.

Drivers can cause a change in one datablock to update a property of any other datablock, and only the depsgraph is aware of this relation. So I think it should be involved? Just the existing depsgraph is not sufficient to replace the notifier system, since it does not cover selection. The depsgraph tagging could include selection changes on datablocks, or the depsgraph could feed into the notifier system and selection changes would feed into it separately. Personally I've always though the depsgraph should replace the non-UI part of the notifier system, especially now that is more granular than before. But I'm not sure @Sergey agrees with this. Listening to changes from specific datablocks is tricky. You also need to listen to when that datablock changes to another datablock, and that depends on context, selection, layers, editor settings, etc. In principle you can get faster redraws if you have two editors displaying different datablocks of the same type, and redrawing just one. In practice the complexity of getting this right may not be worth it, and I think it would be better to just handle notifiers per datablock type.

Joshua Leung commented

2017-11-14 02:37:19 +01:00

@brecht makes a good point here about involving the depsgraph more here. With the batch_cache_dirty() stuff that a lot of datablocks now have to do to get selections redrawn in the 3D view, it makes sense to consider just having all that type of tagging being done via the depsgraph instead of having everything doing this manually. Furthermore, IMO it makes sense to give the depsgraph dedicated opcodes for this (e.g. a DEG_OPCODE_GEOMETRY_BATCHCACHE_UPDATE for general full update, and/or DEG_OPCODE_GEOMETRY_SELECT_UPDATE) since we've currently got a bunch of different datatypes doing these updates in the geometry ubereval.

@brecht makes a good point here about involving the depsgraph more here. With the `batch_cache_dirty()` stuff that a lot of datablocks now have to do to get selections redrawn in the 3D view, it makes sense to consider just having all that type of tagging being done via the depsgraph instead of having everything doing this manually. Furthermore, IMO it makes sense to give the depsgraph dedicated opcodes for this (e.g. a `DEG_OPCODE_GEOMETRY_BATCHCACHE_UPDATE` for general full update, and/or `DEG_OPCODE_GEOMETRY_SELECT_UPDATE`) since we've currently got a bunch of different datatypes doing these updates in the geometry ubereval.

Sergey Sharybin commented

2017-11-14 10:45:00 +01:00

@brecht, dependency graph should indeed replace the non-interface part, but as far as i can tell here the scope is about manipulators which are exactly part of the interface. Interface MIGHT use some tagging information from the dependency graph, but then you run really quickly into chicken-egg issues because tags flush happens after notifiers are handled.

@JoshuaLeung, from the very beginning i've kept saying that using notifier system for batch caches is a no-go solution. Not sure what is the reason why it was ignored, and guess i'd just need to go ahead and clean up the mess here.

@brecht, dependency graph should indeed replace the non-interface part, but as far as i can tell here the scope is about manipulators which are exactly part of the interface. Interface **MIGHT** use some tagging information from the dependency graph, but then you run really quickly into chicken-egg issues because tags flush happens after notifiers are handled. @JoshuaLeung, from the very beginning i've kept saying that using notifier system for batch caches is a no-go solution. Not sure what is the reason why it was ignored, and guess i'd just need to go ahead and clean up the mess here.

Joshua Leung commented

2017-11-14 11:18:12 +01:00

@Sergey Whatever you do here, I just hope we get the gp branch merged before you do it :P

Campbell Barton commented

2017-11-15 08:30:24 +01:00

I wasn't sure if drivers were relavant to the discussion since IIRC drivers won't modify the original data with COW - data blocks (if we want to have the same object in different states in multiple workspaces).

I think this system is useful even without ID-datablock / viewport integration, since it can be used for manipulators and UI updates. So will focus on getting this working for now, later we can look into data-block updating.

I wasn't sure if drivers were relavant to the discussion since IIRC drivers won't modify the original data with COW - data blocks (if we want to have the same object in different states in multiple workspaces). I think this system is useful even without ID-datablock / viewport integration, since it can be used for manipulators and UI updates. So will focus on getting this working for now, later we can look into data-block updating.

Brecht Van Lommel commented

2017-11-15 18:10:32 +01:00

Drivers don't modify the original data with the new depsgraph anymore I guess. Maybe we want to keep showing the modified values in the UI anyway, but that's more a design topic. Regardless, editors like the 3D view or UV editor display not just original data.

I agree this doesn't need to be tackled immediately, just wanted to give some feedback since datablocks were mentioned in the task description.

Drivers don't modify the original data with the new depsgraph anymore I guess. Maybe we want to keep showing the modified values in the UI anyway, but that's more a design topic. Regardless, editors like the 3D view or UV editor display not just original data. I agree this doesn't need to be tackled immediately, just wanted to give some feedback since datablocks were mentioned in the task description.

Campbell Barton commented

2017-11-23 10:40:00 +01:00

How should object properties be handled that only impact overlay information?

eg, object name, bounds, camera limits, mesh wire.

AFAICS these shouldn't need a depsgraph recalculation, only a redraw, so might be better handled by a notifier system.

edit: Discussed with @mont29, we agree having the 3d view manially subscribe to too many fine-grain properties is probably not worth it. We can simply have the viewport subscribe to object and object-data type changes (mesh, camera, lamp etc).

If we want later, we can make them find-grained or use some other solution.

How should object properties be handled that only impact overlay information? - eg, object name, bounds, camera limits, mesh wire. AFAICS these shouldn't need a depsgraph recalculation, only a redraw, so might be better handled by a notifier system. edit: Discussed with @mont29, we agree having the 3d view manially subscribe to too many fine-grain properties is probably not worth it. We can simply have the viewport subscribe to object and object-data type changes (mesh, camera, lamp etc). If we want later, we can make them find-grained or use some other solution.

Campbell Barton commented

2017-12-04 10:31:39 +01:00

Committed to 2.8x branch, closing.

Campbell Barton commented

2017-12-04 10:31:54 +01:00

Changed status from 'Open' to: 'Resolved'

Campbell Barton closed this issue

2017-12-04 10:31:54 +01:00

Ray Mairlot commented

2018-01-10 16:43:24 +01:00

Added subscriber: @RayMairlot

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Rewrite Notifier System (2.8) #53308

Process Overview

Details

Challenges