refactor!: Switch to simple unsigned 64-bit integer for node IDs (#276)

Author: mwcampbell

Cargo.lock

@@ -12,11 +12,11 @@
 
 const char WINDOW_TITLE[] = "Hello world";
 
-static accesskit_node_id WINDOW_ID;
-static accesskit_node_id BUTTON_1_ID;
-static accesskit_node_id BUTTON_2_ID;
-static accesskit_node_id ANNOUNCEMENT_ID;
-static accesskit_node_id INITIAL_FOCUS;
+const accesskit_node_id WINDOW_ID = 0;
+const accesskit_node_id BUTTON_1_ID = 1;
+const accesskit_node_id BUTTON_2_ID = 2;
+const accesskit_node_id ANNOUNCEMENT_ID = 3;
+const accesskit_node_id INITIAL_FOCUS = BUTTON_1_ID;
 
 const accesskit_rect BUTTON_1_RECT = {20.0, 20.0, 100.0, 60.0};
 
@@ -25,21 +25,10 @@ const accesskit_rect BUTTON_2_RECT = {20.0, 60.0, 100.0, 100.0};
 const Sint32 SET_FOCUS_MSG = 0;
 const Sint32 DO_DEFAULT_ACTION_MSG = 1;
 
-const bool node_id_cmp(const accesskit_node_id *id1,
-                       const accesskit_node_id *id2) {
-  return memcmp(id1, id2, sizeof(accesskit_node_id)) == 0;
-}
-
-accesskit_node_id *node_id_dup(const accesskit_node_id *src) {
-  accesskit_node_id *result = malloc(sizeof(accesskit_node_id));
-  memcpy(result, src, sizeof(accesskit_node_id));
-  return result;
-}
-
 accesskit_node *build_button(accesskit_node_id id, const char *name,
                              accesskit_node_class_set *classes) {
   accesskit_rect rect;
-  if (node_id_cmp(&id, &BUTTON_1_ID)) {
+  if (id == BUTTON_1_ID) {
     rect = BUTTON_1_RECT;
   } else {
     rect = BUTTON_2_RECT;
@@ -260,7 +249,7 @@ void window_state_press_button(struct window_state *state,
                                const struct accesskit_sdl_adapter *adapter,
                                accesskit_node_id id) {
   const char *text;
-  if (node_id_cmp(&id, &BUTTON_1_ID)) {
+  if (id == BUTTON_1_ID) {
     text = "You pressed button 1";
   } else {
     text = "You pressed button 2";
@@ -294,7 +283,7 @@ void do_action(const accesskit_action_request *request, void *userdata) {
   SDL_zero(event);
   event.type = state->event_type;
   event.user.windowID = state->window_id;
-  event.user.data1 = node_id_dup(&request->target);
+  event.user.data1 = (void *)((uintptr_t)(request->target));
   if (request->action == ACCESSKIT_ACTION_FOCUS) {
     event.user.code = SET_FOCUS_MSG;
     SDL_PushEvent(&event);
@@ -334,11 +323,6 @@ int main(int argc, char *argv[]) {
     fprintf(stderr, "Couldn't register user event: (%s)\n", SDL_GetError());
     return -1;
   }
-  WINDOW_ID = accesskit_node_id_new(1).value;
-  BUTTON_1_ID = accesskit_node_id_new(2).value;
-  BUTTON_2_ID = accesskit_node_id_new(3).value;
-  ANNOUNCEMENT_ID = accesskit_node_id_new(4).value;
-  INITIAL_FOCUS = BUTTON_1_ID;
 
   struct window_state state;
   window_state_init(&state);
@@ -386,7 +370,7 @@ int main(int argc, char *argv[]) {
       switch (event.key.keysym.sym) {
         case SDLK_TAB:
           window_state_lock(&state);
-          if (node_id_cmp(&state.focus, &BUTTON_1_ID)) {
+          if (state.focus == BUTTON_1_ID) {
             state.focus = BUTTON_2_ID;
           } else {
             state.focus = BUTTON_1_ID;
@@ -401,18 +385,19 @@ int main(int argc, char *argv[]) {
           window_state_unlock(&state);
           break;
       }
-    } else if (event.type == user_event && event.user.windowID == window_id &&
-               (node_id_cmp(event.user.data1, &BUTTON_1_ID) ||
-                node_id_cmp(event.user.data1, &BUTTON_2_ID))) {
-      window_state_lock(&state);
-      accesskit_node_id *target = event.user.data1;
-      if (event.user.code == SET_FOCUS_MSG) {
-        state.focus = *target;
-        window_state_update_focus(&state, &adapter);
-      } else if (event.user.code == DO_DEFAULT_ACTION_MSG) {
-        window_state_press_button(&state, &adapter, *target);
+    } else if (event.type == user_event && event.user.windowID == window_id) {
+      accesskit_node_id target =
+          (accesskit_node_id)((uintptr_t)(event.user.data1));
+      if (target == BUTTON_1_ID || target == BUTTON_2_ID) {
+        window_state_lock(&state);
+        if (event.user.code == SET_FOCUS_MSG) {
+          state.focus = target;
+          window_state_update_focus(&state, &adapter);
+        } else if (event.user.code == DO_DEFAULT_ACTION_MSG) {
+          window_state_press_button(&state, &adapter, target);
+        }
+        window_state_unlock(&state);
       }
-      window_state_unlock(&state);
     }
   }
 

bindings/c/examples/sdl/hello_world.c

@@ -7,11 +7,11 @@ const WCHAR CLASS_NAME[] = L"AccessKitTest";
 
 const WCHAR WINDOW_TITLE[] = L"Hello world";
 
-static accesskit_node_id WINDOW_ID;
-static accesskit_node_id BUTTON_1_ID;
-static accesskit_node_id BUTTON_2_ID;
-static accesskit_node_id ANNOUNCEMENT_ID;
-static accesskit_node_id INITIAL_FOCUS;
+const accesskit_node_id WINDOW_ID = 0;
+const accesskit_node_id BUTTON_1_ID = 1;
+const accesskit_node_id BUTTON_2_ID = 2;
+const accesskit_node_id ANNOUNCEMENT_ID = 3;
+const accesskit_node_id INITIAL_FOCUS = BUTTON_1_ID;
 
 const accesskit_rect BUTTON_1_RECT = {20.0, 20.0, 100.0, 60.0};
 
@@ -20,21 +20,10 @@ const accesskit_rect BUTTON_2_RECT = {20.0, 60.0, 100.0, 100.0};
 const uint32_t SET_FOCUS_MSG = WM_USER;
 const uint32_t DO_DEFAULT_ACTION_MSG = WM_USER + 1;
 
-const bool node_id_cmp(const accesskit_node_id *id1,
-                       const accesskit_node_id *id2) {
-  return memcmp(id1, id2, sizeof(accesskit_node_id)) == 0;
-}
-
-accesskit_node_id *node_id_dup(const accesskit_node_id *src) {
-  accesskit_node_id *result = malloc(sizeof(accesskit_node_id));
-  memcpy(result, src, sizeof(accesskit_node_id));
-  return result;
-}
-
 accesskit_node *build_button(accesskit_node_id id, const char *name,
                              accesskit_node_class_set *classes) {
   accesskit_rect rect;
-  if (node_id_cmp(&id, &BUTTON_1_ID)) {
+  if (id == BUTTON_1_ID) {
     rect = BUTTON_1_RECT;
   } else {
     rect = BUTTON_2_RECT;
@@ -122,10 +111,10 @@ accesskit_tree_update *window_state_build_initial_tree(
 void do_action(const accesskit_action_request *request, void *userdata) {
   HWND window = userdata;
   if (request->action == ACCESSKIT_ACTION_FOCUS) {
-    LPARAM lparam = (LPARAM)node_id_dup(&request->target);
+    LPARAM lparam = (LPARAM)(request->target);
     PostMessage((HWND)window, SET_FOCUS_MSG, 0, lparam);
   } else if (request->action == ACCESSKIT_ACTION_DEFAULT) {
-    LPARAM lparam = (LPARAM)node_id_dup(&request->target);
+    LPARAM lparam = (LPARAM)(request->target);
     PostMessage((HWND)window, DO_DEFAULT_ACTION_MSG, 0, lparam);
   }
 }
@@ -149,7 +138,7 @@ accesskit_windows_adapter *window_state_get_or_init_accesskit_adapter(
 void window_state_press_button(struct window_state *state,
                                accesskit_node_id id) {
   const char *text;
-  if (node_id_cmp(&id, &BUTTON_1_ID)) {
+  if (id == BUTTON_1_ID) {
     text = "You pressed button 1";
   } else {
     text = "You pressed button 2";
@@ -238,7 +227,7 @@ LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
   } else if (msg == WM_KEYDOWN) {
     if (wParam == VK_TAB) {
       struct window_state *state = get_window_state(hwnd);
-      if (node_id_cmp(&state->focus, &BUTTON_1_ID)) {
+      if (state->focus == BUTTON_1_ID) {
         state->focus = BUTTON_2_ID;
       } else {
         state->focus = BUTTON_1_ID;
@@ -252,21 +241,19 @@ LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
       return DefWindowProc(hwnd, msg, wParam, lParam);
     }
   } else if (msg == SET_FOCUS_MSG) {
-    accesskit_node_id *id = (accesskit_node_id *)lParam;
-    if (node_id_cmp(id, &BUTTON_1_ID) || node_id_cmp(id, &BUTTON_2_ID)) {
+    accesskit_node_id id = (accesskit_node_id)lParam;
+    if (id == BUTTON_1_ID || id == BUTTON_2_ID) {
       struct window_state *state = get_window_state(hwnd);
-      state->focus = *id;
+      state->focus = id;
       bool is_window_focused = state->is_window_focused;
       update_focus(hwnd, is_window_focused);
     }
-    free(id);
   } else if (msg == DO_DEFAULT_ACTION_MSG) {
-    accesskit_node_id *id = (accesskit_node_id *)lParam;
-    if (node_id_cmp(id, &BUTTON_1_ID) || node_id_cmp(id, &BUTTON_2_ID)) {
+    accesskit_node_id id = (accesskit_node_id)lParam;
+    if (id == BUTTON_1_ID || id == BUTTON_2_ID) {
       struct window_state *window_state = get_window_state(hwnd);
-      window_state_press_button(window_state, *id);
+      window_state_press_button(window_state, id);
     }
-    free(id);
   } else {
     return DefWindowProc(hwnd, msg, wParam, lParam);
   }
@@ -314,12 +301,6 @@ int main() {
     return 0;
   }
 
-  WINDOW_ID = accesskit_node_id_new(1).value;
-  BUTTON_1_ID = accesskit_node_id_new(2).value;
-  BUTTON_2_ID = accesskit_node_id_new(3).value;
-  ANNOUNCEMENT_ID = accesskit_node_id_new(4).value;
-  INITIAL_FOCUS = BUTTON_1_ID;
-
   hwnd = create_window(WINDOW_TITLE, INITIAL_FOCUS);
 
   if (hwnd == NULL) {

bindings/c/examples/windows/hello_world.c

@@ -9,7 +9,6 @@ use paste::paste;
 use std::{
     ffi::{CStr, CString},
     mem,
-    num::NonZeroU128,
     os::raw::{c_char, c_void},
     ptr, slice,
 };
@@ -318,24 +317,7 @@ macro_rules! vec_property_methods {
     }
 }
 
-/// Call `accesskit_node_id_new` to create this struct.
-///
-/// If you have to manually populate this, ensure it is not filled up only with zeroes.
-#[derive(Clone, Copy)]
-#[repr(C)]
-pub struct node_id([u8; 16]);
-
-impl From<NodeId> for node_id {
-    fn from(id: NodeId) -> Self {
-        Self(id.0.get().to_le_bytes())
-    }
-}
-
-impl From<node_id> for NodeId {
-    fn from(id: node_id) -> Self {
-        Self(unsafe { NonZeroU128::new_unchecked(u128::from_le_bytes(id.0)) })
-    }
-}
+pub type node_id = u64;
 
 slice_struct! { node_ids, NodeId, node_id }
 
@@ -860,13 +842,6 @@ vec_property_methods! {
     (CustomAction, custom_actions, *mut custom_actions, set_custom_actions, custom_action, push_custom_action, clear_custom_actions)
 }
 
-impl node_id {
-    #[no_mangle]
-    pub extern "C" fn accesskit_node_id_new(id: u64) -> opt_node_id {
-        NonZeroU128::new(id as u128).map(NodeId).into()
-    }
-}
-
 impl node_builder {
     #[no_mangle]
     pub extern "C" fn accesskit_node_builder_new(role: Role) -> *mut node_builder {

bindings/c/src/common.rs

@@ -22,12 +22,7 @@ use serde::{
     ser::{SerializeMap, SerializeSeq, Serializer},
     Deserialize, Serialize,
 };
-use std::{
-    collections::BTreeSet,
-    num::{NonZeroU128, NonZeroU64},
-    ops::DerefMut,
-    sync::Arc,
-};
+use std::{collections::BTreeSet, ops::DerefMut, sync::Arc};
 #[cfg(feature = "serde")]
 use std::{fmt, mem::size_of_val};
 
@@ -690,9 +685,7 @@ pub enum TextDecoration {
     Wavy,
 }
 
-// This is NonZeroU128 because we regularly store Option<NodeId>.
-// 128-bit to handle UUIDs.
-pub type NodeIdContent = NonZeroU128;
+pub type NodeIdContent = u64;
 
 /// The stable identity of a [`Node`], unique within the node's tree.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
@@ -701,10 +694,17 @@ pub type NodeIdContent = NonZeroU128;
 #[repr(transparent)]
 pub struct NodeId(pub NodeIdContent);
 
-impl From<NonZeroU64> for NodeId {
+impl From<NodeIdContent> for NodeId {
+    #[inline]
+    fn from(inner: NodeIdContent) -> Self {
+        Self(inner)
+    }
+}
+
+impl From<NodeId> for NodeIdContent {
     #[inline]
-    fn from(inner: NonZeroU64) -> Self {
-        Self(inner.into())
+    fn from(outer: NodeId) -> Self {
+        outer.0
     }
 }
 

common/src/lib.rs

@@ -23,25 +23,22 @@ mod tests {
     use accesskit::{
         Affine, NodeBuilder, NodeClassSet, NodeId, Rect, Role, Tree, TreeUpdate, Vec2,
     };
-    use std::num::NonZeroU128;
 
     use crate::FilterResult;
 
-    pub const ROOT_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(1) });
-    pub const PARAGRAPH_0_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(2) });
-    pub const STATIC_TEXT_0_0_IGNORED_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(3) });
-    pub const PARAGRAPH_1_IGNORED_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(4) });
-    pub const STATIC_TEXT_1_0_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(5) });
-    pub const PARAGRAPH_2_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(6) });
-    pub const STATIC_TEXT_2_0_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(7) });
-    pub const PARAGRAPH_3_IGNORED_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(8) });
-    pub const EMPTY_CONTAINER_3_0_IGNORED_ID: NodeId =
-        NodeId(unsafe { NonZeroU128::new_unchecked(9) });
-    pub const LINK_3_1_IGNORED_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(10) });
-    pub const STATIC_TEXT_3_1_0_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(11) });
-    pub const BUTTON_3_2_ID: NodeId = NodeId(unsafe { NonZeroU128::new_unchecked(12) });
-    pub const EMPTY_CONTAINER_3_3_IGNORED_ID: NodeId =
-        NodeId(unsafe { NonZeroU128::new_unchecked(13) });
+    pub const ROOT_ID: NodeId = NodeId(0);
+    pub const PARAGRAPH_0_ID: NodeId = NodeId(1);
+    pub const STATIC_TEXT_0_0_IGNORED_ID: NodeId = NodeId(2);
+    pub const PARAGRAPH_1_IGNORED_ID: NodeId = NodeId(3);
+    pub const STATIC_TEXT_1_0_ID: NodeId = NodeId(4);
+    pub const PARAGRAPH_2_ID: NodeId = NodeId(5);
+    pub const STATIC_TEXT_2_0_ID: NodeId = NodeId(6);
+    pub const PARAGRAPH_3_IGNORED_ID: NodeId = NodeId(7);
+    pub const EMPTY_CONTAINER_3_0_IGNORED_ID: NodeId = NodeId(8);
+    pub const LINK_3_1_IGNORED_ID: NodeId = NodeId(9);
+    pub const STATIC_TEXT_3_1_0_ID: NodeId = NodeId(10);
+    pub const BUTTON_3_2_ID: NodeId = NodeId(11);
+    pub const EMPTY_CONTAINER_3_3_IGNORED_ID: NodeId = NodeId(12);
 
     pub fn test_tree() -> crate::tree::Tree {
         let mut classes = NodeClassSet::new();