From 17db3db29668a4c28f57350279ff077d45360fc2 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Thu, 19 Nov 2020 09:48:23 +0000 Subject: [PATCH] visibility: Centralize keyboard panel visibility policy and handling With the policy being disentangled from application, it becomes testable. This prepares for moving the entire visibility mechanism to the new class and taking away more pieces of ServerContextService. In addition, this is a good warmup before trying to implement sizing policy. --- src/imservice.c | 3 +- src/imservice.rs | 37 ++------ src/server-context-service.c | 43 ++++----- src/server-context-service.h | 3 +- src/server-main.c | 11 ++- src/submission.h | 4 +- src/submission.rs | 33 +++---- src/ui_manager.h | 6 ++ src/ui_manager.rs | 170 ++++++++++++++++++++++++++++++++++- 9 files changed, 223 insertions(+), 87 deletions(-) diff --git a/src/imservice.c b/src/imservice.c index b6b04784..25dc65e4 100644 --- a/src/imservice.c +++ b/src/imservice.c @@ -25,6 +25,7 @@ static const struct zwp_input_method_v2_listener input_method_listener = { struct submission* get_submission(struct zwp_input_method_manager_v2 *immanager, struct zwp_virtual_keyboard_manager_v1 *vkmanager, + struct vis_manager *vis_manager, struct wl_seat *seat, EekboardContextService *state) { struct zwp_input_method_v2 *im = NULL; @@ -35,7 +36,7 @@ struct submission* get_submission(struct zwp_input_method_manager_v2 *immanager, if (vkmanager) { vk = zwp_virtual_keyboard_manager_v1_create_virtual_keyboard(vkmanager, seat); } - return submission_new(im, vk, state); + return submission_new(im, vk, state, vis_manager); } /// Un-inlined diff --git a/src/imservice.rs b/src/imservice.rs index a25edd5e..462c4203 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -6,7 +6,6 @@ use std::boxed::Box; use std::ffi::CString; use std::fmt; -use std::mem; use std::num::Wrapping; use std::string::String; @@ -24,7 +23,7 @@ pub mod c { use std::os::raw::{c_char, c_void}; - pub use ::submission::c::UIManager; + pub use ::ui_manager::c::UIManager; pub use ::submission::c::StateManager; // The following defined in C @@ -42,8 +41,6 @@ pub mod c { pub fn eek_input_method_delete_surrounding_text(im: *mut InputMethod, before: u32, after: u32); pub fn eek_input_method_commit(im: *mut InputMethod, serial: u32); fn eekboard_context_service_set_hint_purpose(state: *const StateManager, hint: u32, purpose: u32); - pub fn server_context_service_set_im_active(imservice: *const UIManager, active: u32); - pub fn server_context_service_keyboard_release_visibility(imservice: *const UIManager); } // The following defined in Rust. TODO: wrap naked pointers to Rust data inside RefCells to prevent multiple writers @@ -153,7 +150,7 @@ pub mod c { }; if active_changed { - imservice.apply_active_to_ui(); + (imservice.active_callback)(imservice.current.active); if imservice.current.active { unsafe { eekboard_context_service_set_hint_purpose( @@ -179,9 +176,7 @@ pub mod c { // the keyboard is already decommissioned imservice.current.active = false; - if let Some(ui) = imservice.ui_manager { - unsafe { server_context_service_keyboard_release_visibility(ui); } - } + (imservice.active_callback)(imservice.current.active); } // FIXME: destroy and deallocate @@ -334,8 +329,7 @@ pub struct IMService { pub im: *mut c::InputMethod, /// Unowned reference. Be careful, it's shared with C at large state_manager: *const c::StateManager, - /// Unowned reference. Be careful, it's shared with C at large - ui_manager: Option<*const c::UIManager>, + active_callback: Box, pending: IMProtocolState, current: IMProtocolState, // turn current into an idiomatic representation? @@ -352,12 +346,13 @@ impl IMService { pub fn new( im: *mut c::InputMethod, state_manager: *const c::StateManager, + active_callback: Box, ) -> Box { // IMService will be referenced to by C, // so it needs to stay in the same place in memory via Box let imservice = Box::new(IMService { im, - ui_manager: None, + active_callback, state_manager, pending: IMProtocolState::default(), current: IMProtocolState::default(), @@ -373,26 +368,6 @@ impl IMService { imservice } - pub fn set_ui_manager(&mut self, mut ui_manager: Option<*const c::UIManager>) { - mem::swap(&mut self.ui_manager, &mut ui_manager); - // Now ui_manager is what was previously self.ui_manager. - // If there wasn't any, we need to consider if UI was requested. - if let None = ui_manager { - self.apply_active_to_ui(); - } - } - - fn apply_active_to_ui(&self) { - if let Some(ui) = self.ui_manager { - unsafe { - c::server_context_service_set_im_active( - ui, - self.is_active() as u32, - ); - } - } - } - pub fn commit_string(&self, text: &CString) -> Result<(), SubmitError> { match self.current.active { true => { diff --git a/src/server-context-service.c b/src/server-context-service.c index 6923ab0b..44364aec 100644 --- a/src/server-context-service.c +++ b/src/server-context-service.c @@ -43,10 +43,9 @@ struct _ServerContextService { struct submission *submission; // unowned struct squeek_layout_state *layout; struct ui_manager *manager; // unowned + struct vis_manager *vis_manager; // owned gboolean visible; - gboolean enabled; - gboolean im_active; PhoshLayerSurface *window; GtkWidget *widget; // nullable guint hiding; @@ -288,7 +287,7 @@ server_context_service_hide_keyboard (ServerContextService *self) /// In this case, the user doesn't really need the keyboard surface /// to disappear completely. void -server_context_service_keyboard_release_visibility (ServerContextService *self) +server_context_service_release_visibility (ServerContextService *self) { g_return_if_fail (SERVER_IS_CONTEXT_SERVICE(self)); @@ -297,6 +296,13 @@ server_context_service_keyboard_release_visibility (ServerContextService *self) } } +static void +server_context_service_set_physical_keyboard_present (ServerContextService *self, gboolean physical_keyboard_present) +{ + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); + squeek_visman_set_keyboard_present(self->vis_manager, physical_keyboard_present); +} + static void server_context_service_set_property (GObject *object, guint prop_id, @@ -310,7 +316,7 @@ server_context_service_set_property (GObject *object, self->visible = g_value_get_boolean (value); break; case PROP_ENABLED: - server_context_service_set_enabled (self, g_value_get_boolean (value)); + server_context_service_set_physical_keyboard_present (self, !g_value_get_boolean (value)); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -385,12 +391,14 @@ server_context_service_class_init (ServerContextServiceClass *klass) } static void -server_context_service_init (ServerContextService *self) { +server_context_service_init (ServerContextService *self) {} + +static void +init (ServerContextService *self) { const char *schema_name = "org.gnome.desktop.a11y.applications"; GSettingsSchemaSource *ssrc = g_settings_schema_source_get_default(); g_autoptr(GSettingsSchema) schema = NULL; - self->enabled = TRUE; if (!ssrc) { g_warning("No gsettings schemas installed."); return; @@ -407,37 +415,24 @@ server_context_service_init (ServerContextService *self) { } ServerContextService * -server_context_service_new (EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman) +server_context_service_new (EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman, struct vis_manager *visman) { ServerContextService *ui = g_object_new (SERVER_TYPE_CONTEXT_SERVICE, NULL); ui->submission = submission; ui->state = self; ui->layout = layout; ui->manager = uiman; + ui->vis_manager = visman; + init(ui); return ui; } void -server_context_service_update_visible (ServerContextService *self, gboolean delay) { - if (self->enabled && self->im_active) { +server_context_service_update_visible (ServerContextService *self, gboolean visible) { + if (visible) { server_context_service_show_keyboard(self); - } else if (delay) { - server_context_service_keyboard_release_visibility(self); } else { server_context_service_hide_keyboard(self); } } -void -server_context_service_set_enabled (ServerContextService *self, gboolean enabled) -{ - g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); - self->enabled = enabled; - server_context_service_update_visible(self, FALSE); -} - -void -server_context_service_set_im_active(ServerContextService *self, uint32_t active) { - self->im_active = active; - server_context_service_update_visible(self, TRUE); -} diff --git a/src/server-context-service.h b/src/server-context-service.h index a091d0f9..99aca139 100644 --- a/src/server-context-service.h +++ b/src/server-context-service.h @@ -29,11 +29,10 @@ G_BEGIN_DECLS /** Manages the lifecycle of the window displaying layouts. */ G_DECLARE_FINAL_TYPE (ServerContextService, server_context_service, SERVER, CONTEXT_SERVICE, GObject) -ServerContextService *server_context_service_new(EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman); +ServerContextService *server_context_service_new(EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman, struct vis_manager *visman); enum squeek_arrangement_kind server_context_service_get_layout_type(ServerContextService *); void server_context_service_show_keyboard (ServerContextService *self); void server_context_service_hide_keyboard (ServerContextService *self); -void server_context_service_set_enabled (ServerContextService *self, gboolean enabled); G_END_DECLS #endif /* SERVER_CONTEXT_SERVICE_H */ diff --git a/src/server-main.c b/src/server-main.c index 6149df5b..d199acab 100644 --- a/src/server-main.c +++ b/src/server-main.c @@ -277,8 +277,11 @@ main (int argc, char **argv) } } + struct vis_manager *vis_manager = squeek_visman_new(); + instance.submission = get_submission(instance.wayland.input_method_manager, instance.wayland.virtual_keyboard_manager, + vis_manager, instance.wayland.seat, instance.settings_context); @@ -288,15 +291,15 @@ main (int argc, char **argv) instance.settings_context, instance.submission, &instance.layout_choice, - instance.ui_manager); + instance.ui_manager, + vis_manager); if (!ui_context) { g_error("Could not initialize GUI"); exit(1); } instance.ui_context = ui_context; - if (instance.submission) { - submission_set_ui(instance.submission, instance.ui_context); - } + squeek_visman_set_ui(vis_manager, instance.ui_context); + if (instance.dbus_handler) { dbus_handler_set_ui_context(instance.dbus_handler, instance.ui_context); } diff --git a/src/submission.h b/src/submission.h index acb20323..49a15116 100644 --- a/src/submission.h +++ b/src/submission.h @@ -4,17 +4,19 @@ #include "input-method-unstable-v2-client-protocol.h" #include "virtual-keyboard-unstable-v1-client-protocol.h" #include "eek/eek-types.h" +#include "src/ui_manager.h" struct submission; struct squeek_layout; struct submission* get_submission(struct zwp_input_method_manager_v2 *immanager, struct zwp_virtual_keyboard_manager_v1 *vkmanager, + struct vis_manager *vis_manager, struct wl_seat *seat, EekboardContextService *state); // Defined in Rust -struct submission* submission_new(struct zwp_input_method_v2 *im, struct zwp_virtual_keyboard_v1 *vk, EekboardContextService *state); +struct submission* submission_new(struct zwp_input_method_v2 *im, struct zwp_virtual_keyboard_v1 *vk, EekboardContextService *state, struct vis_manager *vis_manager); void submission_set_ui(struct submission *self, ServerContextService *ui_context); void submission_use_layout(struct submission *self, struct squeek_layout *layout, uint32_t time); #endif diff --git a/src/submission.rs b/src/submission.rs index c256de04..f64810f8 100644 --- a/src/submission.rs +++ b/src/submission.rs @@ -24,6 +24,7 @@ use ::imservice; use ::imservice::IMService; use ::keyboard::{ KeyCode, KeyStateId, Modifiers, PressType }; use ::layout; +use ::ui_manager::VisibilityManager; use ::util::vec_remove; use ::vkeyboard; use ::vkeyboard::VirtualKeyboard; @@ -38,14 +39,11 @@ pub mod c { use std::os::raw::c_void; use ::imservice::c::InputMethod; + use ::util::c::Wrapped; use ::vkeyboard::c::ZwpVirtualKeyboardV1; // The following defined in C - /// ServerContextService* - #[repr(transparent)] - pub struct UIManager(*const c_void); - /// EekboardContextService* #[repr(transparent)] pub struct StateManager(*const c_void); @@ -55,12 +53,18 @@ pub mod c { fn submission_new( im: *mut InputMethod, vk: ZwpVirtualKeyboardV1, - state_manager: *const StateManager + state_manager: *const StateManager, + visibility_manager: Wrapped, ) -> *mut Submission { let imservice = if im.is_null() { None } else { - Some(IMService::new(im, state_manager)) + let visibility_manager = visibility_manager.clone_ref(); + Some(IMService::new( + im, + state_manager, + Box::new(move |active| visibility_manager.borrow_mut().set_im_active(active)), + )) }; // TODO: add vkeyboard too Box::::into_raw(Box::new( @@ -75,23 +79,6 @@ pub mod c { )) } - /// Use to initialize the UI reference - #[no_mangle] - pub extern "C" - fn submission_set_ui(submission: *mut Submission, ui_manager: *const UIManager) { - if submission.is_null() { - panic!("Null submission pointer"); - } - let submission: &mut Submission = unsafe { &mut *submission }; - if let Some(ref mut imservice) = &mut submission.imservice { - imservice.set_ui_manager(if ui_manager.is_null() { - None - } else { - Some(ui_manager) - }) - }; - } - #[no_mangle] pub extern "C" fn submission_use_layout( diff --git a/src/ui_manager.h b/src/ui_manager.h index b0c64592..0f95e2e4 100644 --- a/src/ui_manager.h +++ b/src/ui_manager.h @@ -3,6 +3,7 @@ #include +#include "eek/eek-types.h" #include "outputs.h" struct ui_manager; @@ -11,4 +12,9 @@ struct ui_manager *squeek_uiman_new(void); void squeek_uiman_set_output(struct ui_manager *uiman, struct squeek_output_handle output); uint32_t squeek_uiman_get_perceptual_height(struct ui_manager *uiman); +struct vis_manager; + +struct vis_manager *squeek_visman_new(void); +void squeek_visman_set_ui(struct vis_manager *visman, ServerContextService *ui_context); +void squeek_visman_set_keyboard_present(struct vis_manager *visman, uint32_t keyboard_present); #endif diff --git a/src/ui_manager.rs b/src/ui_manager.rs index c7af6521..a9196c1b 100644 --- a/src/ui_manager.rs +++ b/src/ui_manager.rs @@ -10,9 +10,49 @@ use std::cmp::min; use ::outputs::c::OutputHandle; -mod c { +pub mod c { use super::*; + use std::os::raw::c_void; use ::util::c::Wrapped; + + /// ServerContextService* + #[repr(transparent)] + pub struct UIManager(*const c_void); + + #[no_mangle] + extern "C" { + pub fn server_context_service_update_visible(imservice: *const UIManager, active: u32); + pub fn server_context_service_release_visibility(imservice: *const UIManager); + } + + #[no_mangle] + pub extern "C" + fn squeek_visman_new() -> Wrapped { + Wrapped::new(VisibilityManager { + ui_manager: None, + visibility_state: VisibilityFactors { + im_active: false, + physical_keyboard_present: false, + } + }) + } + + /// Use to initialize the UI reference + #[no_mangle] + pub extern "C" + fn squeek_visman_set_ui(visman: Wrapped, ui_manager: *const UIManager) { + let visman = visman.clone_ref(); + let mut visman = visman.borrow_mut(); + visman.set_ui_manager(Some(ui_manager)) + } + + #[no_mangle] + pub extern "C" + fn squeek_visman_set_keyboard_present(visman: Wrapped, present: u32) { + let visman = visman.clone_ref(); + let mut visman = visman.borrow_mut(); + visman.set_keyboard_present(present != 0) + } #[no_mangle] pub extern "C" @@ -79,3 +119,131 @@ impl Manager { } } } + +#[derive(PartialEq, Debug)] +enum Visibility { + Hidden, + Visible, +} + +#[derive(Debug)] +enum VisibilityTransition { + /// Hide immediately + Hide, + /// Hide if no show request comes soon + Release, + /// Show instantly + Show, + /// Don't do anything + NoTransition, +} + +/// Contains visibility policy +#[derive(Clone, Debug)] +struct VisibilityFactors { + im_active: bool, + physical_keyboard_present: bool, +} + +impl VisibilityFactors { + /// Static policy. + /// Use when transitioning from an undefined state (e.g. no UI before). + fn desired(&self) -> Visibility { + match self { + VisibilityFactors { + im_active: true, + physical_keyboard_present: false, + } => Visibility::Visible, + _ => Visibility::Hidden, + } + } + /// Stateful policy + fn transition_to(&self, next: &Self) -> VisibilityTransition { + use self::Visibility::*; + let im_deactivation = self.im_active && !next.im_active; + match (self.desired(), next.desired(), im_deactivation) { + (Visible, Hidden, true) => VisibilityTransition::Release, + (Visible, Hidden, _) => VisibilityTransition::Hide, + (Hidden, Visible, _) => VisibilityTransition::Show, + _ => VisibilityTransition::NoTransition, + } + } +} + +// Temporary struct for migration. Should be integrated with Manager eventually. +pub struct VisibilityManager { + /// Owned reference. Be careful, it's shared with C at large + ui_manager: Option<*const c::UIManager>, + visibility_state: VisibilityFactors, +} + +impl VisibilityManager { + fn set_ui_manager(&mut self, ui_manager: Option<*const c::UIManager>) { + let new = VisibilityManager { + ui_manager, + ..unsafe { self.clone() } + }; + self.apply_changes(new); + } + + fn apply_changes(&mut self, new: Self) { + if let Some(ui) = &new.ui_manager { + if self.ui_manager.is_none() { + // Previous state was never applied, so effectively undefined. + // Just apply the new one. + let new_state = new.visibility_state.desired(); + unsafe { + c::server_context_service_update_visible( + *ui, + (new_state == Visibility::Visible) as u32, + ); + } + } else { + match self.visibility_state.transition_to(&new.visibility_state) { + VisibilityTransition::Hide => unsafe { + c::server_context_service_update_visible(*ui, 0); + }, + VisibilityTransition::Show => unsafe { + c::server_context_service_update_visible(*ui, 1); + }, + VisibilityTransition::Release => unsafe { + c::server_context_service_release_visibility(*ui); + }, + VisibilityTransition::NoTransition => {} + } + } + } + *self = new; + } + + pub fn set_im_active(&mut self, im_active: bool) { + let new = VisibilityManager { + visibility_state: VisibilityFactors { + im_active, + ..self.visibility_state.clone() + }, + ..unsafe { self.clone() } + }; + self.apply_changes(new); + } + + pub fn set_keyboard_present(&mut self, keyboard_present: bool) { + let new = VisibilityManager { + visibility_state: VisibilityFactors { + physical_keyboard_present: keyboard_present, + ..self.visibility_state.clone() + }, + ..unsafe { self.clone() } + }; + self.apply_changes(new); + } + + /// The struct is not really safe to clone due to the ui_manager reference. + /// This is only a helper for getting desired visibility. + unsafe fn clone(&self) -> Self { + VisibilityManager { + ui_manager: self.ui_manager.clone(), + visibility_state: self.visibility_state.clone(), + } + } +}