Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The buttons in the text selection toolbar bounce on desktop #107733

Closed
justinmc opened this issue Jul 15, 2022 · 14 comments
Closed

The buttons in the text selection toolbar bounce on desktop #107733

justinmc opened this issue Jul 15, 2022 · 14 comments
Labels
a: desktop Running on desktop a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list platform-linux Building on or for Linux specifically team-linux Owned by the Linux platform team

Comments

@justinmc
Copy link
Contributor

On desktop, the buttons in the text selection toolbar have some small vertical movement after they appear. Notice the downward movement of the buttons in the gif below as I move my mouse.

Steps to reproduce

  1. Run any app with text that can be selected, such as the one given below with a TextField, on desktop.
  2. Right click to show the toolbar.
  3. Wait a bit, maybe move the mouse.

Expected: The toolbar buttons stay in place and don't move.
Actual: The toolbar buttons move vertically by a pixel or two.

Example app
import 'package:flutter/material.dart';

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(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}
class MyHomePage extends StatelessWidget {
  const MyHomePage({Key? key, required this.title}) : super(key: key);

  final String title;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            TextField(),
          ],
        ),
      ),
    );
  }
}
@justinmc justinmc added a: text input Entering text in a text field or keyboard related problems a: desktop Running on desktop labels Jul 15, 2022
@HansMuller HansMuller added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 19, 2022
@TahaTesser
Copy link
Member

I'm unable to reproduce on the latest master channel on the macOS desktop.

Screen.Recording.2022-07-20.at.17.35.22.mov
Code Sample
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key, this.dark = false, this.useMaterial3 = true});

  final bool dark;
  final bool useMaterial3;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      themeMode: dark ? ThemeMode.dark : ThemeMode.light,
      theme: ThemeData(
        useMaterial3: useMaterial3,
        brightness: Brightness.light,
        colorSchemeSeed: const Color(0xff6750a4),
      ),
      darkTheme: ThemeData(
        useMaterial3: useMaterial3,
        brightness: Brightness.dark,
        colorSchemeSeed: const Color(0xff6750a4),
      ),
      home: const Example(),
    );
  }
}

class Example extends StatelessWidget {
  const Example({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('TextField'),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: [
            TextField(
              controller: TextEditingController(
                  text:
                      'Flutter Flutter Flutter Flutter Flutter Flutter Flutter Flutter Flutter Flutter'),
              style: const TextStyle(fontSize: 34.0),
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: () {},
        child: const Icon(Icons.add),
      ),
    );
  }
}

@gspencergoog
Copy link
Contributor

Justin, where did you reproduce this? It looks like it's drawing a different menu from the one that Taha shows. (Desktop Triage)

@gspencergoog gspencergoog added the P2 Important issues not at the top of the work list label Jul 21, 2022
@justinmc
Copy link
Contributor Author

justinmc commented Aug 8, 2022

Sorry, this was on Linux! I just tried it again and I still see it on Linux, using a physical mouse. I tried on Mac and did not see it, using a trackpad. Not sure if that has anything to do with it. It seems strange that the platform would affect this.

@justinmc justinmc added the platform-linux Building on or for Linux specifically label Aug 8, 2022
@jpnurmi
Copy link
Member

jpnurmi commented Aug 19, 2022

This is caused by fractional translation. It's not 100% deterministic but relatively easy to reproduce with popups and dialogs and device pixel ratio 1.

fractional.mp4

@tgucio
Copy link
Contributor

tgucio commented Aug 19, 2022

Likely same as #107921 (seen on Windows, also with dialogs, Chips etc.)

jpnurmi added a commit to jpnurmi/engine that referenced this issue Aug 19, 2022
With fractional coordinates, it's not enough to round up device bounds
width and height. Round out to get a large enough integer-size texture.

Fixes: flutter/flutter#107733
Fixes: flutter/flutter#107921
Fixes: flutter/flutter#109608
jpnurmi added a commit to jpnurmi/engine that referenced this issue Aug 19, 2022
With fractional coordinates, it's not enough to round up device bounds
width and height. Round out to get a large enough integer-size texture.

Fixes: flutter/flutter#107733
Fixes: flutter/flutter#107921
Fixes: flutter/flutter#109608
jpnurmi added a commit to jpnurmi/engine that referenced this issue Aug 19, 2022
With fractional coordinates, it's not enough to round up device bounds
width and height. Round out to get a large enough integer-size texture.

Fixes: flutter/flutter#107733
Fixes: flutter/flutter#107921
Fixes: flutter/flutter#109608
jpnurmi added a commit to jpnurmi/engine that referenced this issue Aug 19, 2022
With fractional coordinates, it's not enough to round up device bounds
width and height. Round out to get a large enough integer-size texture.

Fixes: flutter/flutter#107733
Fixes: flutter/flutter#107921
Fixes: flutter/flutter#109608
@justinmc
Copy link
Contributor Author

@jpnurmi Thanks for the insight!

@jonahwilliams Could you help me understand how #103909 affects this bug? I'm wondering if it will be fixed by that or if there's anything else we need to do in the framework.

@jonahwilliams
Copy link
Member

I have not debugged the issue, though @jpnurmi is looking at it. #103909 landed a long time ago - essentially it removed forced pixel snapping from composited layers. That fixed some instances of the above, where an animated transition would appear to "snap" into place as it concluded. According to some other filled issues, that makes some previous rendering that would have been pixel aligned non-pixel aligned, and so on low DPR there can be apparent snapping as something animates across pixels.

Its possible there is another issue mixed in, where we might still be rounding out in places - which would actually be a bug and not just an artifact of rendering at low DPR

@justinmc
Copy link
Contributor Author

Thanks I appreciate the explanation! I'll keep an eye out for an update from @jpnurmi.

A large monitor like where I saw this bug would be a low DPR compared to a late model smartphone if I'm not mistaken, so maybe that's evidence in favor of this being due to the snapping thing.

@jonahwilliams
Copy link
Member

Right, so if you're on a low DPR device like a desktop monitor (probably 1.0 or 1.25), then drawing something like text at fractional pixel boundaries may be a noticable shift from pixel aligned (depending on a lot of other things like how AA is working).

The pixel snapping I removed worked at the level of the composited layer, so if you painted text at 0,0 or at some other position that when multiplied by dpr landed at round number, your text was guaranteed to be pixel aligned. But as far as I know, you could still draw your text at something like 0.33, 0.33 and end up non-pixel aligned.

In general, this is the same sort of problem as the hairline appearing between solid color containers.

@jpnurmi
Copy link
Member

jpnurmi commented Aug 20, 2022

With checkerboardRasterCacheImages turned on, it's easy to see how they are incorrectly drawn:

checkerboard_raster_cache_images.mp4

@jpnurmi
Copy link
Member

jpnurmi commented Aug 20, 2022

Sometimes even lines and the checkerboard are not straight:
Screenshot from 2022-08-20 11-17-33

image

The respective raster cache image looks fine, though:
rasterize_931_457_2

main.dart
import 'dart:async';

import 'package:flutter/material.dart';

// linux/my_application.cc:
// gtk_window_set_default_size(window, 577, 381);

void main() {
  runApp(const MaterialApp(
    checkerboardRasterCacheImages: true,
    home: MyHomePage(),
  ));
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key});

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  void initState() {
    super.initState();
    WidgetsBinding.instance.addPostFrameCallback((_) => _showTestDialog());
  }

  Future<void> _showTestDialog() {
    return showDialog(
      context: context,
      builder: (BuildContext buildContext) {
        return AlertDialog(
          insetPadding: const EdgeInsets.all(0.5),
          content: SizedBox.expand(
            child: Center(
              child: OutlinedButton(
                style: OutlinedButton.styleFrom(
                  side: const BorderSide(color: Colors.green, width: 1),
                ),
                onPressed: () {},
                child: const Text('Button', style: TextStyle(fontSize: 128)),
              ),
            ),
          ),
        );
      },
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: ElevatedButton(
          onPressed: _showTestDialog,
          child: const Text('Show dialog'),
        ),
      ),
    );
  }
}

@Hixie
Copy link
Contributor

Hixie commented May 2, 2024

FWIW, I'm not able to reproduce this. I tried today's tip of tree (roughly 3.22.0-22.0.pre.13, 21da3a8) and a version from when the issue was filed (3.1.0-0.0.pre.1695, 4f95282). Linux desktop. 96 dpi, 1.0 dpr.

@justinmc
Copy link
Contributor Author

justinmc commented May 2, 2024

I just tried the same and I was able to reproduce it on 4f95282 but not on tip of tree. It seems like it's been fixed, so I will close this, but if anyone else reproduces it on master let me know.

@justinmc justinmc closed this as completed May 2, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2024
@cbracken cbracken added team-linux Owned by the Linux platform team and removed team-desktop labels Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list platform-linux Building on or for Linux specifically team-linux Owned by the Linux platform team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants