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

refactor(gui): pull dbus methods out; introduce jeepney #584

Merged
merged 4 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions adr/006-use-jeepney-for-dbus-calls.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# Use jeepney for dbus calls

- Status: accepted
- Deciders: @dynobo
- Date: 2024-01-07

## Context and Problem Statement

## Decision Drivers

- Robustness
- Implementation efforts
- Complexity reduction
- Dependency reduction
- Startup time

## Considered Options

### (A) Stay with [QtDBus](https://doc.qt.io/qtforpython-6/PySide6/QtDBus/index.html)

- Current implementation.

### (B) Switch back to [jeepney](https://pypi.org/project/jeepney/)

- Initially NormCap used to use `jeepney`, but it was removed in favor of QtDBus to get
rid of that additional dependency and lower the import time.

### (C) Switch to [dbus-next](https://pypi.org/project/dbus-next/)

- The fork [asyncdbus](https://pypi.org/project/asyncdbus/) seems less well spread and
not significantly better maintained.

### (D) Hybrid QtDbus + jeepney/dbus-next

- Use jeepney/dbus-next only where QtDBus has known issues.

## Decision Outcome

**Chosen option: (B) Switch back to jeepney.**

### Positive Consequences

- Integration with Window Calls extensions becomes possible and increases robustness of
window positioning on Gnome/Wayland.
- DBus logic can be simplified and easier maintained.
- Fewer workarounds needed.

### Negative Consequences

- It will require initial effort to switch to `jeepney`.
- It will require more effort to switch back to QtDBus, once it becomes stable enough.
- Additional dependency with common risks.
- Additional module adds to import time.

## Pros and Cons of the Options

### (A) Stay with QtDBus

- Good, as no additional dependency required
- Good, as no additional module to add to import time
- Bad, as it seems not widely used and not well maintained (bad documentation, few SO
topics, few tutorials)
- Bad, as relatively cumbersome to develop with, due to its lower level nature.
- Bad, because it has significant number of flaws/bugs, e.g.:
- Can't send `u` (int32) datatype, as required for the window ID when using
`org.gnome.Shell.Extensions.Window`. See
[#PYSIDE-1904](https://bugreports.qt.io/browse/PYSIDE-1904).
- Can't reliably
[decode arguments](https://github.com/dynobo/normcap/blob/d79ef9c1c9ca022066944563c65290ccaadf6a45/normcap/screengrab/dbus_portal.py#L140-L161).

### (B) Switch back to jeepney

- Good, as quite simple to implement
- Good, as it provides a higher level API, stub generator
- Good, as it doesn't have any 3rd party dependencies
- Good, as it provides async api (currently not needed)
- Good, as it provides blocking api
- Bad, as it is an additional dependency for NormCap
- Neutral, as its development isn't very active

### (C) Switch to dbus-next

**Note:** No practical experience with dbus-next, arguments are purely based on docs.

- Good, as quite simple to implement
- Good, as it provides a higher level API, stub generator
- Good, as it doesn't have any 3rd party dependencies
- Good, as it provides async api (currently not needed)
- Bad, as its async event loop can't be easily integrated with Qt's event loop (would
require the additional dependency [qasync](https://pypi.org/project/qasync/))
- Bad, as it does not provide blocking api
- Bad, as it is an additional dependency for NormCap
- Neutral, as its development isn't very active

### (D) Hybrid QtDbus + jeepney/dbus-next

- Good, as it is easier to switch to preferred QtDbus only, once the issues are solved
- Bad, as implementing with different APIs increases complexity

## References

- e592d132 - refactor(gui): remove qtdbus versions of window positioning
214 changes: 214 additions & 0 deletions normcap/gui/dbus.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
import json
import logging
import os
import tempfile
import traceback

from jeepney.io.blocking import Proxy, open_dbus_connection
from jeepney.wrappers import Message, MessageGenerator, new_method_call

from normcap.gui.models import Rect

logger = logging.getLogger(__name__)


class DBusShell(MessageGenerator):
interface = "org.gnome.Shell"

def __init__(
self, object_path: str = "/org/gnome/Shell", bus_name: str = "org.gnome.Shell"
) -> None:
super().__init__(object_path=object_path, bus_name=bus_name)

def eval_(self, script: str) -> Message:
return new_method_call(self, "Eval", "s", (script,))


class DBusKwinScripting(MessageGenerator):
interface = "org.kde.kwin.Scripting"

def __init__(
self,
object_path: str = "/Scripting",
bus_name: str = "org.kde.KWin",
) -> None:
super().__init__(object_path=object_path, bus_name=bus_name)

def load_script(self, script_file: str) -> Message:
return new_method_call(self, "loadScript", "s", (script_file,))

def start(self) -> Message:
return new_method_call(self, "start")


class DBusWindowCalls(MessageGenerator):
interface = "org.gnome.Shell.Extensions.Windows"

def __init__(
self,
object_path: str = "/org/gnome/Shell/Extensions/Windows",
bus_name: str = "org.gnome.Shell",
) -> None:
super().__init__(object_path=object_path, bus_name=bus_name)

def list_(self) -> Message:
return new_method_call(self, "List")

def get_title(self, win_id: int) -> Message:
return new_method_call(self, "GetTitle", "u", (win_id,))

def move_resize( # noqa: PLR0913
self, win_id: int, x: int, y: int, width: int, height: int
) -> Message:
return new_method_call(
self, "MoveResize", "uiiuu", (win_id, x, y, width, height)
)


def move_window_via_gnome_shell_eval(title_id: str, position: Rect) -> bool:
"""Move currently active window to a certain position.

This is a workaround for not being able to reposition windows on wayland.
It only works on Gnome Shell.

Args:
title_id: Window title (has to be unique)
position: Target geometry

Returns:
If call was successful
"""
logger.debug(
"Moving window '%s' to %s via org.gnome.Shell.Eval", title_id, position
)
js_code = f"""
const GLib = imports.gi.GLib;
global.get_window_actors().forEach(function (w) {{
var mw = w.meta_window;
if (mw.get_title() == "{title_id}") {{
mw.move_resize_frame(
0,
{position.left},
{position.top},
{position.width},
{position.height}
);
}}
}});
"""
try:
with open_dbus_connection() as router:
proxy = Proxy(DBusShell(), router)
response = proxy.eval_(script=js_code)
if not response[0]:
raise RuntimeError("DBus response was not OK!") # noqa: TRY301
except Exception as exc:
logger.warning("Failed to move window via org.gnome.Shell.Eval!")
logger.debug("".join(traceback.format_exception(exc)).strip())
return False
else:
return True


def move_window_via_kde_kwin_scripting(title_id: str, position: Rect) -> bool:
"""Move currently active window to a certain position.

This is a workaround for not being able to reposition windows on wayland.
It only works on KDE.

Args:
title_id: Window title (has to be unique)
position: Target geometry

Returns:
If call was successful
"""
logger.debug(
"Moving window '%s' to %s via org.kde.kwin.Scripting", title_id, position
)
js_code = f"""
const clients = workspace.clientList();
for (var i = 0; i < clients.length; i++) {{
if (clients[i].caption() == "{title_id}" ) {{
clients[i].geometry = {{
"x": {position.left},
"y": {position.top},
"width": {position.width},
"height": {position.height}
}};
}}
}}
"""
with tempfile.NamedTemporaryFile(delete=True, suffix=".js") as script_file:
script_file.write(js_code.encode())

try:
with open_dbus_connection() as router:
proxy = Proxy(DBusKwinScripting(), router)
response = proxy.load_script(script_file=script_file.name)
if response[0] != 0:
raise RuntimeError( # noqa: TRY301
"org.kde.kwin.Scripting.loadScript response: %s", response
)
response = proxy.start()

except Exception as exc:
logger.warning("Failed to move window via org.kde.kwin.Scripting!")
logger.debug("".join(traceback.format_exception(exc)).strip())
return False
else:
return True


def move_windows_via_window_calls_extension(title_id: str, position: Rect) -> bool:
"""Move currently active window to a certain position.

This is a workaround for not being able to reposition windows on wayland.
It only works on Gnome and requires the Gnome Shell Extension 'Window Calls'
https://github.com/ickyicky/window-calls

Args:
title_id: Window title (has to be unique)
position: Target geometry

Returns:
If call was successful
"""
logger.debug(
"Moving window '%s' to %s via org.gnome.Shell.extensions.windows",
title_id,
position,
)
try:
with open_dbus_connection() as router:
proxy = Proxy(DBusWindowCalls(), router)

response = proxy.list_()
all_windows = json.loads(response[0])
normcap_windows = [w for w in all_windows if w["pid"] == os.getpid()]

window_id = None
for window in normcap_windows:
response = proxy.get_title(window["id"])
window_title = response[0]
if window_title == title_id:
window_id = window["id"]

response = proxy.move_resize(
window_id,
position.left,
position.top,
position.width,
position.height,
)
except Exception as exc:
logger.warning("Failed to move window via org.gnome.Shell.extensions.windows!")
logger.debug("".join(traceback.format_exception(exc)).strip())
logger.warning(
"If you experience issues with NormCap's in a multi monitor setting, "
"try installing the Gnome Shell Extension 'Window Calls' "
"from https://extensions.gnome.org/extension/4724/window-calls/"
)
return False
else:
return True
2 changes: 1 addition & 1 deletion normcap/gui/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def pypi_json(self) -> str:

@dataclass()
class Rect:
"""Rectangular selection on screen.
"""Rectangle to represent section of screen.

All points are inclusive (are part of the rectangle).
"""
Expand Down
Loading