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.

Sign up for GitHub

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

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

Merged
merged 10 commits into from
Aug 18, 2023
+228 −321
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9e1b288
refactor!: Switch to simple unsigned 64-bit integer for node IDs
mwcampbell Aug 18, 2023
2f17c4a
More optimal algorithm for constructing UIA runtime IDs, that can act…
mwcampbell Aug 18, 2023
3721471
Show in our examples that a node ID of 0 is now OK
mwcampbell Aug 18, 2023
4c515aa
Also change node IDs in our end-to-end Windows tests
mwcampbell Aug 18, 2023
92b5f51
Further simplify conversion to UIA runtime IDs; the priority should b…
mwcampbell Aug 18, 2023
649c51d
Further optimize conversion to UIA runtime IDs. Now assumes a 64-bit …
mwcampbell Aug 18, 2023
7cffda5
Remove unnecessary imports of TryInto
mwcampbell Aug 18, 2023
b2831bc
Zero-based node IDs in all tests
mwcampbell Aug 18, 2023
4e6a8b7
Dummy changes to the Unix and macOS crates to make sure they get brea…
mwcampbell Aug 18, 2023
e96d15c
reformat
mwcampbell Aug 18, 2023
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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 21 additions & 36 deletions bindings/c/examples/sdl/hello_world.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down
51 changes: 16 additions & 35 deletions bindings/c/examples/windows/hello_world.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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";
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down
27 changes: 1 addition & 26 deletions bindings/c/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use paste::paste;
use std::{
ffi::{CStr, CString},
mem,
num::NonZeroU128,
os::raw::{c_char, c_void},
ptr, slice,
};
Expand Down Expand Up @@ -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 }

Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 12 additions & 12 deletions common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)]
Expand All @@ -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
}
}

Expand Down
29 changes: 13 additions & 16 deletions consumer/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading