sni: fix passing relative coordinates to dbus methods

[Cherser-s]

Fixes passing relative coordinates to ContextMenu, Activate, SecondaryActivate DBus methods, while handling mouse click events. Resolves #2165.

[alebastr]

the x and y parameters are in screen coordinates and is to be considered an hint to the item about where to show the context menu.

This doesn't really do what you wanted: Wayland windows have no access to the global coordinate space, and root_x/root_y are calculated in a fake coordinate space originated from (0,0) of a toplevel surface (as explained in details in gdk/wayland/gdkwindow-wayland.c). So the change just replaces one set of wrong coordinates with another.

It might be possible to translate toplevel-based coordinates into global using the layer-surface size/margins/anchors and xdg-output data, but I don't think it worth the effort. No wayland-native apps will be able to use these values anyways.

[Cherser-s]

Ok I implemented the coordinate translation, though I don't check the case if both bar size (width or height) and margins are being set for now.

[alebastr]

Hm... I was pretty sure that fixed width doesn't work, appears that someone has fixed that. layer-shell is still going to send the wayland protocol configure event, but it may be lost in translation to GDK events. `Bar::output->monitor.property_geometry().signal_changed()` could be used to supplement the configure event.

…

On Mon, Aug 21, 2023 at 11:28 AM Cherser-s ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/modules/sni/item.cpp <#2417 (comment)>: > @@ -409,8 +411,44 @@ void Item::makeMenu() { } bool Item::handleClick(GdkEventButton* const& ev) { + int x = ev->x_root; + int y = ev->y_root; + auto window = gdk_window_get_toplevel(ev->window); Yeah, but signal_configure_event ain't enough, since if one specifies both width and height, then the configure event won't get called. — Reply to this email directly, view it on GitHub <#2417 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAUOSP6GGRRCNDIV6SD3XN3XWOSEXANCNFSM6AAAAAA3TDMCDI> . You are receiving this because you commented.Message ID: ***@***.***>

-- With best regards, Aleksei Bavshin

[Cherser-s]

Nope, the configure event just isn't triggered if window has sufficient size to begin with. I worked it around with also configuring offsets within the map event.

[alebastr]

Please, clang-format your changes. It's enforced by CI, but apparently Github CI runs should be approved by repo owner for first-time contributions :(

For monitor geometry changes, consider following snippet

void onOutputGeometryChanged() { configureGlobalOffset(window.get_width(), window.get_height()); }

// Bar::Bar
  output->monitor->property_geometry().signal_changed().connect(
      sigc_mem_fun(*this, &Bar::onOutputGeometryChanged));

This should fix the global coordinates after scale factor change even if the surface size wasn't modified.

[alebastr]

LGTM, thanks for addressing my comments.

@Alexays can you please approve the CI run for this PR?

[Cherser-s]

Can output's geometry change without creating a new Bar object beforehand?

[alebastr]

Can output's geometry change without creating a new Bar object beforehand?

Yes, the primary cases are output rotation and scale changes. Bar won't be recreated on that kind of change, but it will receive a new preferred size from the compositor. And possibly ignore it if a fixed width/height is used, without triggering the GDK configure event.

[Cherser-s]

Bar won't be recreated on that kind of change

Yup, if both width and height are set, then monitor geometry change handling needed

[Alexays]

LGTM, It's ok to be merged?

[Cherser-s]

I would say yes, I didn't plan on adding anything else.

[Alexays]

Thanks!