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

[WIP] Basic WM #14

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
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
137 changes: 106 additions & 31 deletions compositor_dart/lib/compositor_dart.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// ignore_for_file: public_member_api_docs, sort_constructors_first
library compositor_dart;

import 'dart:async';
Expand All @@ -11,6 +12,18 @@ import 'package:logging/logging.dart';

enum KeyStatus { released, pressed }

enum WindowEventType { maximize, minimize }

class WindowEvent {
final WindowEventType windowEventType;
final int handle;

WindowEvent({
required this.windowEventType,
required this.handle,
});
}

class Surface {
// This is actually used as a pointer on the compositor side.
// It should always be returned exactly as is to the compositiors,
Expand All @@ -20,16 +33,42 @@ class Surface {
final int pid;
final int gid;
final int uid;

final Compositor compositor;
final bool isPopup;
final int parentHandle;
final int surfaceWidth;
final int surfaceHeight;
final int offsetTop;
final int offsetLeft;
final int offsetRight;
final int offsetBottom;
final int contentWidth;
final int contentHeight;
bool isMaximized;
bool isMinimized;

Surface({
required this.handle,
required this.pid,
required this.gid,
required this.uid,
required this.compositor,
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep a reference to Compositor from Surfaces or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not necassry. Compositor will be created in main, and probably main should keep track of it. Is there actually any use case where we might have more than 1 compositor ?

Copy link
Member

Choose a reason for hiding this comment

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

There will never be more than one compositor at a time in a given isolate, but the Compositor it belongs to is still part of the state conceptually in my mind.

At the very least it prevents us from having to pass it around everywhere along with the surface. Most times you have a Surface you need access to the compositor as well.

In my mind we have a few options:

  1. Pass the compositor object around alongside every surface. Might result in quite a bit of boilerplate as the dart side library expands in size.
  2. Just rely on the fact that there will only ever be one compositor object. Access it through a global everywhere. Not sure I like this, I like to keep state local even with singletons.
  3. Store the Compositor object within each surface. Only downside I can think of here is memory, but that's only a pointer.

Copy link
Contributor

Choose a reason for hiding this comment

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

what about providing the compositor using an InheritedWidget?
the app in order to use the compositor will need to use as root a Compositor widget of some sort which will then be used to create, maintain and dispose the compositor instance and also expose it to descendants using Compositor.of(context).
This way a Surface won't need to hold a compositor instance but instead only the SurfaceView will need to get it, using the of method indeed.
It would also be possible to detect if the app is running with the compositor or not because of Compositor.maybeOf(context) returning null if no compositor is available for example

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not using the widget tree to pass around the compositor instance.

The main reason for this is that the compositor instance inherently has another lifetime compared to something in the widget tree.

It creates a platform channel and uses this to communicate with native code on the other side for the lifetime of the isolate. It also accumulates and maintains state for its whole lifetime, something conceptually incompatible with being maintained as part of the widget tree. It is analogous to the various Bindings in the flutter framework. (WidgetBinding for instance)

It's just inherently a singleton with a global lifetime, and I think we would save ourselves some pain and complexity in the long run by just treating it as such from the beginning.

That being said, whether we should pass it around as a part of the surface objects or access it as a global every time is still an open question for me. Having written this out and thought through it a bit more I'm leaning towards the global route.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point then, i'm more leaning towards the global approach myself too

required this.isPopup,
required this.parentHandle,
required this.surfaceWidth,
required this.surfaceHeight,
required this.offsetTop,
required this.offsetLeft,
required this.offsetRight,
required this.offsetBottom,
required this.contentWidth,
required this.contentHeight,
this.isMaximized = false,
this.isMinimized = false,
});

@override
String toString() {
return 'Surface(handle: $handle, pid: $pid, gid: $gid, uid: $uid, isPopup: $isPopup, parentHandle: $parentHandle, isMaximized: $isMaximized, isMinimized: $isMinimized)';
}
}

class CompositorSockets {
Expand All @@ -41,7 +80,8 @@ class CompositorSockets {
class _CompositorPlatform {
final MethodChannel channel = const MethodChannel("wlroots");

final HashMap<String, Future<dynamic> Function(MethodCall)> handlers = HashMap();
final HashMap<String, Future<dynamic> Function(MethodCall)> handlers =
HashMap();

_CompositorPlatform() {
channel.setMethodCallHandler((call) async {
Expand All @@ -62,15 +102,18 @@ class _CompositorPlatform {
handlers[method] = handler;
}

Future<void> surfaceToplevelSetSize(Surface surface, int width, int height) async {
await channel.invokeListMethod("surface_toplevel_set_size", [surface.handle, width, height]);
Future<void> surfaceToplevelSetSize(
Surface surface, int width, int height) async {
await channel.invokeListMethod(
"surface_toplevel_set_size", [surface.handle, width, height]);
}

Future<void> clearFocus(Surface surface) async {
await channel.invokeMethod("surface_clear_focus", [surface.handle]);
}

Future<void> surfaceSendKey(Surface surface, int keycode, KeyStatus status, Duration timestamp) async {
Future<void> surfaceSendKey(Surface surface, int keycode, KeyStatus status,
Duration timestamp) async {
await channel.invokeListMethod(
"surface_keyboard_key",
[
Expand All @@ -82,8 +125,13 @@ class _CompositorPlatform {
);
}

Future<void> surfaceFocusViewWithHandle(int handle) async {
await channel.invokeMethod("surface_focus_from_handle", [handle]);
}

Future<CompositorSockets> getSocketPaths() async {
var response = await channel.invokeMethod("get_socket_paths") as Map<dynamic, dynamic>;
var response =
await channel.invokeMethod("get_socket_paths") as Map<dynamic, dynamic>;
return CompositorSockets(
wayland: response["wayland"] as String,
x: response["x"] as String,
Expand Down Expand Up @@ -113,6 +161,7 @@ class Compositor {
// Emits an event when a surface has been added and is ready to be presented on the screen.
StreamController<Surface> surfaceMapped = StreamController.broadcast();
StreamController<Surface> surfaceUnmapped = StreamController.broadcast();
StreamController<WindowEvent> windowEvents = StreamController.broadcast();

int? keyToXkb(int physicalKey) => physicalToXkbMap[physicalKey];

Expand All @@ -123,40 +172,66 @@ class Compositor {
pid: call.arguments["client_pid"],
gid: call.arguments["client_gid"],
uid: call.arguments["client_uid"],
compositor: this,
isPopup: call.arguments["is_popup"],
parentHandle: call.arguments["parent_handle"],
surfaceHeight: call.arguments['surface_height'],
surfaceWidth: call.arguments['surface_width'],
offsetTop: call.arguments['offset_top'],
offsetLeft: call.arguments['offset_left'],
offsetRight: call.arguments['offset_right'],
offsetBottom: call.arguments['offset_bottom'],
contentWidth: call.arguments['content_width'],
contentHeight: call.arguments['content_height'],
);
surfaces[surface.handle] = surface;
surfaces.putIfAbsent(surface.handle, () => surface);

surfaceMapped.add(surface);
});

platform.addHandler("surface_unmap", (call) async {
int handle = call.arguments["handle"];
Surface surface = surfaces[handle]!;
surfaces.remove(handle);
surfaceUnmapped.add(surface);
if (surfaces.containsKey(handle)) {
Surface surface = surfaces[handle]!;
surfaces.remove(handle);
surfaceUnmapped.add(surface);
}
Comment on lines -128 to +197
Copy link
Member

Choose a reason for hiding this comment

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

For any given surface, I believe map and unmap should be only called strictly interspersed in a sequence:

map, unmap, map, unmap, ...

This should mean that:

  • When map is called, there should never be a surface of that handle in the surfaces map, we add it
  • When unmap is called, there should always be a surface of that handle in the surfaces, we remove it

We should likely add assertions for this, if this assumption is broken it would be a bug in the native code

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if the fact that things fail here on multiple surfaces means there is an error somewhere in my code where multiple surfaces get the same handle id? I think there are some debug prints for when a surface is mapped with its handle id, are those different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I've checked, handles were different but I think it was the way we managed compositor - now it's created in and managed in main.

});

platform.addHandler("flutter/keyevent", (call) async {});
}

/// Returns `true` if we are currently running in the compositor embedder.
/// If so, all functionality in this library is available.
///
/// Returns `false` in all other cases. If so, no funcitonality in this
/// library should be used.
Future<bool> isCompositor() async {
if (_isCompositor != null) return _isCompositor!;

try {
await platform.channel.invokeMethod("is_compositor");
_isCompositor = true;
} on MissingPluginException {
_isCompositor = false;
platform.addHandler('window_maximize', (call) async {
int handle = call.arguments['handle'];

windowEvents.add(WindowEvent(
windowEventType: WindowEventType.maximize, handle: handle));
});

platform.addHandler('window_minimize', (call) async {
int handle = call.arguments['handle'];

windowEvents.add(WindowEvent(
windowEventType: WindowEventType.minimize, handle: handle));
});

/// Returns `true` if we are currently running in the compositor embedder.
/// If so, all functionality in this library is available.
///
/// Returns `false` in all other cases. If so, no funcitonality in this
/// library should be used.
Future<bool> isCompositor() async {
if (_isCompositor != null) return _isCompositor!;

try {
await platform.channel.invokeMethod("is_compositor");
_isCompositor = true;
} on MissingPluginException {
_isCompositor = false;
}

return _isCompositor!;
}

return _isCompositor!;
/// Will return the paths of the compositor sockets.
Future<CompositorSockets> getSocketPaths() => platform.getSocketPaths();
}

/// Will return the paths of the compositor sockets.
Future<CompositorSockets> getSocketPaths() => platform.getSocketPaths();
}
11 changes: 8 additions & 3 deletions compositor_dart/lib/platform/interceptor_widgets_binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,27 @@ import 'package:flutter/services.dart';
class InterceptorWidgetsBinding extends WidgetsFlutterBinding {
InterceptorBinaryMessenger? _binaryMessenger;

static WidgetsBinding? instance;

@override
void initInstances() {
super.initInstances();
_binaryMessenger = InterceptorBinaryMessenger(super.defaultBinaryMessenger);
instance = this;
}

@override
BinaryMessenger get defaultBinaryMessenger {
return _binaryMessenger == null ? super.defaultBinaryMessenger : _binaryMessenger!;
return _binaryMessenger == null
? super.defaultBinaryMessenger
: _binaryMessenger!;
}

static WidgetsBinding ensureInitialized() {
if (WidgetsBinding.instance == null) {
if (InterceptorWidgetsBinding.instance == null) {
InterceptorWidgetsBinding();
}
return WidgetsBinding.instance!;
return InterceptorWidgetsBinding.instance!;
}

static void runApp(Widget app) {
Expand Down
44 changes: 35 additions & 9 deletions compositor_dart/lib/surface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class _MeasureSizeRenderObject extends RenderProxyBox {
if (oldSize == newSize) return;

oldSize = newSize;
WidgetsBinding.instance?.addPostFrameCallback((_) {
WidgetsBinding.instance.addPostFrameCallback((_) {
onChange(newSize);
});
}
Expand All @@ -46,8 +46,15 @@ class _MeasureSize extends SingleChildRenderObjectWidget {

class SurfaceView extends StatefulWidget {
final Surface surface;
final Compositor compositor;
final Function(Surface surface)? onPointerClick;

const SurfaceView({Key? key, required this.surface}) : super(key: key);
const SurfaceView({
Key? key,
required this.surface,
required this.compositor,
this.onPointerClick,
}) : super(key: key);

@override
State<StatefulWidget> createState() {
Expand All @@ -61,15 +68,23 @@ class _SurfaceViewState extends State<SurfaceView> {
@override
void initState() {
super.initState();
controller = _CompositorPlatformViewController(surface: widget.surface);
controller = _CompositorPlatformViewController(
surface: widget.surface,
compositor: widget.compositor,
onPointerClick: widget.onPointerClick,
);
}

@override
void didUpdateWidget(SurfaceView oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.surface != widget.surface) {
controller.dispose();
controller = _CompositorPlatformViewController(surface: widget.surface);
controller = _CompositorPlatformViewController(
surface: widget.surface,
compositor: widget.compositor,
onPointerClick: widget.onPointerClick,
);
}
}

Expand All @@ -90,7 +105,7 @@ class _SurfaceViewState extends State<SurfaceView> {
int? keycode = physicalToXkbMap[event.physicalKey.usbHidUsage];

if (keycode != null) {
controller.surface.compositor.platform.surfaceSendKey(
controller.compositor.platform.surfaceSendKey(
widget.surface,
keycode,
status,
Expand Down Expand Up @@ -120,18 +135,26 @@ class _SurfaceViewState extends State<SurfaceView> {
}

class _CompositorPlatformViewController extends PlatformViewController {
Surface surface;
final Surface surface;
final Compositor compositor;
final Function(Surface surface)? onPointerClick;
Size size = const Size(100, 100);

_CompositorPlatformViewController({required this.surface});
_CompositorPlatformViewController({
required this.surface,
required this.compositor,
this.onPointerClick,
});

void setSize(Size size) {
this.size = size;
Compositor.compositor.platform.surfaceToplevelSetSize(surface, size.width.round(), size.height.round());
Compositor.compositor.platform.surfaceToplevelSetSize(
surface, size.width.round(), size.height.round());
}

@override
Future<void> clearFocus() => Compositor.compositor.platform.clearFocus(surface);
Future<void> clearFocus() =>
Compositor.compositor.platform.clearFocus(surface);

@override
Future<void> dispatchPointerEvent(PointerEvent event) async {
Expand All @@ -154,6 +177,9 @@ class _CompositorPlatformViewController extends PlatformViewController {
Offset scrollAmount = Offset.zero;
if (event is PointerDownEvent) {
eventType = pointerDownEvent;
if (onPointerClick != null) {
onPointerClick!(surface);
}
} else if (event is PointerUpEvent) {
eventType = pointerUpEvent;
} else if (event is PointerHoverEvent) {
Expand Down
13 changes: 13 additions & 0 deletions demo/lib/constants.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import 'package:flutter/material.dart';

final systemBorderRadius = BorderRadius.circular(6);

const double windowWidth = 400;
const double windowHeight = 500;
const double initialPositionX = 200;
const double initialPositionY = 70;
const double windowDecorationHeight = 30;
const double windowDecorationControlSize = 14;
const Color windowDecorationColor = Color(0xff1c1c1e);
const Color windowDecorationControlColor = Color(0xffffffff);
const borderWidth = 1;
Loading