Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ class IosControlsDocumentLayerState extends DocumentLayoutLayerState<IosHandlesD
bool get isCaretDisplayed => layoutData?.caret != null;

@visibleForTesting
bool get isCaretVisible => _caretBlinkController.opacity == 1.0 && isCaretDisplayed;
bool get isCaretVisible => _caretBlinkController.isVisible && isCaretDisplayed;

@visibleForTesting
Duration get caretFlashPeriod => _caretBlinkController.flashPeriod;
Expand Down
77 changes: 70 additions & 7 deletions super_text_layout/lib/src/infrastructure/blink_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,27 @@ class BlinkController with ChangeNotifier {
@visibleForTesting
static bool indeterminateAnimationsEnabled = true;

/// Creates a [BlinkController] that uses a ticker to switch between visible
/// and invisible.
///
/// If [animate] is `true`, the caret animates its [opacity] when switching
/// between visible and invisible. If [animate] is `false`, the caret
/// switches between fully opaque and fully transparent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a point here about performance? The reason we originally got rid of animated blinking was to reduce Flutter re-renders.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the original performance issue really about re-renders? Or was it related to repaints? I'm wondering if using e RepaintBoundary in our overlayers would make a difference.

BlinkController({
required TickerProvider tickerProvider,
Duration flashPeriod = const Duration(milliseconds: 500),
}) : _flashPeriod = flashPeriod {
bool animate = false,
Duration fadeDuration = const Duration(milliseconds: 200),
}) : _flashPeriod = flashPeriod,
_fadeDuration = fadeDuration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can the fade duration be independent of the flash period?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caret isn't constantly fading in and out. It stays visible or invisible during the flash period, then quickly fades in/out.

Should we just use half of the flash period?

_animate = animate {
_ticker = tickerProvider.createTicker(_onTick);
}

BlinkController.withTimer({
Duration flashPeriod = const Duration(milliseconds: 500),
}) : _flashPeriod = flashPeriod;
}) : _fadeDuration = Duration.zero,
_flashPeriod = flashPeriod;

@override
void dispose() {
Expand All @@ -39,10 +50,13 @@ class BlinkController with ChangeNotifier {

Timer? _timer;

final Duration _flashPeriod;

/// Duration to switch between visible and invisible.
Duration get flashPeriod => _flashPeriod;
final Duration _flashPeriod;

/// Duration of the fade in or out transition when switching
/// between visible and invisible.
final Duration _fadeDuration;

/// Returns `true` if this controller is currently animating a blinking
/// signal, or `false` if it's not.
Expand All @@ -62,8 +76,33 @@ class BlinkController with ChangeNotifier {
notifyListeners();
}

/// Whether or not the caret is currently visible.
///
/// When [isVisible] switches from `true` to `false`, the [opacity] is
/// still greater than `0.0` while the fade-out animation is playing.
bool get isVisible => _isVisible;
bool _isVisible = true;
double get opacity => _isVisible ? 1.0 : 0.0;

double get opacity => _opacity;
double _opacity = 1.0;

/// Whether or not the caret should animate its opacity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also specify the alternative. It's possible a developer reads this and then asks "what does it mean to not animate the opacity?" - let's make the distinction between fading blinking vs on/off blinking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates.

///
/// If `true`, the caret fades in and out when switching between visible
/// and invisible. If `false`, the caret switches between fully opaque
/// and fully transparent.
bool _animate = false;

/// Whether or not the caret is keeping the current opacity until the next blink.
///
/// This is used to keep the caret fully opaque for the [_flashPeriod], before it
/// switches to `false` when the caret blinks for the first time. After [_isCaretOpaqueUntilNextBlink]
/// is set to `true`, the caret fades in and out.
///
/// For example, when the user places the caret, we want it to be visible with
/// full opacity until the caret switches to invisible instead of immediately
/// start the fade out animation.
bool _isCaretOpaqueUntilNextBlink = false;

void startBlinking() {
if (!indeterminateAnimationsEnabled) {
Expand All @@ -73,6 +112,9 @@ class BlinkController with ChangeNotifier {
}

if (_ticker != null) {
// Don't animate the opacity until the flash period has elapsed.
_isCaretOpaqueUntilNextBlink = true;

// We're using a Ticker to blink. Restart it.
_ticker!
..stop()
Expand All @@ -88,12 +130,17 @@ class BlinkController with ChangeNotifier {
}

void stopBlinking() {
_isVisible = true; // If we're not blinking then we need to be visible
// If we're not blinking then we need to be visible with full opacity.
_isVisible = true;
_opacity = 1.0;

// Keep the caret opaque until the flash period has elapsed, when it
// starts to fade out.
_isCaretOpaqueUntilNextBlink = true;

if (_ticker != null) {
// We're using a Ticker to blink. Stop it.
_ticker?.stop();
_ticker = null;
} else {
// We're using a Timer to blink. Stop it.
_timer?.cancel();
Expand All @@ -118,6 +165,13 @@ class BlinkController with ChangeNotifier {
}

void _onTick(Duration elapsedTime) {
if (isBlinking && _animate && !_isCaretOpaqueUntilNextBlink) {
final percentage = ((elapsedTime - _lastBlinkTime).inMilliseconds / _fadeDuration.inMilliseconds).clamp(0.0, 1.0);
// If the caret is visible, we want to fade in. Otherwise, we want to fade out.
_opacity = _isVisible ? percentage : 1 - percentage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment to clarify the meaning of _isVisible in this calculation, because you're actually using it as a proxy to decide whether we're "fading in" or "fading out", and that's not immediately obvious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

notifyListeners();
}

if (elapsedTime - _lastBlinkTime >= _flashPeriod) {
_blink();
_lastBlinkTime = elapsedTime;
Expand All @@ -126,6 +180,15 @@ class BlinkController with ChangeNotifier {

void _blink() {
_isVisible = !_isVisible;
if (!_animate) {
// We are not animating the visibility, so the caret is
// either fully visible or fully invisible.
_opacity = _isVisible ? 1.0 : 0.0;
} else {
// We are animating the visibility, start fading in or out.
_isCaretOpaqueUntilNextBlink = false;
}

notifyListeners();

if (_timer != null && _isBlinkingEnabled) {
Expand Down
Loading