xrt,comp,ipc...: Clarifying docs about "system compositor" and "multi compositor"

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
+        for @ref xrt_instance, `xdev` for @ref xrt_device, `xsysd` sometimes
-        used for @ref xrt_system_compositor.
+        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.

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
+compositor is initialized by a system compositor, a shared memory segment for
-initialized, and other internal state is set up. (See `ipc_server_process.c`.)
+device data is initialized, and other internal state is set up. (See
+`ipc_server_process.c`.)
 
 There are three main communication needs:
 

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
+ * Create a "system compositor" that can handle multiple clients (each
- * a single native compositor. Both the native compositor and the pacing factory
+ * through a "multi compositor") and that drives a single native compositor.
- * is owned by the multi compositor and destroyed by it.
+ * 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,

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 <jakob@collabora.com>
  * @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
+	 * The multi-client system compositor has called @ref xrt_comp_end_session
-	 * on it's @ref xrt_compositor_native.
+	 * 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
+	 * The multi-client system compositor has called @ref xrt_comp_begin_session
-	 * on it's @ref xrt_compositor_native.
+	 * on its @ref xrt_compositor_native.
 	 */
 	MULTI_SYSTEM_STATE_RUNNING,
 
 	/*!
-	 * There are no active sessions and the multi compositor system is
+	 * There are no active sessions and the multi-client system compositor is
-	 * drawing on or more clear frames.
+	 * instructing the native compositor to draw one or more clear frames.
 	 *
-	 * The multi system compositor have not yet @ref xrt_comp_begin_session
+	 * The multi-client system compositor has not yet called @ref xrt_comp_begin_session
-	 * on it's @ref xrt_compositor_native.
+	 * on its @ref xrt_compositor_native.
 	 */
 	MULTI_SYSTEM_STATE_STOPPING,
 };
 
 /*!
- * The multi compositor system, multiplexes access to the native compositors
+ * The multi-client system compositor multiplexes access to a single native compositor,
- * and tracks some state needed.
+ * 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 state of the multi-client system compositor.
-		 * the oth thread, aka multi compositor system main thread.
+		 * 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);

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
+ * The system compositor handles composition for a system.
- * XrSystemID.
+ * 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`
+	 * Some system compositors might only support one `xrt_compositor`
-	 * is active at a time, will return `XRT_ERROR_MULTI_SESSION_NOT_IMPLEMENTED`
+	 * 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

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

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,