Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@luckysmg
Copy link
Contributor

@luckysmg luckysmg commented Apr 14, 2023

Fix

Preview

Maybe video will be compressed by github. Here is original video zip can show the effect better:

videos.zip

Locally test for reviewers

flutter framework ref: master

Demo code
void main() {
  runApp(const MyApp());
}


class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: DemoScreen(),
    );
  }
}

class DemoScreen extends StatefulWidget {
  const DemoScreen({Key? key}) : super(key: key);

  @override
  State<DemoScreen> createState() => _DemoScreenState();
}

class _DemoScreenState extends State<DemoScreen> with TickerProviderStateMixin {
  final TextEditingController _messageController = TextEditingController();
  final FocusNode _focusNode = FocusNode();
  late final _messageNotifier = ValueNotifier(_messageController.text.isEmpty);
  bool hasFocus = false;

  @override
  Widget build(BuildContext context) {
    return GestureDetector(
      onTap: () {
        _focusNode.unfocus();
      },
      child: Scaffold(
        appBar: AppBar(
          title: const Text('Chat screen'),
        ),
        body: Column(
          children: <Widget>[
            const Spacer(),
            const Text(
              'keyboard animation',
              style: TextStyle(fontSize: 30),
            ),
            const Spacer(),
            Container(
              padding: EdgeInsets.only(
                bottom: MediaQuery.paddingOf(context).bottom + 10,
                top: 6.0,
                left: 15.0,
                right: 15.0,
              ),
              constraints: const BoxConstraints(
                minHeight: 36,
              ),
              child: Row(
                crossAxisAlignment: CrossAxisAlignment.end,
                children: [
                  Expanded(
                    child: DecoratedBox(
                      decoration: BoxDecoration(
                        color: Colors.white,
                        borderRadius: BorderRadius.circular(19),
                        border: Border.all(
                          width: 0.5,
                          color: const Color(0xffCFCFCF),
                        ),
                      ),
                      child: Row(
                        crossAxisAlignment: CrossAxisAlignment.end,
                        children: [
                          Container(
                            margin: const EdgeInsets.fromLTRB(7, 6, 7, 6),
                            decoration: const BoxDecoration(
                              color: Color(0xFF007AEA),
                              shape: BoxShape.circle,
                            ),
                            width: 24,
                            height: 24,
                            child: const Icon(
                              Icons.add,
                              color: Colors.white,
                            ),
                          ),
                          const SizedBox(width: 9.0),
                          Expanded(
                            child: Padding(
                              padding: const EdgeInsets.only(left: 0, right: 8),
                              child: TextFormField(
                                controller: _messageController,
                                minLines: 1,
                                textCapitalization: TextCapitalization.sentences,
                                focusNode: _focusNode,
                                decoration: const InputDecoration(
                                  border: InputBorder.none,
                                  isDense: true,
                                  hintText: "Message",
                                ),
                              ),
                            ),
                          ),
                          ValueListenableBuilder<bool>(
                            valueListenable: _messageNotifier,
                            builder: (ctx, value, child) {
                              if (value) {
                                return const Padding(
                                  padding: EdgeInsets.fromLTRB(7, 6, 7, 6),
                                  child: Icon(
                                    Icons.emoji_emotions_outlined,
                                    color: Color(0xFF9E9E9E),
                                  ),
                                );
                              }
                              return const SizedBox.shrink();
                            },
                          )
                        ],
                      ),
                    ),
                  ),
                  const SizedBox(
                    width: 8,
                  ),
                  Container(
                    height: 36,
                    width: 36,
                    decoration: const BoxDecoration(
                      color: Color(0xFF007AEA),
                      shape: BoxShape.circle,
                    ),
                    child: ValueListenableBuilder<bool>(
                      valueListenable: _messageNotifier,
                      builder: (ctx, value, child) {
                        return GestureDetector(
                          onTap: () {
                            _focusNode.unfocus();
                            hasFocus = false;
                          },
                          child: const Icon(
                            Icons.send,
                            color: Colors.white,
                          ),
                        );
                      },
                    ),
                  ),
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

Before

The keyboard inset is still little behind the system's keyboard, and we can see the random jitter (especially on 60FPS devices, tested on 60HZ devices like iPhone12 can obviously watch the issue)

Attention on third time when opening the keyboard in below video we can see the random jitter.

RPReplay_Final1681459397.mov

After

The animation is smoother and the bottom inset can follow the system keyboard better.

4.14.mp4

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.


static constexpr int kMicrosecondsPerSecond = 1000 * 1000;
static constexpr CGFloat kScrollViewContentSize = 2.0;
static constexpr fml::TimeDelta kKeyboardAnimationUpdateViewportMetricsDelay =
Copy link
Contributor

Choose a reason for hiding this comment

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

This does make the animation better, but an arbitrary delay could also cause janks.

Maybe instead of relying on a arbitrary delay, we could try a solution where the keyboard animation and rasterizer is synchronized, similar to macOS PlatformView.
One thing to try is to get a callback from the Shell when frame is finished rasterizing and synchronously run the keyboard animation with a mutex.
https://github.com/flutter/engine/blob/main/shell/common/shell.cc#L1447

Copy link
Contributor Author

@luckysmg luckysmg Apr 17, 2023

Choose a reason for hiding this comment

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

Since an arbitrary delay maybe cause some another issue. What I m going to do is to make SetViewportMetrics call after vsync process callback. Maybe using raster callback is a little bit late. I will raise a new PR for changing that and have a try on that. This marks as draft. ^_^

@luckysmg
Copy link
Contributor Author

luckysmg commented May 10, 2023

Compared with #41295 and #41268 I think maybe this is the PR can solve this problem with low risk. What do you think about this? @cyanglaz

Theoretically, this delay is safe when is smaller than min frame time (8ms)

@luckysmg luckysmg marked this pull request as ready for review May 10, 2023 09:44
@cyanglaz cyanglaz self-assigned this May 15, 2023
@vashworth vashworth requested a review from hellohuanlin May 22, 2023 19:13
@luckysmg
Copy link
Contributor Author

I will try a new way to implement this recently. Marks as draft. ^_^

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants