st/oxr: Fix conformance failure and tidy up action set attached logic

This caused a action set to act as if it has been attached, one might say that
this commit fixes a overly attached action set.

Extreme programmed with Ryan Pavlik, which I also ~~stole~~ borrowed the header
comments from verbatim.

doc/changes/state_trackers/mr.411.md

@@ -0,0 +1,3 @@
+OpenXR: Fix overly attached action sets, which would appear to be attached to
+a session even after the session has been destroyed. Also tidy up comments and
+other logic surrounding this.

src/xrt/state_trackers/oxr/oxr_api_action.c

@@ -66,7 +66,7 @@ oxr_xrAttachSessionActionSets(XrSession session,
 	OXR_VERIFY_ARG_TYPE_AND_NOT_NULL(
 	    &log, bindInfo, XR_TYPE_SESSION_ACTION_SETS_ATTACH_INFO);
 
-	if (sess->actionsAttached) {
+	if (sess->act_set_attachments != NULL) {
 		return oxr_error(&log, XR_ERROR_ACTIONSETS_ALREADY_ATTACHED,
 		                 "(session) has already had action sets "
 		                 "attached, can only attach action sets once.");
@@ -158,7 +158,7 @@ oxr_xrSuggestInteractionProfileBindings(
 		struct oxr_action *act;
 		OXR_VERIFY_ACTION_NOT_NULL(&log, s->action, act);
 
-		if (act->act_set->data->attached) {
+		if (act->act_set->data->ever_attached) {
 			return oxr_error(
 			    &log, XR_ERROR_ACTIONSETS_ALREADY_ATTACHED,
 			    "(suggestedBindings->suggestedBindings[%zu]->"
@@ -220,7 +220,7 @@ oxr_xrGetInputSourceLocalizedName(
 	OXR_VERIFY_SESSION_AND_INIT_LOG(&log, session, sess,
 	                                "xrGetInputSourceLocalizedName");
 
-	if (!sess->actionsAttached) {
+	if (sess->act_set_attachments == NULL) {
 		return oxr_error(
 		    &log, XR_ERROR_ACTIONSET_NOT_ATTACHED,
 		    "ActionSet(s) have not been attached to this session");
@@ -337,7 +337,7 @@ oxr_xrCreateAction(XrActionSet actionSet,
 	OXR_VERIFY_ARG_LOCALIZED_NAME(&log, createInfo->localizedActionName);
 	OXR_VERIFY_ARG_NOT_NULL(&log, action);
 
-	if (act_set->data->attached) {
+	if (act_set->data->ever_attached) {
 		return oxr_error(
 		    &log, XR_ERROR_ACTIONSETS_ALREADY_ATTACHED,
 		    "(actionSet) has been attached and is now immutable");
@@ -552,7 +552,7 @@ oxr_xrEnumerateBoundSourcesForAction(
 	    XR_TYPE_BOUND_SOURCES_FOR_ACTION_ENUMERATE_INFO);
 	OXR_VERIFY_ACTION_NOT_NULL(&log, enumerateInfo->action, act);
 
-	if (!sess->actionsAttached) {
+	if (sess->act_set_attachments == NULL) {
 		return oxr_error(&log, XR_ERROR_ACTIONSET_NOT_ATTACHED,
 		                 "(session) xrAttachSessionActionSets has not "
 		                 "been called on this session.");

src/xrt/state_trackers/oxr/oxr_binding.c

@@ -385,7 +385,7 @@ oxr_action_get_current_interaction_profile(
 {
 	struct oxr_instance *inst = sess->sys->inst;
 
-	if (!sess->actionsAttached) {
+	if (sess->act_set_attachments == NULL) {
 		return oxr_error(log, XR_ERROR_ACTIONSET_NOT_ATTACHED,
 		                 "xrAttachSessionActionSets has not been "
 		                 "called on this session.");

src/xrt/state_trackers/oxr/oxr_input.c

@@ -1117,6 +1117,12 @@ oxr_session_get_action_set_attachment(
 	void *ptr = NULL;
 	*act_set =
 	    XRT_CAST_OXR_HANDLE_TO_PTR(struct oxr_action_set *, actionSet);
+	*act_set_attached = NULL;
+
+	// In case no action_sets have been attached.
+	if (sess->act_sets_attachments_by_key == NULL) {
+		return;
+	}
 
 	int ret = u_hashmap_int_find(sess->act_sets_attachments_by_key,
 	                             (*act_set)->act_set_key, &ptr);
@@ -1173,17 +1179,8 @@ oxr_session_attach_action_sets(struct oxr_logger *log,
 	oxr_find_profile_for_device(log, inst, sess->sys->right, &right);
 	//! @todo add other subaction paths here
 
-	// Before allocating, make sure nothing has been attached yet.
-
-	for (uint32_t i = 0; i < bindInfo->countActionSets; i++) {
-		struct oxr_action_set *act_set = XRT_CAST_OXR_HANDLE_TO_PTR(
-		    struct oxr_action_set *, bindInfo->actionSets[i]);
-		if (act_set->data->attached) {
-			return XR_ERROR_ACTIONSETS_ALREADY_ATTACHED;
-		}
-	}
-
-	// Allocate room for list.
+	// Allocate room for list. No need to check if anything has been
+	// attached the API function does that.
 	sess->num_action_set_attachments = bindInfo->countActionSets;
 	sess->act_set_attachments = U_TYPED_ARRAY_CALLOC(
 	    struct oxr_action_set_attachment, sess->num_action_set_attachments);
@@ -1193,7 +1190,7 @@ oxr_session_attach_action_sets(struct oxr_logger *log,
 		struct oxr_action_set *act_set = XRT_CAST_OXR_HANDLE_TO_PTR(
 		    struct oxr_action_set *, bindInfo->actionSets[i]);
 		struct oxr_action_set_ref *act_set_ref = act_set->data;
-		act_set_ref->attached = true;
+		act_set_ref->ever_attached = true;
 		struct oxr_action_set_attachment *act_set_attached =
 		    &sess->act_set_attachments[i];
 		oxr_action_set_attachment_init(log, sess, act_set,
@@ -1235,8 +1232,6 @@ oxr_session_attach_action_sets(struct oxr_logger *log,
 		sess->right = right->path;
 	}
 
-	sess->actionsAttached = true;
-
 	return oxr_session_success_result(sess);
 }
 

src/xrt/state_trackers/oxr/oxr_objects.h

@@ -1197,8 +1197,12 @@ struct oxr_session
 
 	/*!
 	 * An array of action set attachments that this session owns.
+	 *
+	 * If non-null, this means action sets have been attached to this
+	 * session.
 	 */
 	struct oxr_action_set_attachment *act_set_attachments;
+
 	/*!
 	 * Length of @ref oxr_session::act_set_attachments.
 	 */
@@ -1206,6 +1210,10 @@ struct oxr_session
 
 	/*!
 	 * A map of action set key to action set attachments.
+	 *
+	 * If non-null, this means action sets have been attached to this
+	 * session, since this map points to elements of
+	 * oxr_session::act_set_attachments
 	 */
 	struct u_hashmap_int *act_sets_attachments_by_key;
 
@@ -1214,11 +1222,13 @@ struct oxr_session
 	 *
 	 * The action attachments are actually owned by the action set
 	 * attachments, but we own the action set attachments, so this is OK.
+	 *
+	 * If non-null, this means action sets have been attached to this
+	 * session, since this map points to @p oxr_action_attachment members of
+	 * oxr_session::act_set_attachments elements.
 	 */
 	struct u_hashmap_int *act_attachments_by_key;
 
-	//! Has xrAttachSessionActionSets been called?
-	bool actionsAttached;
 
 	/*!
 	 * Currently bound interaction profile.
@@ -1650,8 +1660,11 @@ struct oxr_action_set_ref
 	//! Application supplied name of this action.
 	char name[XR_MAX_ACTION_SET_NAME_SIZE];
 
-	//! Has this action set been attached.
-	bool attached;
+	/*!
+	 * Has this action set even been attached to any session, marking it as
+	 * immutable.
+	 */
+	bool ever_attached;
 
 	//! Unique key for the session hashmap.
 	uint32_t act_set_key;