From a7d553d93ef08b39f9d852a936181f92f910af1d Mon Sep 17 00:00:00 2001 From: Ryan Pavlik Date: Mon, 12 Sep 2022 10:59:41 -0500 Subject: [PATCH] xrt,comp,ipc...: Clarifying docs about "system compositor" and "multi compositor" --- doc/conventions.md | 4 +- doc/ipc-design.md | 7 +-- .../compositor/multi/comp_multi_interface.h | 7 +-- src/xrt/compositor/multi/comp_multi_private.h | 46 ++++++++++++------- src/xrt/include/xrt/xrt_compositor.h | 46 ++++++++++++++----- src/xrt/include/xrt/xrt_gfx_native.h | 7 +-- src/xrt/ipc/client/ipc_client_compositor.c | 8 ++++ 7 files changed, 84 insertions(+), 41 deletions(-) diff --git a/doc/conventions.md b/doc/conventions.md index 7ba33e415..46b74427e 100644 --- a/doc/conventions.md +++ b/doc/conventions.md @@ -84,8 +84,8 @@ directed-acyclic-graph. words if you go by the `_` delimiters, but for clarity we treat swapchain as if it were two words when abbreviating. A few other places in the `xrt` headers use `x` + an abbreviated name form, like `xinst` - for @ref xrt_instance, `xdev` for @ref xrt_device, `xsysc` sometimes - used for @ref xrt_system_compositor. + for @ref xrt_instance, `xdev` for @ref xrt_device, `xsysd` sometimes + used for @ref xrt_system_devices. - `create` and `destroy` are used when the functions actually perform allocation and return the new object, or deallocation of the passed-in object. diff --git a/doc/ipc-design.md b/doc/ipc-design.md index 6fb79ea66..57984efa8 100644 --- a/doc/ipc-design.md +++ b/doc/ipc-design.md @@ -5,11 +5,12 @@ Copyright 2021-2022, Collabora, Ltd. and the Monado contributors SPDX-License-Identifier: BSL-1.0 --> -- Last updated: 15-July-2022 +- Last updated: 12-September-2022 When the service starts, an `xrt_instance` is created and selected, a native -system compositor is initialized, a shared memory segment for device data is -initialized, and other internal state is set up. (See `ipc_server_process.c`.) +compositor is initialized by a system compositor, a shared memory segment for +device data is initialized, and other internal state is set up. (See +`ipc_server_process.c`.) There are three main communication needs: diff --git a/src/xrt/compositor/multi/comp_multi_interface.h b/src/xrt/compositor/multi/comp_multi_interface.h index f5cffa6b5..1f5faa23b 100644 --- a/src/xrt/compositor/multi/comp_multi_interface.h +++ b/src/xrt/compositor/multi/comp_multi_interface.h @@ -20,9 +20,10 @@ struct u_pacing_app_factory; /*! - * Create a system compositor that can handle multiple clients and that drives - * a single native compositor. Both the native compositor and the pacing factory - * is owned by the multi compositor and destroyed by it. + * Create a "system compositor" that can handle multiple clients (each + * through a "multi compositor") and that drives a single native compositor. + * Both the native compositor and the pacing factory is owned by the system + * compositor and destroyed by it. */ xrt_result_t comp_multi_create_system_compositor(struct xrt_compositor_native *xcn, diff --git a/src/xrt/compositor/multi/comp_multi_private.h b/src/xrt/compositor/multi/comp_multi_private.h index b6aaa02c3..6b4905dcf 100644 --- a/src/xrt/compositor/multi/comp_multi_private.h +++ b/src/xrt/compositor/multi/comp_multi_private.h @@ -2,7 +2,7 @@ // SPDX-License-Identifier: BSL-1.0 /*! * @file - * @brief Multi-client compositor internal structs. + * @brief System compositor capable of supporting multiple clients: internal structs. * @author Jakob Bornecrantz * @ingroup comp_multi */ @@ -87,7 +87,8 @@ struct multi_event }; /*! - * A single compositor. + * A single compositor for feeding the layers from one session/app into + * the multi-client-capable system compositor. * * @ingroup comp_multi */ @@ -209,6 +210,7 @@ multi_compositor_create(struct multi_system_compositor *msc, * Push a event to be delivered to the client. * * @ingroup comp_multi + * @private @memberof multi_compositor */ void multi_compositor_push_event(struct multi_compositor *mc, const union xrt_compositor_event *xce); @@ -218,6 +220,7 @@ multi_compositor_push_event(struct multi_compositor *mc, const union xrt_composi * thread and copies data from multi_compositor::scheduled to multi_compositor::delivered while holding the slot_lock. * * @ingroup comp_multi + * @private @memberof multi_compositor */ void multi_compositor_deliver_any_frames(struct multi_compositor *mc, uint64_t display_time_ns); @@ -225,12 +228,12 @@ multi_compositor_deliver_any_frames(struct multi_compositor *mc, uint64_t displa /* * - * System compositor. + * Multi-client-capable system compositor * */ /*! - * State of the multi compositor system, use to track the calling of native + * State of the multi-client system compositor. Use to track the calling of native * compositor methods @ref xrt_comp_begin_session and @ref xrt_comp_end_session. * * It is driven by the number of active app sessions. @@ -242,34 +245,35 @@ enum multi_system_state /*! * Initial state and post stopping state. * - * The multi system compositor have called @ref xrt_comp_end_session - * on it's @ref xrt_compositor_native. + * The multi-client system compositor has called @ref xrt_comp_end_session + * on its @ref xrt_compositor_native. */ MULTI_SYSTEM_STATE_STOPPED, /*! * The main session is running. * - * The multi system compositor have called @ref xrt_comp_begin_session - * on it's @ref xrt_compositor_native. + * The multi-client system compositor has called @ref xrt_comp_begin_session + * on its @ref xrt_compositor_native. */ MULTI_SYSTEM_STATE_RUNNING, /*! - * There are no active sessions and the multi compositor system is - * drawing on or more clear frames. + * There are no active sessions and the multi-client system compositor is + * instructing the native compositor to draw one or more clear frames. * - * The multi system compositor have not yet @ref xrt_comp_begin_session - * on it's @ref xrt_compositor_native. + * The multi-client system compositor has not yet called @ref xrt_comp_begin_session + * on its @ref xrt_compositor_native. */ MULTI_SYSTEM_STATE_STOPPING, }; /*! - * The multi compositor system, multiplexes access to the native compositors - * and tracks some state needed. + * The multi-client system compositor multiplexes access to a single native compositor, + * merging layers from one or more client apps/sessions. * * @ingroup comp_multi + * @implements xrt_system_compositor */ struct multi_system_compositor { @@ -290,8 +294,9 @@ struct multi_system_compositor struct { /*! - * The state of the multi compositor system, this is updated on - * the oth thread, aka multi compositor system main thread. + * The state of the multi-client system compositor. + * This is updated on the multi_system_compositor::oth + * thread, aka multi-client system compositor main thread. * It is driven by the active_count field. */ enum multi_system_state state; @@ -316,6 +321,12 @@ struct multi_system_compositor struct multi_compositor *clients[MULTI_MAX_CLIENTS]; }; +/*! + * Cast helper + * + * @ingroup comp_multi + * @private @memberof multi_system_compositor + */ static inline struct multi_system_compositor * multi_system_compositor(struct xrt_system_compositor *xsc) { @@ -323,10 +334,11 @@ multi_system_compositor(struct xrt_system_compositor *xsc) } /*! - * The client compositor calls this function to update when it's session is + * The client compositor calls this function to update when its session is * started or stopped. * * @ingroup comp_multi + * @private @memberof multi_system_compositor */ void multi_system_compositor_update_session_status(struct multi_system_compositor *msc, bool active); diff --git a/src/xrt/include/xrt/xrt_compositor.h b/src/xrt/include/xrt/xrt_compositor.h index 5697b561f..b5f4553c5 100644 --- a/src/xrt/include/xrt/xrt_compositor.h +++ b/src/xrt/include/xrt/xrt_compositor.h @@ -1875,12 +1875,13 @@ xrt_comp_native_destroy(struct xrt_compositor_native **xcn_ptr) /* * - * System compositor. + * System composition: how to composite on a system, either directly or by combining layers from multiple apps * */ /*! - * Capabilities and information about the system compositor and device together. + * Capabilities and information about the system compositor (and its wrapped native compositor, if any), + * and device together. */ struct xrt_system_compositor_info { @@ -1901,7 +1902,7 @@ struct xrt_system_compositor_info } max; //!< Maximums for this view. } views[2]; //!< View configuration information. - //! Maximum number of layers supported by the compositor, never changes. + //! Maximum number of composition layers supported, never changes. uint32_t max_layers; /*! @@ -1937,6 +1938,8 @@ struct xrt_system_compositor; /*! * @interface xrt_multi_compositor_control * Special functions to control multi session/clients. + * Effectively an optional aspect of @ref xrt_system_compositor + * exposed by implementations that can combine layers from multiple sessions/clients. */ struct xrt_multi_compositor_control { @@ -1967,28 +1970,40 @@ struct xrt_multi_compositor_control }; /*! - * The system compositor is a long lived object, it has the same life time as a - * XrSystemID. + * The system compositor handles composition for a system. + * It is not itself a "compositor" (as in xrt_compositor), but it can create/own compositors. + * - In a multi-app capable system, the system compositor may own an internal compositor, and + * xrt_system_compositor::create_native_compositor will + * create a compositor that submits layers to a merging mechanism. + * - In a non-multi-app capable system, xrt_system_compositor::create_native_compositor + * creates normal, native compositors, that do not wrap or feed into any other compositor. + * + * This is a long lived object: it has the same life time as an XrSystemID. */ struct xrt_system_compositor { - //! Info regarding the system. - struct xrt_system_compositor_info info; - /*! - * Does this system compositor support multi client controls. + * An optional aspect/additional interface, providing multi-app control. + * Populated if this system compositor supports multi client controls. */ struct xrt_multi_compositor_control *xmcc; + //! Info regarding the system. + struct xrt_system_compositor_info info; + /*! * Create a new native compositor. * * This signals that you want to start XR, and as such implicitly brings * up a new session. Does not "call" `xrBeginSession`. * - * Some system compositors might only support that one `xrt_compositor` - * is active at a time, will return `XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED` + * Some system compositors might only support one `xrt_compositor` + * active at a time, will return `XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED` * if this is the case. + * + * In a multi-session capable system compositor, this may return a "proxy" + * for feeding a single client's layers to a compositor or a layer merging mechanism, + * rather than a raw native compositor (not wrapping or forwarding) directly. */ xrt_result_t (*create_native_compositor)(struct xrt_system_compositor *xsc, const struct xrt_session_info *xsi, @@ -2007,6 +2022,9 @@ struct xrt_system_compositor * * Helper for calling through the function pointer. * + * If the system compositor @p xsc does not implement @ref xrt_multi_composition_control, + * this returns @ref XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED. + * * @public @memberof xrt_system_compositor */ static inline xrt_result_t @@ -2024,6 +2042,9 @@ xrt_syscomp_set_state(struct xrt_system_compositor *xsc, struct xrt_compositor * * * Helper for calling through the function pointer. * + * If the system compositor @p xsc does not implement @ref xrt_multi_composition_control, + * this returns @ref XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED. + * * @public @memberof xrt_system_compositor */ static inline xrt_result_t @@ -2042,6 +2063,9 @@ xrt_syscomp_set_z_order(struct xrt_system_compositor *xsc, struct xrt_compositor * * Helper for calling through the function pointer. * + * If the system compositor @p xsc does not implement @ref xrt_multi_composition_control, + * this returns @ref XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED. + * * @public @memberof xrt_system_compositor */ static inline xrt_result_t diff --git a/src/xrt/include/xrt/xrt_gfx_native.h b/src/xrt/include/xrt/xrt_gfx_native.h index 063983221..5dc89f1f4 100644 --- a/src/xrt/include/xrt/xrt_gfx_native.h +++ b/src/xrt/include/xrt/xrt_gfx_native.h @@ -1,4 +1,4 @@ -// Copyright 2019-2020, Collabora, Ltd. +// Copyright 2019-2022, Collabora, Ltd. // SPDX-License-Identifier: BSL-1.0 /*! * @file @@ -16,11 +16,8 @@ extern "C" { #endif - -struct time_state; - /*! - * Creates the main system compositor. + * Creates the main system compositor (and its nested native compositor). * * @ingroup xrt_iface * @relates xrt_system_compositor diff --git a/src/xrt/ipc/client/ipc_client_compositor.c b/src/xrt/ipc/client/ipc_client_compositor.c index 561c1b4d8..a1fbdeef1 100644 --- a/src/xrt/ipc/client/ipc_client_compositor.c +++ b/src/xrt/ipc/client/ipc_client_compositor.c @@ -973,6 +973,14 @@ ipc_syscomp_destroy(struct xrt_system_compositor *xsc) * */ +/*! + * + * + * This actually creates an IPC client "native" compositor with deferred initialization. + * It owns a special implementation of the @ref xrt_system_compositor interface + * whose "create_native_compositor" method actually completes the deferred initialization + * of the compositor, effectively finishing creation of a compositor IPC proxy. + */ int ipc_client_create_system_compositor(struct ipc_connection *ipc_c, struct xrt_image_native_allocator *xina,