diff --git a/flutter/lib/models/model.dart b/flutter/lib/models/model.dart index d28392430..a4527eda6 100644 --- a/flutter/lib/models/model.dart +++ b/flutter/lib/models/model.dart @@ -3544,7 +3544,8 @@ class CursorModel with ChangeNotifier { // Update last cursor data. // Do not use the previous `image` and `id`, because `_id` may be changed. _updateCurData(); - if (id != _id) _cacheMap[id]?.releasePixels(); + final cache = _cacheMap[id]; + if (id != _id && cache != null) _switchedAway(cache); return true; } @@ -3639,14 +3640,38 @@ class CursorModel with ChangeNotifier { /// The shape in use keeps its pixels, so a new raster is made from them at once; the others /// keep none once their native cursor holds them. void registered(CursorData cache, String key) { + _nativeIds.add(cache.id); + _awaitingNative.remove(cache.id); if (cache.id != _id && identical(_cacheMap[cache.id], cache)) { cache.releasePixels(); } } - /// The peer's shapes used last that keep a painted image; the core rebuilds the others. An - /// animated cursor is a shape per frame, 18 for the Windows busy cursor and 23 for KDE's, and - /// a cycle longer than this limit would rebuild every frame. + // The shapes a native cursor holds, and those keeping their pixels until one does, oldest + // first: one decoded after the peer moved on must still get a native cursor when shown + // again, rather than be decoded again every time it comes back. + final _nativeIds = {}; + final _awaitingNative = {}; + + void _switchedAway(CursorData cache) { + _awaitingNative.remove(cache.id); + // Mobile only paints the cursor, from its image. + if (isMobile || _nativeIds.contains(cache.id)) { + cache.releasePixels(); + return; + } + _awaitingNative.add(cache.id); + while (_awaitingNative.length > kRecentShapes) { + final oldest = _awaitingNative.first; + _awaitingNative.remove(oldest); + if (oldest != _id) _cacheMap[oldest]?.releasePixels(); + } + } + + /// The peer's shapes used last that keep a painted image, and the most that keep their pixels + /// waiting for a native cursor; the core rebuilds the others. An animated cursor is a shape per + /// frame, 18 for the Windows busy cursor and 23 for KDE's, and a cycle longer than this limit + /// would rebuild every frame. static const kRecentShapes = 64; /// The native cursor shown last, shown on while the shape in use is made. @@ -3729,7 +3754,7 @@ class CursorModel with ChangeNotifier { // A shape not decoded yet leaves the one shown before in place until it is. if (cache != null) _cache = cache; if (previous != null && !identical(previous, _cache)) { - previous.releasePixels(); + _switchedAway(previous); } final tmp = _images.remove(_id); if (tmp != null) { @@ -3803,6 +3828,8 @@ class CursorModel with ChangeNotifier { _clearCache(); _cache = null; _cacheMap.clear(); + _nativeIds.clear(); + _awaitingNative.clear(); _unavailable = null; _generation++; } diff --git a/flutter/test/cursor_replay_test.dart b/flutter/test/cursor_replay_test.dart index 4560aacdc..552b16cbb 100644 --- a/flutter/test/cursor_replay_test.dart +++ b/flutter/test/cursor_replay_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:io'; import 'package:flutter/services.dart'; @@ -14,10 +15,22 @@ class _Cursor extends CursorModel { _Cursor(FFI ffi) : super(WeakReference(ffi)); final core = {}; final fetched = []; + // While set, a fetch answers only when [answerFetches] is called. + bool holdFetches = false; + final _held = , String>{}; @override - Future fetchCursorShape(String id) async { + Future fetchCursorShape(String id) { fetched.add(id); - return core[id]; + if (!holdFetches) return Future.value(core[id]); + final answer = Completer(); + _held[answer] = id; + return answer.future; + } + + void answerFetches() { + final held = Map.of(_held); + _held.clear(); + held.forEach((answer, id) => answer.complete(core[id])); } bool showRemoteCursor = false; @@ -95,12 +108,16 @@ void main() { binding.defaultBinaryMessenger.setMockMethodCallHandler(channel, null); }); - test('only the shape in use keeps an image, and nothing else keeps pixels', - () async { + test( + 'only the shape in use keeps an image, and a shape its native cursor holds ' + 'keeps no pixels', () async { + final cursor = ffi.cursorModel; for (var round = 0; round < 3; round++) { - await _feed(ffi, '1', seed: 1); - await _feed(ffi, '2', seed: 2); - await _feed(ffi, '3', seed: 3); + for (final (id, seed) in [('1', 1), ('2', 2), ('3', 3)]) { + await _feed(ffi, id, seed: seed); + buildCursorOfCache(cursor, 1.0, cursor.cache); + await _settle(); + } } expect(ffi.cursorModel.shapeIds, ['3']); for (final id in ['1', '2']) { @@ -155,7 +172,8 @@ void main() { expect(registered.length, 100); }); - test('a raster made before is found again after the shape was switched away from', + test( + 'a raster made before is found again after the shape was switched away from', () async { final cursor = ffi.cursorModel; await _feed(ffi, 'A', size: 32); @@ -180,10 +198,12 @@ void main() { () async { final cursor = ffi.cursorModel; await _feed(ffi, '1', size: 32); + buildCursorOfCache(cursor, 1.0, cursor.cache); + await _settle(); await _feed(ffi, '2', size: 32); final shown = _key(buildCursorOfCache(cursor, 1.0, cursor.cache)); await _settle(); - _select(ffi, '1'); // its pixels went when it was switched away from + _select(ffi, '1'); // its native cursor held its pixels, so they went expect(_key(buildCursorOfCache(cursor, 0.5, cursor.cache)), shown); await _settle(); expect(ffi.cursor.fetched, ['1']); @@ -191,6 +211,46 @@ void main() { cursor.nativeKey(cursor.cache!, 0.5)); }); + test('shapes that keep decoding late still get their native cursors', + () async { + final cursor = ffi.cursorModel; + await _feed(ffi, 'arrow'); + buildCursorOfCache(cursor, 1.0, cursor.cache); + await _settle(); + // A and B arrive, each decoding only after the peer moved on. + for (final (id, seed) in [('A', 1), ('B', 2)]) { + final pixels = _pixels(8, seed); + ffi.cursor.core[id] = + CursorShape(hotx: 0, hoty: 0, width: 8, height: 8, colors: pixels); + final decoding = ffi.ffiModel.handleCursorData(id, 0, 0, 8, 8, pixels); + _select(ffi, 'arrow'); + await decoding; + } + // Each fetch answers only after the next switch. + ffi.cursor.holdFetches = true; + for (var i = 0; i < 6; i++) { + _select(ffi, i.isEven ? 'A' : 'B'); + ffi.cursor.answerFetches(); + await _settle(); + buildCursorOfCache(cursor, 1.0, cursor.cache); + await _settle(); + } + expect(registered.where((key) => key.contains('_A_')), isNotEmpty); + expect(registered.where((key) => key.contains('_B_')), isNotEmpty); + }); + + test('shapes waiting for a native cursor keep their pixels, within the limit', + () async { + const max = CursorModel.kRecentShapes; + for (var i = 0; i <= max + 1; i++) { + await _feed(ffi, '$i'); + } + expect(ffi.cursorModel.cachedShape('0')!.hasPixels, isFalse, + reason: 'the one waiting longest is let go'); + expect(ffi.cursorModel.cachedShape('1')!.hasPixels, isTrue); + expect(ffi.cursorModel.cachedShape('$max')!.hasPixels, isTrue); + }); + test('two animated cursors and the everyday set keep their native cursors', () async { // Each frame is a shape of its own: 18 for the Windows busy cursor, 23 for @@ -256,8 +316,9 @@ void main() { await Future.delayed(Duration.zero); _select(ffi, '2'); await _settle(); - expect(ffi.cursorModel.cachedShape('1')!.hasPixels, isFalse, - reason: 'decoded after the peer moved on, it is let go'); + expect(ffi.cursorModel.cachedShape('1')!.hasPixels, isTrue, + reason: + 'decoded after the peer moved on, it waits for its native cursor'); _select(ffi, '1'); ffi.cursorModel.image; await _settle();