fix(tao): place native popup layers against the screen (#569) - #651
Merged
Conversation
#569) `nativePopupLayers` made every Compose `Popup` a real OS window, but the *decision* of where to put it stayed window-rooted, in two stacked ways. The layers each built a work-area-sized `WindowInfo` so a popup could lay out and flip against the display — and then `setContent` replayed the owner window's composition locals over it, so `Popup.skiko.kt` read the window's `containerSize` and clipped every popup back inside the window. The intended design had never taken effect. The layers now re-provide `LocalWindowInfo` inside the replayed locals. With that in force the box is screen-sized but still rooted at the window's content origin, so a `DropdownMenu` in a window near the bottom of the display did not flip up — Compose saw a whole work area of room below the anchor — and walked off the screen. Each layer now clamps its native frame into the work area of the display it lands on, at the single point where it pushes that frame: `popupScreenClampOffset`, fed the owner's content origin on screen plus every display's work area. Only the native frame moves; `boundsInWindow` stays what Compose believes, which is what hit-testing and the surface content are expressed in. Re-clamped on every push, so an open popup survives an owner drag, across monitors included. Dialogs go through the same layers but must not follow the display: `Dialog.skiko.kt` places at `containerSize.center`, so a window-owned dialog centred on the screen would sit visibly off-centre and drift as the window moved. Layers report the window size for dialogs and the work area for popups, discriminated on `scrimColor` — only `Dialog.skiko.kt` writes it, from `DialogAppearanceController.properties` during `DialogLayout`'s composition, before `layer.Content { }` reads the container. macOS needs the NSView's own origin on screen (a native title bar sits between it and the window frame), hence `nativeGetContentRect`. Wayland reports no geometry and is left unclamped: a popup there is a `wl_subsurface` placed relative to the parent, with no global position. Also exposes `nativePopupLayers` on `JewelDecoratedWindow`, which had no such parameter at all — Jewel apps could not opt in. Jewel needs nothing further: `DefaultPopupRenderer` delegates to `androidx.compose.ui.window.Popup`, so its combo boxes, menus and tooltips flow through the fixed layers. Tests: 18 unit cases on the clamp geometry, and 13 headful cases driving real windows parked at real work-area edges — including one that reads the popup HWND's rect back through Win32 and asserts it matches the reported frame to the pixel, and two that pin the dialog contract. A new "Popups" tab in nucleus-demo parks the window at any corner and opens menus anchored at each window edge.
A Compose Dialog opened through nativePopupLayers had no scrim, clipped its shadow and its appearance animation at the layout edge, slid diagonally towards the display centre while scaling in, and stayed put when the window was resized. - Scrims: the owner window paints every layer's scrim after its content and each layer paints the scrims of the layers above it (PopupScrimRegistry, TaoSceneBundle.renderOverlay). A scrim change marks the owner scene visually dirty, so the Windows clean-frame present skip no longer eats the fade. - Draw margin: the native surface extends 32 dp past boundsInWindow so shadows and the 10 dp slide-in are not clipped. Compose 1.12 renders a scene as one RenderNode drawable with unbounded bounds, so the R-tree cull-rect measurement upstream uses reports the whole canvas; a constant margin replaces it. The screen clamp is decided on the content rect; the interactive region stays the content (Windows content rect, macOS region hit-test, Linux press filter). - Dialog scene size: a dialog's root Layout fills the layer scene's constraints and carries the appearance GraphicsLayer, so its scale pivots on the scene centre. Dialog layers now run their inner scene at the owner window size, popups keep the work area. - Resize: the dialog container size is read from the owner's snapshot-backed WindowInfo, so the dialog re-centres when the window is resized. DialogAppearanceHeadfulCases films both layer modes with java.awt.Robot and compares slide-in, scrim ramp and settle time; PopupFrameRecord gains the content frame next to the inflated native frame.
The appearance film now also records the hide animation and counts grabs that repeat the previous frame during either animation — dropped frames show up as a stall count the native layer must not exceed. The owner window carries forty rows of text so its per-frame present costs something. The first-visible check is one-sided: the native surface legitimately shows its first frame before the owner's next present.
…t fades out (#569) Dialog.skiko.kt's disappearance swaps the layer's content for an empty Layout that replays the recorded picture, and Compose then reports a zero-size boundsInWindow at the window centre for the whole fade-out. An in-scene layer draws into the window canvas and does not care; the native surface followed the bounds and shrank to a 32 dp square around a point, so a closing dialog collapsed and vanished instead of fading out. Each layer now sizes and places its surface on the last non-empty bounds. The appearance film gains the Material 3 AlertDialog nucleus-demo opens, a warm-up before filming, a duration-based comparison, and the smallest height the dialog spanned while fading out.
…face
The Windows and Linux context menu flyouts drew inside the window's render
target whenever the window ran without nativePopupLayers, so a menu opened
near an edge was clipped by the window like any in-scene popup. An
OS-looking menu has to leave the window like the menus it imitates, and the
application's choice for its other popups must not decide that.
nativePopupLayers is a whole-scene switch (platform vs canvas layers), so a
per-popup opt-in needs the seam Compose 1.12 actually uses: Popup picks its
layer through LocalComposeSceneContext. A friend-package Java accessor
reaches that internal local without reflection; NativePopupLayers { } then
provides, for its subtree only, the window scene's own context with
createLayer routed to the window's native popup layer factory — the same
factory attach() uses when nativePopupLayers is on. The context menu
representation wraps the Windows and Linux flyouts in it; macOS stays on
NSMenu.
…tates Three things the menu got wrong on a Linux desktop, found by driving a real right click against a nested GNOME Shell and reading back both screenshots and the app's own trace (scripts/context-menu-wayland-e2e.py, with the fixture it drives in nucleus-application's tests). A menu opened near the bottom of the screen was cut off. On native Wayland a client cannot know where its own window is, so it cannot keep a popup on screen by itself — the X11 clamp of #569 has nothing to work with there. The popup layer now maps as an xdg_popup instead of a wl_subsurface and lets the compositor place it: it flips above the pointer when there is no room below and slides along an edge, which is what GTK's own menus do. The tao patch carries the anchor point, the surface size and the shadow margins in one request, because GDK builds the positioner from the window's geometry as it stands at map time — a popup still sized 1x1 there asks the compositor to constrain a 1x1 rectangle and is never flipped. One popup per parent takes that path (an xdg_popup must be its parent's topmost popup); a dialog keeps the subsurface, since it belongs to its window rather than to the display. A second right click only closed the menu instead of moving it. The press that dismisses a popup is delivered to the scene in the same turn as the dismissal, so Compose's contextMenuOpenDetector — disabled while the menu is open — was still disabled when the press arrived, and the press did nothing. The host now recomposes and re-lays-out the scene between the two, so the detector is listening again by the time it sees the press. The menu also appeared a beat late: the layer painted its first frame only on the owner window's next redraw, though that first render is what measures the content and puts the popup on screen at all. It renders as soon as its GPU side is up. Measured against the fixture's trace, press to first present is now 40 ms steady, 108 ms for the first menu of a session.
The flyout drew its shadow with Modifier.shadow and the themes passed the OS box-shadow alphas (Adwaita 9 % / 5 %) as ambientColor / spotColor. Compose desktop multiplies those alphas by its fixed elevation factors (0.039 ambient, 0.19 spot), so the menu on GNOME darkened the pixels next to it by about 1 % — measured on the E2E screenshots — and read as having no shadow at all. An elevation shadow also cannot reproduce a CSS box-shadow, which is how GTK, Breeze and Fluent all describe theirs. The themes now carry those declarations as box-shadow layers (offset, blur, spread, colour), drawn as the menu's rounded rectangle under a Gaussian mask with the CSS standard deviation of half the blur radius: libadwaita's _popovers.scss for Adwaita, Breeze's ShadowLarge for KDE, Fluent 2's shadow16 token for Windows. On the nested GNOME Shell the bottom edge now darkens the backdrop by about 10 %, tapering over 17 px, as the GTK menus next to it do.
…eb token The Fluent flyout took its shadow from Fluent 2's shadow16 token, and got that wrong too (its ambient layer is 0 0 8px, not 0 0 2px). The menu imitates the Windows 11 context menu, whose shadow is WinUI's ThemeShadow at Translation.Z = 32: GetDropShadowRecipe gives a single directional layer, blur radius 16 (+1) shifted down 8, at 0.14 in light and 0.26 in dark, and no ambient layer at that elevation. The composition blur radius is the ~3 sigma extent WinUI reserves around the caster, so it becomes an 11 dp CSS blur.
Checked field by field against MenuFlyout_themeresources.xaml and
Common_themeresources_any.xaml in microsoft-ui-xaml. The flyout had the touch
metrics and a few guesses; a right click opens a MenuFlyout with the mouse,
and GetShouldBeNarrow then puts every item in its NarrowPadding state.
item row 36 -> 28 (MenuFlyoutItemThemePaddingNarrow 11,4,11,5 around a
14 px label), with the 4,2,4,2 MenuFlyoutItemMargin the rows
had no vertical part of
label inset 12 -> 11
presenter MinWidth 168 -> 96 (FlyoutThemeMinWidth), no MaxWidth,
padding 4 -> 0,2 inside the 1 px border
separator 12 px insets, 4 px above and below -> edge to edge, 1 px
chevron E76C -> E974 (ChevronRightMed), 12 px, 24 px from the label,
TextFillColorSecondary rather than the label colour
shortcut CaptionTextBlockStyle 12 px, 24 px gap, margin 24,4,0,0,
TextFillColorSecondary
colours the bound resources themselves, translucent where WinUI's
are: TextFillColorPrimary/Secondary/Disabled,
SubtleFillColorSecondary for pointer-over,
DividerStrokeColorDefault, SurfaceStrokeColorFlyout
The presenter border is BackgroundSizing=InnerBorderEdge: the ring is outside
the background and blends with what is behind the menu. The flyout now paints
it that way for every theme, which is also how libadwaita's 0 0 0 1px
box-shadow ring works; Breeze strokes over its own fill, so its border colours
are pre-composited and its menu padding drops by the ring it now sits inside,
leaving its pixels as they were.
The chevron and shortcut colours move from per-theme alphas into the colour
set, since WinUI's differ between light and dark. The acrylic backdrop is not
reproduced: the surface is the brush's FallbackColor.
…er it is Six findings from a review of the #569 branch, each reproduced by a test that fails first. The margin a native popup layer's surface carries past `boundsInWindow` is transparent, but on Linux it is still the popup's window as far as the display server is concerned, and the layer swallowed everything that landed there: a click on a button 20 px beside an open menu closed the menu and never pressed the button, and hovering past the menu's edge froze the owner's hover state. Windows and macOS get the pass-through from the OS, which is handed the *content* rect. GTK's own input shaping does not take on a popup toplevel — the region reaches GDK and the X window keeps its full input shape — so the layer routes the event to the owner itself (`TaoPopupHostLinux.forwardMarginPointer`), and only while the point is over the owner's content: a press over another application is not ours to deliver. The press path is the one an owner press already takes, dismissal and mid-turn recompose included, now shared as `dismissPopupsBeforePress`. `NativePopupMarginInputHeadfulCases` drives a real pointer with `Robot` against a real popup and asserts on what the owner window's scene received — including a case with no popup open, without which "the owner saw nothing" would be as consistent with a broken driver as with a swallowed press. The macOS layer recorded its picture with a cull rect rooted at the picture origin while the scene draws in owner-window coordinates, so the rect the replay matrix maps lands off the drawable. Skia unrolls a one-op picture and never consults the rect, and a Compose scene is exactly one op, so a bare popup survived it; a popup dimmed by a dialog above it does not — the scrims go into the same picture and the whole frame is quick-rejected. `MacPopupPictureCullTest` runs the layer's frame against a real scene through the production record and replay paths and reads the pixels back. A compositor-placed popup (`xdg_popup`) that re-measured after it was mapped resized its EGL buffer while the `xdg_surface` geometry stayed at the anchored size — the buffer/geometry disagreement of #502. GDK positions a popup once, so the layer re-maps instead: hide, re-anchor at the new size, show. Also: closing a layer that was still dimming left the owner window dark until an unrelated invalidation, because `PopupScrimRegistry.unregister` removed the entry without reporting the change; the popup screen clamp read `TaoMonitors.all`, which invents a 1920x1080 monitor at the origin when the platform names none, and would have dragged a popup onto a display that does not exist — it asks `reported` now, and treats empty as "no geometry"; and nucleus-demo was missing a trailing comma, which failed `ktlintCheck` and took the whole `tao-headful` job down with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #569
Summary
nativePopupLayersturns every ComposePopupinto a real OS window, but everything downstream of that still reasoned in window coordinates. This branch makes native popup layers behave like the OS surfaces they are — placed against the screen, dressed like in-scene layers, and, on Linux, driven by the compositor — and gives the context menu its own opt-in so an app doesn't have to switch its whole scene to get an unclipped menu.Placement against the screen (#569)
LocalWindowInfoinside the replayed owner locals, soPopup.skiko.ktfinally sees the work-area-sized box the design always intended instead of the window'scontainerSize.popupScreenClampOffsetclamps the native frame into the work area of the display it lands on, at the single point where the frame is pushed — re-clamped on every push, so an open popup survives an owner drag across monitors.boundsInWindowstays what Compose believes, keeping hit-testing and surface content consistent.scrimColor.nativeGetContentRect); Wayland reports no geometry and is left to the compositor.nativePopupLayersis now exposed onJewelDecoratedWindow, which had no such parameter at all.Native dialogs that look like in-scene ones (#569)
PopupScrimRegistry,TaoSceneBundle.renderOverlay); a scrim change marks the owner scene visually dirty so the Windows clean-frame present skip no longer eats the fade.boundsInWindowso shadows and the slide-in are not clipped; the clamp and the interactive region still use the content rect.WindowInfo.Dialog.skiko.ktreports a zero-sizeboundsInWindowat the window centre during the fade-out, so each layer now sizes and places on the last non-empty bounds.Native context menu flyout (opt-in per popup)
NativePopupLayers { }provides, for its subtree only, the window scene's ownLocalComposeSceneContextwithcreateLayerrouted to the native popup layer factory. A friend-package Java accessor reaches that internal local without reflection. The context menu representation wraps the Windows and Linux flyouts in it; macOS stays onNSMenu.Linux menus behaving like GNOME's own
xdg_popupinstead of awl_subsurface, so the compositor flips and slides it on screen — the X11 clamp has nothing to work with on native Wayland. The tao patch carries anchor, surface size and shadow margins in one request, since GDK builds the positioner from the geometry as it stands at map time. One popup per parent takes that path; dialogs keep the subsurface.contextMenuOpenDetectoris listening again when the press arrives.Modifier.shadow, whose desktop implementation multiplies them by fixed elevation factors (0.039 / 0.19), leaving a ~1 % darkening nobody could see. The flyout now draws real CSS box-shadow layers (rounded rect under a Gaussian mask, sigma = blur / 2) with each OS's own declarations: libadwaita_popovers.scss, BreezeShadowLarge, and for Windows theThemeShadowrecipe WinUI applies to aMenuFlyoutatTranslation.Z = 32(DropShadowRecipe.h: one directional layer, blur 16 + 1, offset 8, 0.14 light / 0.26 dark). The Windows values are checked against the WinUI source, not on a Windows machine.MenuFlyoutopened with the mouse (NarrowPadding): 28 px rows with the4,2,4,2item margin, 11 px label inset,FlyoutThemeMinWidth96 and no max width, presenter padding0,2, edge-to-edge separators, theE974chevron 24 px from the label,CaptionTextBlockStyleshortcuts, and the bound colour resources themselves (TextFillColor*,SubtleFillColorSecondary,DividerStrokeColorDefault,SurfaceStrokeColorFlyout). The border ring is now painted outside the surface for every theme, as WinUI (InnerBorderEdge) and libadwaita do; Breeze's pixels are unchanged. The acrylic backdrop is not reproduced — the surface is the brush'sFallbackColor.Test plan
PopupScreenClampTest(18 clamp-geometry cases),PopupDrawInflateTest,PopupScrimRegistryTest,NativePopupLayersTestNativePopupPlacementHeadfulCases— 13 cases driving real windows parked at real work-area edges, including reading the popup HWND rect back through Win32 and two pinning the dialog contractDialogAppearanceHeadfulCases— films both layer modes withjava.awt.Robot, comparing slide-in, scrim ramp, settle time, dropped-frame stalls, and the smallest height spanned while fading outscripts/context-menu-wayland-e2e.pyagainst a nested GNOME Shell, driving a real right click and reading back screenshots + the app tracenucleus-demo— park the window at any corner, open menus anchored at each window edgenativePopupLayers = true(jewel-demo)./gradlew preMergeReview follow-up
Six findings from a code review of this branch, each reproduced by a test that fails first (commit
22d6c02):NativePopupMarginInputHeadfulCases(4 cases,Robot-driven).MacPopupPictureCullTestrecords and replays a real scene through the production paths and reads the pixels back.xdg_surfacegeometry stayed at the anchored size. GDK positions a popup once, so the layer re-maps instead.PopupScrimRegistry.unregisterdropped a still-dimming layer without reporting the change, leaving the owner window dark.TaoMonitors.all, which invents a 1920×1080 monitor at the origin when the platform names none; it asks the newreportednow and treats empty as "no geometry".ktlintCheck, which failed thegradlejob, which skippedtao-headfulentirely — this branch's headful cases had never run in CI.Known-failing on CI, pre-existing on this branch
Measured A/B against this branch before the fixes: macOS went 9 → 3 failures (the
dialog appearance — native popup layer matches the in-scene layercase now passes), Windows is unchanged, and Linux halts at the same watchdog on the same case with and without them. What is left is environmental or pre-existing:#569 a popup outside the owner window is left alone…,…above the top of the work area slides down,…moving the owner window re-clamps…,a Dialog whose window hangs off the display…— tuned to a 1600×1200 display; the runners are smaller, and macOS refuses to move a window above the menu bar.dialog appearance filmedcases need a real screen capture and do not get one on CI (nor under Xvfb).