Skip to content

Improve initial size and placement logic for popups and tooltips on Linux - #189994

Merged
mattkae merged 10 commits into
flutter:masterfrom
canonical:improve-sized-to-content-linux
Aug 19, 2026
Merged

mattkae merged 10 commits into
flutter:masterfrom
canonical:improve-sized-to-content-linux

Conversation

@mattkae

@mattkae mattkae commented Jul 24, 2026 •

Copy link
Copy Markdown
Contributor

Why?

The initial sizing of popup and tooltip windows on Linux was broken such that windows that calculated their initial size too slowly would have their content overflowing off the edge of the screen. Because they never reposition themselves, this content would be lost forever.

This PR fixes that initial sizing such that we wait for the convergence of initial sizes between the embedder and the framework before showing the popup/tooltip.

What's new?

  • Add a new on_size_changed callback to the FlViewMonitor that notifies us when the view's size changes
  • Tooltips and popups on Linux are now only shown when the first frame has arrived AND the size of the view in the Linux embedder agrees with the size of the content in the framework
    • This essentially spins until they converge to the same size, with a timeout of 250ms.
  • Made it so that X11 windows can be repositoned whereas Wayland windows cannot

This fix consistently manages to place my windows correctly, which is very awesome.

How it works

mermaid-diagram-2026-07-24T19-27-31

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically a: desktop Running on desktop team-linux Owned by the Linux platform team labels Jul 24, 2026
@mattkae
mattkae requested a review from robert-ancell July 24, 2026 19:29
@mattkae
mattkae marked this pull request as ready for review July 24, 2026 19:30
@mattkae
mattkae requested a review from a team as a code owner July 24, 2026 19:30

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces deferred-show and reposition-on-resize behavior for anchored, sized-to-content Linux windows (tooltips and popups) by monitoring view size changes via FFI. The review feedback highlights several important issues to address: potential null pointer exceptions when accessing RendererBinding.instance or native type names, resource leaks from unclosed _viewMonitor and uncancelled _showFallbackTimer instances, and a lack of exception safety when updating window positions that could permanently lock the repositioning state.

@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@mattkae mattkae added the CICD Run CI/CD label Jul 24, 2026
@flutter-dashboard

Copy link
Copy Markdown

This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled.

@mattkae mattkae changed the title Improve size to content logic for popups and tooltips on Linux Improve initial size and placement logic for popups and tooltips on Linux Jul 24, 2026

@robert-ancell robert-ancell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested here and works, thanks!

@Piinks

Piinks commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

CI failure was an infra issue, re-running. This might need a rebase to merge as well.

@mattkae
mattkae enabled auto-merge August 10, 2026 12:24
@mattkae

mattkae commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor Author

@knopp
The sized to content work can be misleading and tricky. For example, this works:

// ignore_for_file: invalid_use_of_internal_member
// ignore_for_file: implementation_imports
import 'package:flutter/material.dart';
import 'package:flutter/src/widgets/_window.dart';
import 'package:flutter/src/widgets/_window_positioner.dart';

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  runWidget(const WindowingApp());
}

final ThemeData _appTheme = ThemeData(
  colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
);

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

  @override
  State<WindowingApp> createState() => _WindowingAppState();
}

class _CallbackPopupDelegate extends PopupWindowControllerDelegate {
  _CallbackPopupDelegate({required this.onDestroyCallback});

  final VoidCallback onDestroyCallback;

  @override
  void onWindowDestroyed() {
    onDestroyCallback();
  }
}

class _WindowingAppState extends State<WindowingApp> {
  final WindowController _windowController = WindowController(
    size: Size(800, 600),
    title: "Tester",
  );
  PopupWindowController? _popupWindowController;

  void _togglePopup(Rect anchorRect) {
    if (_popupWindowController != null) {
      _popupWindowController!.destroy();
      return;
    }

    setState(() {
      _popupWindowController = PopupWindowController(
        parent: _windowController,
        anchorRect: anchorRect,
        positioner: const WindowPositioner(
          parentAnchor: .topRight,
          childAnchor: .topLeft,
          constraintAdjustment: WindowPositionerConstraintAdjustment(
            flipY: true,
            slideX: true,
            slideY: true,
            resizeY: true,
          ),
        ),
        delegate: _CallbackPopupDelegate(
          onDestroyCallback: () {
            setState(() {
              _popupWindowController = null;
            });
          },
        ),
      );
    });
  }

  @override
  Widget build(BuildContext context) {
    return ViewCollection(
      views: [
        Window(
          controller: _windowController,
          child: MyApp(onTogglePopup: _togglePopup),
        ),
        if (_popupWindowController != null)
          PopupWindow(
            controller: _popupWindowController!,
            child: SizedBox(
              width: 400,
              height: 200,
              child: MaterialApp(
                debugShowCheckedModeBanner: false,
                theme: _appTheme,
                home: Material(
                  color: _appTheme.colorScheme.surfaceContainer,
                  child: ListView.builder(
                    shrinkWrap: true,
                    itemCount: 100,
                    itemBuilder: (BuildContext context, int index) {
                      return ListTile(
                        dense: true,
                        title: Text('Item $index'),
                        onTap: () => _popupWindowController?.destroy(),
                      );
                    },
                  ),
                ),
              ),
            ),
          ),
      ],
    );
  }
}

class MyApp extends StatelessWidget {
  const MyApp({super.key, required this.onTogglePopup});

  final ValueChanged<Rect> onTogglePopup;

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: _appTheme,
      home: MyHomePage(
        title: 'Flutter Demo Home Page',
        onTogglePopup: onTogglePopup,
      ),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({
    super.key,
    required this.title,
    required this.onTogglePopup,
  });

  final String title;
  final ValueChanged<Rect> onTogglePopup;

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

class _MyHomePageState extends State<MyHomePage> {
  final GlobalKey _fabKey = GlobalKey();

  void _onPressed() {
    final Rect? anchorRect = _getAnchorRect();
    if (anchorRect == null) {
      return;
    }
    widget.onTogglePopup(anchorRect);
  }

  Rect? _getAnchorRect() {
    final RenderBox? renderBox =
        _fabKey.currentContext?.findRenderObject() as RenderBox?;
    if (renderBox == null) {
      return null;
    }
    final Rect rect = renderBox.localToGlobal(Offset.zero) & renderBox.size;
    return rect;
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: .center,
          children: [const Text('Click the button to show the popup')],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        key: _fabKey,
        onPressed: _onPressed,
        tooltip: 'Show Popup',
        child: const Icon(Icons.add),
      ),
    );
  }
}

But replacing the SizedBox with a Container causes the popup to render but never appear on screen, even though the list is shrink wrapped, which should provide a size:

          PopupWindow(
            controller: _popupWindowController!,
            child: Container(
              child: MaterialApp(
                debugShowCheckedModeBanner: false,
                theme: _appTheme,
                home: Material(
                  color: _appTheme.colorScheme.surfaceContainer,
                  child: ListView.builder(
                    shrinkWrap: true,
                    itemCount: 100,
                    itemBuilder: (BuildContext context, int index) {
                      return ListTile(
                        dense: true,
                        title: Text('Item $index'),
                        onTap: () => _popupWindowController?.destroy(),
                      );
                    },
                  ),
                ),
              ),
            ),
          )

@knopp

knopp commented Aug 19, 2026

Copy link
Copy Markdown
Member

The problem in this case is that MaterialApp can never size itself to content. It is because it has Navigator, which has Overlay, which has unfortunate property where it always sizes to constraints.biggest. Which is likely bigger than what compositor is happy with.

Try removing the Material app, build your own scaffold of widgets that are needed for popup content. If you need Overlay in there, make sure to set alwaysSizeToContent=true when creating it.

@mattkae
mattkae added this pull request to the merge queue Aug 19, 2026
Merged via the queue into flutter:master with commit f6061d0 Aug 19, 2026
16 of 17 checks passed
@mattkae
mattkae deleted the improve-sized-to-content-linux branch August 19, 2026 04:04
mattkae added a commit to canonical/flutter that referenced this pull request Aug 21, 2026
pull Bot pushed a commit to TheRakeshPurohit/flutter that referenced this pull request Aug 24, 2026
……ips on Linux (flutter#189994) (flutter#191498)

This reverts commit f6061d0.

Reverts flutter#189994

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Co-authored-by: Robert Ancell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD engine flutter/engine related. See also e: labels. framework flutter/packages/flutter repository. See also f: labels. platform-linux Building on or for Linux specifically team-linux Owned by the Linux platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants