-
Notifications
You must be signed in to change notification settings - Fork 531
Refactor visibility_detector to avoid forcing compositing. #367
Changes from 1 commit
6688c84
a81e311
7d21eaa
4abf7ca
c45a66a
9cbdb1b
d002ed3
13c3bde
b5e43fd
88f2b70
d88c017
9ac9fbe
eeb4d13
4491555
76f825b
af74e16
1ba8777
1058986
16f1ee1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,7 @@ mixin RenderVisibilityDetectorBase on RenderObject { | |
| return _updates.length; | ||
| } | ||
|
|
||
| static Map<Key, RenderVisibilityDetectorBase> _updates = | ||
| <Key, RenderVisibilityDetectorBase>{}; | ||
| static Map<Key, VoidCallback> _updates = <Key, VoidCallback>{}; | ||
| static Map<Key, VisibilityInfo> _lastVisibility = <Key, VisibilityInfo>{}; | ||
|
|
||
| /// See [VisibilityDetectorController.notifyNow]. | ||
|
|
@@ -58,17 +57,15 @@ mixin RenderVisibilityDetectorBase on RenderObject { | |
|
|
||
| /// Executes visibility callbacks for all updated instances. | ||
| static void _processCallbacks() { | ||
| for (final detector in _updates.values) { | ||
| detector._fireCallback(); | ||
| for (final callback in _updates.values) { | ||
| callback(); | ||
| } | ||
| _updates.clear(); | ||
| } | ||
|
|
||
| void _fireCallback() { | ||
| assert(_info != null); | ||
|
|
||
| void _fireCallback(ContainerLayer layer, Rect bounds) { | ||
| final oldInfo = _lastVisibility[key]; | ||
| final info = _info!; | ||
| final info = _determineVisibility(layer, bounds); | ||
| final visible = !info.visibleBounds.isEmpty; | ||
|
|
||
| if (oldInfo == null) { | ||
|
|
@@ -111,23 +108,17 @@ mixin RenderVisibilityDetectorBase on RenderObject { | |
| forget(key); | ||
| _compositionCallbackCanceller?.call(); | ||
| _compositionCallbackCanceller = null; | ||
| _info = null; | ||
| _lastVisibility.remove(key); | ||
| } else { | ||
| markNeedsPaint(); | ||
| } | ||
| } | ||
|
|
||
| VisibilityInfo? _info; | ||
|
|
||
| /// Invokes the visibility callback if [VisibilityInfo] hasn't meaningfully | ||
| /// changed since the last time we invoked it. | ||
| void _setInfo(VisibilityInfo value) { | ||
| if (_info != null && value.matchesVisibility(_info!)) { | ||
| return; | ||
| } | ||
| void _scheduleUpdate(ContainerLayer layer, Rect bounds) { | ||
| bool isFirstUpdate = _updates.isEmpty; | ||
| _updates[key] = this; | ||
| _updates[key] = () { | ||
| _fireCallback(layer, bounds); | ||
| }; | ||
| final updateInterval = VisibilityDetectorController.instance.updateInterval; | ||
| if (updateInterval == Duration.zero) { | ||
| // Even with [Duration.zero], we still want to defer callbacks to the end | ||
|
|
@@ -148,14 +139,15 @@ mixin RenderVisibilityDetectorBase on RenderObject { | |
| } else { | ||
| assert(_timer!.isActive); | ||
| } | ||
| _info = value; | ||
| } | ||
|
|
||
| void _determineVisibility(ContainerLayer layer, Rect bounds) { | ||
| if (!layer.attached) { | ||
| VisibilityInfo _determineVisibility(ContainerLayer layer, Rect bounds) { | ||
| if (_disposed || !layer.attached || !attached) { | ||
| // layer is detached and thus invisible. | ||
| _setInfo(VisibilityInfo(key: key, size: _info?.size ?? Size.zero)); | ||
| return; | ||
| return VisibilityInfo( | ||
| key: key, | ||
| size: _lastVisibility[key]?.size ?? Size.zero, | ||
| ); | ||
| } | ||
| final transform = Matrix4.identity(); | ||
|
|
||
|
|
@@ -186,22 +178,19 @@ mixin RenderVisibilityDetectorBase on RenderObject { | |
| clip = clip.intersect(MatrixUtils.transformRect(transform, parentClip)); | ||
| } | ||
| } | ||
| _setInfo(VisibilityInfo.fromRects( | ||
| return VisibilityInfo.fromRects( | ||
| key: key, | ||
| widgetBounds: MatrixUtils.transformRect(transform, bounds), | ||
| clipRect: clip, | ||
| )); | ||
| ); | ||
| } | ||
|
|
||
| // Needed in case we get called back after disposal. | ||
| bool _disposed = false; | ||
|
|
||
| @override | ||
| void dispose() { | ||
| _setInfo(VisibilityInfo(key: key, size: _info?.size ?? Size.zero)); | ||
| _disposed = true; | ||
| _compositionCallbackCanceller?.call(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to "forget" this RO in dispose so the visibility logic is no longer triggered for it?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, never mind. You do want them to fire one more time after it got disposed it seems... Odd.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that may be our only chance to let clients know that the RO is no longer visible (assuming it ever was). There are tests that fail if you add a forget here. |
||
| _compositionCallbackCanceller = null; | ||
| _disposed = true; | ||
| super.dispose(); | ||
| } | ||
| } | ||
|
|
@@ -228,8 +217,8 @@ class RenderVisibilityDetector extends RenderProxyBox | |
| if (onVisibilityChanged != null) { | ||
|
||
| _compositionCallbackCanceller = | ||
| context.addCompositionCallback((ContainerLayer layer) { | ||
| assert(!_disposed); | ||
| _determineVisibility(layer, offset & semanticBounds.size); | ||
| assert(!debugDisposed!); | ||
| _scheduleUpdate(layer, offset & semanticBounds.size); | ||
| }); | ||
| } | ||
| super.paint(context, offset); | ||
|
|
@@ -259,7 +248,7 @@ class RenderSliverVisibilityDetector extends RenderProxySliver | |
| void paint(PaintingContext context, Offset offset) { | ||
| if (onVisibilityChanged != null) { | ||
| context.addCompositionCallback((ContainerLayer layer) { | ||
| assert(!_disposed); | ||
| assert(!debugDisposed!); | ||
|
|
||
| Size widgetSize; | ||
| Offset widgetOffset; | ||
|
|
@@ -294,7 +283,7 @@ class RenderSliverVisibilityDetector extends RenderProxySliver | |
| Size(geometry!.scrollExtent, constraints.crossAxisExtent); | ||
| break; | ||
| } | ||
| _determineVisibility(layer, offset + widgetOffset & widgetSize); | ||
| _scheduleUpdate(layer, offset + widgetOffset & widgetSize); | ||
| }); | ||
| } | ||
| super.paint(context, offset); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering, why does forget not also clear the lastVisibility for the key? Is there a use case where you want to forget, but still keep that historical information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this to forget