The problem
When removing a output in handling unplugging a HDMI display, weston will reposition all views into the first output if the views aren’t within the region of any outputs in shell_reposition_view_on_output_change() of desktop-shell/shell.c. And if a view is a popup, the geometry of that view is set to a relation position to its parent view here.
When the relative position of a popup view has negative values, the repositioning in aforementioned function will reckon this view invisible and move it to first quarter of the first output!
if (!visible) {
first_output = container_of(ec->output_list.next,
struct weston_output, link);
x = first_output->x + first_output->width / 4;
y = first_output->y + first_output->height / 4;
weston_view_set_position(view, x, y);
}
For example, in our use case, Brightness Bar is a popup view and its parent is System Controls. The following table shows the positions of these two views on an internal display of resolution 1920x1080 (first output):
| Window | Absolute Position | Relative Position |
|---|---|---|
| System Controls | (534,281) | N/A |
| Brightness Bar | (1045,135) | (511,-146) |
The position (511,-146) is out of the region of 1920x1080 in the checking of shell_reposition_view_on_output_change(). When a HDMI display is unplugged, the Brightness Bar will move to absolute position (480,270), which is different from its original absolute position (1045,135).
The fix
Since the position of popup view is relative to its parent, so when repositioning a popup view, we should convert it to the absolute position before checking if the position is within the region of any outputs.
In order to do the conversion, we should check if the role of the view is WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP, but the role information is only accessible in libweston-desktop/xdg-shell-v6.c. To keep it simple, we check if any coordinate is negative:
if (x < 0 || y < 0) {
parent_view = view->geometry.parent;
if (parent_view) {
x += parent_view->geometry.pos_offset.x;
y += parent_view->geometry.pos_offset.y;
}
}
If both coordinates are positive, the check of within a output region is sufficient to make sure the view is visible.
visible = 0;
if (ec->clone_mode && ec->primary_output) {
/* In clone mode, all outputs have the same geometry and region,
* so only need to check if the view is visible in primary output
*/
if (weston_output_contains_point(ec->primary_output, x,y)) {
visible = 1;
}
} else {
wl_list_for_each(output, &ec->output_list, link) {
if (weston_output_contains_point(output, x, y)) {
visible = 1;
break;
}
}
}
Side note
In wayland protocol, xdg_positioner.set_anchor_rect is used to implement the position of a popup surface:
<request name="set_anchor_rect">
<description summary="set the anchor rectangle within the parent surface">
Specify the anchor rectangle within the parent surface that the child
surface will be placed relative to. The rectangle is relative to the
window geometry as defined by xdg_surface.set_window_geometry of the
parent surface.
When the xdg_positioner object is used to position a child surface, the
anchor rectangle may not extend outside the window geometry of the
positioned child's parent surface.
If a negative size is set the invalid_input error is raised.
</description>
<arg name="x" type="int" summary="x position of anchor rectangle"/>
<arg name="y" type="int" summary="y position of anchor rectangle"/>
<arg name="width" type="int" summary="width of anchor rectangle"/>
<arg name="height" type="int" summary="height of anchor rectangle"/>
</request>
In Qt, it’s done in QWaylandXdgSurface::setPopup():
void QWaylandXdgSurface::setPopup(QWaylandWindow *parent)
{
Q_ASSERT(!m_toplevel && !m_popup);
auto positioner = new QtWayland::xdg_positioner(m_shell->m_xdgWmBase->create_positioner());
// set_popup expects a position relative to the parent
QRect windowGeometry = m_window->windowContentGeometry();
QMargins windowMargins = m_window->windowContentMargins() - m_window->clientSideMargins();
QMargins parentMargins = parent->windowContentMargins() - parent->clientSideMargins();
QPoint transientPos = m_window->geometry().topLeft(); // this is absolute
transientPos += QPoint(windowMargins.left(), windowMargins.top());
transientPos -= parent->geometry().topLeft();
transientPos -= QPoint(parentMargins.left(), parentMargins.top());
positioner->set_anchor_rect(transientPos.x(), transientPos.y(), 1, 1);
positioner->set_anchor(QtWayland::xdg_positioner::anchor_top_left);
positioner->set_gravity(QtWayland::xdg_positioner::gravity_bottom_right);
positioner->set_size(windowGeometry.width(), windowGeometry.height());
positioner->set_constraint_adjustment(QtWayland::xdg_positioner::constraint_adjustment_slide_x
| QtWayland::xdg_positioner::constraint_adjustment_slide_y);
m_popup = new Popup(this, parent, positioner);
positioner->destroy();
delete positioner;
}
And there’s actually a commit in upstream qtwayland about invalid anchor rectangles. As the comment in that commit said, Qt’s own clients are supplying invalid anchor rectangles (and violating xdg_positioner protocol), so it’s just a warning but not an error.
Have fun!