Skip to content

Commit

Permalink
feat(wm): use device id for monitor index pref
Browse files Browse the repository at this point in the history
This commit begins to build on some of the knowledge shared by EBNull in
allowing users to specify monitor index preferences using physical
device identifiers. This does not presently go all the way to EDIDs, but
the display model and what I believe is a port identifier on the display
adapter(s) can be used to uniquely identify a display in most use cases.

However, I believe I may have unfortunately run into a bug in either
windows-rs or Rust itself, as when the code calling EnumDisplayDevices
is called, it always fails when running a release build, and always
succeeds when running a debug build. This needs to be investigated
further.

re #612
  • Loading branch information
LGUG2Z committed Dec 23, 2023
1 parent 0696a00 commit 657ac44
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 7 deletions.
1 change: 1 addition & 0 deletions komorebi-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ pub enum SocketMessage {
FlipLayout(Axis),
// Monitor and Workspace Commands
MonitorIndexPreference(usize, i32, i32, i32, i32),
DisplayIndexPreference(usize, String),
EnsureWorkspaces(usize, usize),
EnsureNamedWorkspaces(usize, Vec<String>),
NewWorkspace,
Expand Down
2 changes: 2 additions & 0 deletions komorebi/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ lazy_static! {
]));
static ref MONITOR_INDEX_PREFERENCES: Arc<Mutex<HashMap<usize, Rect>>> =
Arc::new(Mutex::new(HashMap::new()));
static ref DISPLAY_INDEX_PREFERENCES: Arc<Mutex<HashMap<usize, String>>> =
Arc::new(Mutex::new(HashMap::new()));
static ref WORKSPACE_RULES: Arc<Mutex<HashMap<String, WorkspaceRule>>> =
Arc::new(Mutex::new(HashMap::new()));
static ref REGEX_IDENTIFIERS: Arc<Mutex<HashMap<String, Regex>>> =
Expand Down
6 changes: 6 additions & 0 deletions komorebi/src/monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ pub struct Monitor {
#[getset(get = "pub", set = "pub")]
name: String,
#[getset(get = "pub", set = "pub")]
device: Option<String>,
#[getset(get = "pub", set = "pub")]
device_id: Option<String>,
#[getset(get = "pub", set = "pub")]
size: Rect,
#[getset(get = "pub", set = "pub")]
work_area_size: Rect,
Expand All @@ -44,6 +48,8 @@ pub fn new(id: isize, size: Rect, work_area_size: Rect, name: String) -> Monitor
Monitor {
id,
name,
device: None,
device_id: None,
size,
work_area_size,
work_area_offset: None,
Expand Down
5 changes: 5 additions & 0 deletions komorebi/src/process_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ use crate::BORDER_OVERFLOW_IDENTIFIERS;
use crate::BORDER_WIDTH;
use crate::CUSTOM_FFM;
use crate::DATA_DIR;
use crate::DISPLAY_INDEX_PREFERENCES;
use crate::FLOAT_IDENTIFIERS;
use crate::HIDING_BEHAVIOUR;
use crate::INITIAL_CONFIGURATION_LOADED;
Expand Down Expand Up @@ -673,6 +674,10 @@ impl WindowManager {
},
);
}
SocketMessage::DisplayIndexPreference(index_preference, ref display) => {
let mut display_index_preferences = DISPLAY_INDEX_PREFERENCES.lock();
display_index_preferences.insert(index_preference, display.clone());
}
SocketMessage::EnsureWorkspaces(monitor_idx, workspace_count) => {
self.ensure_workspaces_for_monitor(monitor_idx, workspace_count)?;
}
Expand Down
10 changes: 10 additions & 0 deletions komorebi/src/static_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::BORDER_WIDTH;
use crate::DATA_DIR;
use crate::DEFAULT_CONTAINER_PADDING;
use crate::DEFAULT_WORKSPACE_PADDING;
use crate::DISPLAY_INDEX_PREFERENCES;
use crate::FLOAT_IDENTIFIERS;
use crate::HIDING_BEHAVIOUR;
use crate::LAYERED_WHITELIST;
Expand Down Expand Up @@ -312,6 +313,9 @@ pub struct StaticConfig {
/// Set monitor index preferences
#[serde(skip_serializing_if = "Option::is_none")]
pub monitor_index_preferences: Option<HashMap<usize, Rect>>,
/// Set display index preferences
#[serde(skip_serializing_if = "Option::is_none")]
pub display_index_preferences: Option<HashMap<usize, String>>,
}

impl From<&WindowManager> for StaticConfig {
Expand Down Expand Up @@ -432,6 +436,7 @@ impl From<&WindowManager> for StaticConfig {
layered_applications: None,
object_name_change_applications: None,
monitor_index_preferences: Option::from(MONITOR_INDEX_PREFERENCES.lock().clone()),
display_index_preferences: Option::from(DISPLAY_INDEX_PREFERENCES.lock().clone()),
}
}
}
Expand All @@ -444,6 +449,11 @@ impl StaticConfig {
*preferences = monitor_index_preferences.clone();
}

if let Some(display_index_preferences) = &self.display_index_preferences {
let mut preferences = DISPLAY_INDEX_PREFERENCES.lock();
*preferences = display_index_preferences.clone();
}

if let Some(behaviour) = self.window_hiding_behaviour {
let mut window_hiding_behaviour = HIDING_BEHAVIOUR.lock();
*window_hiding_behaviour = behaviour;
Expand Down
3 changes: 3 additions & 0 deletions komorebi/src/window_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use crate::workspace::Workspace;
use crate::BORDER_HWND;
use crate::BORDER_OVERFLOW_IDENTIFIERS;
use crate::DATA_DIR;
use crate::DISPLAY_INDEX_PREFERENCES;
use crate::FLOAT_IDENTIFIERS;
use crate::HOME_DIR;
use crate::LAYERED_WHITELIST;
Expand Down Expand Up @@ -103,6 +104,7 @@ pub struct State {
pub border_overflow_identifiers: Vec<IdWithIdentifier>,
pub name_change_on_launch_identifiers: Vec<IdWithIdentifier>,
pub monitor_index_preferences: HashMap<usize, Rect>,
pub display_index_preferences: HashMap<usize, String>,
}

impl AsRef<Self> for WindowManager {
Expand Down Expand Up @@ -132,6 +134,7 @@ impl From<&WindowManager> for State {
border_overflow_identifiers: BORDER_OVERFLOW_IDENTIFIERS.lock().clone(),
name_change_on_launch_identifiers: OBJECT_NAME_CHANGE_ON_LAUNCH.lock().clone(),
monitor_index_preferences: MONITOR_INDEX_PREFERENCES.lock().clone(),
display_index_preferences: DISPLAY_INDEX_PREFERENCES.lock().clone(),
}
}
}
Expand Down
53 changes: 47 additions & 6 deletions komorebi/src/windows_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ use windows::Win32::Graphics::Dwm::DWM_CLOAKED_APP;
use windows::Win32::Graphics::Dwm::DWM_CLOAKED_INHERITED;
use windows::Win32::Graphics::Dwm::DWM_CLOAKED_SHELL;
use windows::Win32::Graphics::Gdi::CreateSolidBrush;
use windows::Win32::Graphics::Gdi::EnumDisplayDevicesA;
use windows::Win32::Graphics::Gdi::EnumDisplayMonitors;
use windows::Win32::Graphics::Gdi::GetMonitorInfoW;
use windows::Win32::Graphics::Gdi::InvalidateRect;
use windows::Win32::Graphics::Gdi::MonitorFromPoint;
use windows::Win32::Graphics::Gdi::MonitorFromWindow;
use windows::Win32::Graphics::Gdi::DISPLAY_DEVICEA;
use windows::Win32::Graphics::Gdi::HBRUSH;
use windows::Win32::Graphics::Gdi::HDC;
use windows::Win32::Graphics::Gdi::HMONITOR;
Expand Down Expand Up @@ -93,6 +95,7 @@ use windows::Win32::UI::WindowsAndMessaging::ShowWindow;
use windows::Win32::UI::WindowsAndMessaging::SystemParametersInfoW;
use windows::Win32::UI::WindowsAndMessaging::WindowFromPoint;
use windows::Win32::UI::WindowsAndMessaging::CW_USEDEFAULT;
use windows::Win32::UI::WindowsAndMessaging::EDD_GET_DEVICE_INTERFACE_NAME;
use windows::Win32::UI::WindowsAndMessaging::GWL_EXSTYLE;
use windows::Win32::UI::WindowsAndMessaging::GWL_STYLE;
use windows::Win32::UI::WindowsAndMessaging::GW_HWNDNEXT;
Expand Down Expand Up @@ -164,8 +167,8 @@ impl_from_integer_for_windows_result!(usize, isize, u16, u32, i32);
impl<T, E> From<WindowsResult<T, E>> for Result<T, E> {
fn from(result: WindowsResult<T, E>) -> Self {
match result {
WindowsResult::Err(error) => Self::Err(error),
WindowsResult::Ok(ok) => Self::Ok(ok),
WindowsResult::Err(error) => Err(error),
WindowsResult::Ok(ok) => Ok(ok),
}
}
}
Expand Down Expand Up @@ -221,7 +224,7 @@ impl WindowsApi {
let mut monitors: Vec<(String, isize)> = vec![];
let monitors_ref: &mut Vec<(String, isize)> = monitors.as_mut();
Self::enum_display_monitors(
Option::Some(windows_callbacks::valid_display_monitors),
Some(windows_callbacks::valid_display_monitors),
monitors_ref as *mut Vec<(String, isize)> as isize,
)?;

Expand All @@ -230,9 +233,47 @@ impl WindowsApi {

pub fn load_monitor_information(monitors: &mut Ring<Monitor>) -> Result<()> {
Self::enum_display_monitors(
Option::Some(windows_callbacks::enum_display_monitor),
Some(windows_callbacks::enum_display_monitor),
monitors as *mut Ring<Monitor> as isize,
)
)?;

Ok(())
}

pub fn enum_display_devices(
index: u32,
lp_device: Option<[u8; 32]>,
) -> Result<DISPLAY_DEVICEA> {

This comment has been minimized.

Copy link
@EBNull

EBNull Dec 23, 2023

A quick note on the A or W suffixes: in win32 there are often two APIs that do the same thing, with the only difference being the suffix. If this were in C and using windows.h, #define magic would automatically choose based on the compile time environment - namely, W is "wide" and A is "ascii". Using the A functions is why the from_wide thing I saw you try didn't work. If you s/A/W/g across all function and struct names, you should be able to use from_wide.

This comment has been minimized.

Copy link
@LGUG2Z

LGUG2Z Dec 24, 2023

Author Owner

Thanks for this explanation! I guess it shows that this is my first Windows project 😅 I just switched everything to use the Wide fns here: cf86b2c

#[allow(clippy::option_if_let_else)]
let lp_device = if let Some(lp_device) = lp_device {
PCSTR::from_raw(lp_device.as_ptr())
} else {
PCSTR::null()
};

let mut display_device = DISPLAY_DEVICEA {
cb: u32::try_from(std::mem::size_of::<DISPLAY_DEVICEA>())?,
..Default::default()
};

match unsafe {
EnumDisplayDevicesA(
lp_device,
index,
std::ptr::addr_of_mut!(display_device),
EDD_GET_DEVICE_INTERFACE_NAME,
)
}
.ok()
{
Ok(_) => {}
Err(error) => {
tracing::error!("enum_display_devices: {}", error);
return Err(error.into());
}
}

Ok(display_device)
}

pub fn enum_windows(callback: WNDENUMPROC, callback_data_address: isize) -> Result<()> {
Expand All @@ -245,7 +286,7 @@ impl WindowsApi {
if let Some(workspace) = monitor.workspaces_mut().front_mut() {
// EnumWindows will enumerate through windows on all monitors
Self::enum_windows(
Option::Some(windows_callbacks::enum_window),
Some(windows_callbacks::enum_window),
workspace.containers_mut() as *mut VecDeque<Container> as isize,
)?;

Expand Down
37 changes: 36 additions & 1 deletion komorebi/src/windows_callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::winevent_listener::WINEVENT_CALLBACK_CHANNEL;
use crate::BORDER_COLOUR_CURRENT;
use crate::BORDER_RECT;
use crate::BORDER_WIDTH;
use crate::DISPLAY_INDEX_PREFERENCES;
use crate::MONITOR_INDEX_PREFERENCES;
use crate::TRANSPARENCY_COLOUR;

Expand Down Expand Up @@ -72,7 +73,32 @@ pub extern "system" fn enum_display_monitor(
}
}

if let Ok(m) = WindowsApi::monitor(hmonitor.0) {
let current_index = monitors.elements().len();

if let Ok(mut m) = WindowsApi::monitor(hmonitor.0) {
#[allow(clippy::cast_possible_truncation)]
if let Ok(d) = WindowsApi::enum_display_devices(current_index as u32, None) {
let name = String::from_utf8_lossy(&d.DeviceName);

This comment has been minimized.

Copy link
@EBNull

EBNull Dec 23, 2023

same here (as above) - this isn't encoded in utf8, it's encoded in whatever the local system codepage is. To avoid that nonsense, use the W api versions to get UTF-16 bytes, and I think whatever from_wide you were using should decode that properly.

let clean_name = name
.replace('\u{0000}', "")

This comment has been minimized.

Copy link
@EBNull

EBNull Dec 23, 2023

Would from_wide trim the null bytes?

.trim_start_matches(r"\\.\")
.to_string();

if clean_name.eq(m.name()) {
if let Ok(device) = WindowsApi::enum_display_devices(0, Some(d.DeviceName)) {
let id = String::from_utf8_lossy(&device.DeviceID);
let clean_id = id.replace('\u{0000}', "");

let mut split: Vec<_> = clean_id.split('#').collect();

This comment has been minimized.

Copy link
@EBNull

EBNull Dec 23, 2023

I think I did this differently... I'll check and get back to you. I remember trying hard not to do parsing of this mess.

split.remove(0);
split.remove(split.len() - 1);

m.set_device(Option::from(split[0].to_string()));
m.set_device_id(Option::from(split.join("-")));
}
}
}

let monitor_index_preferences = MONITOR_INDEX_PREFERENCES.lock();
let mut index_preference = None;
for (index, monitor_size) in &*monitor_index_preferences {
Expand All @@ -81,6 +107,15 @@ pub extern "system" fn enum_display_monitor(
}
}

let display_index_preferences = DISPLAY_INDEX_PREFERENCES.lock();
for (index, device) in &*display_index_preferences {
if let Some(known_device) = m.device() {
if device == known_device {
index_preference = Option::from(index);
}
}
}

if let Some(preference) = index_preference {
let current_len = monitors.elements().len();
if *preference > current_len {
Expand Down
17 changes: 17 additions & 0 deletions komorebic/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,14 @@ struct MonitorIndexPreference {
bottom: i32,
}

#[derive(Parser, AhkFunction)]
struct DisplayIndexPreference {
/// Preferred monitor index (zero-indexed)
index_preference: usize,
/// Display name as identified in komorebic state

This comment has been minimized.

Copy link
@EBNull

EBNull Dec 23, 2023

I think I ended up allowing config via regex or wildcard on what I called the logical identity (name, pnp name, serial) - up to you how permissive you want to be.

display: String,
}

#[derive(Parser, AhkFunction)]
struct EnsureWorkspaces {
/// Monitor index (zero-indexed)
Expand Down Expand Up @@ -930,6 +938,9 @@ enum SubCommand {
/// Set the monitor index preference for a monitor identified using its size
#[clap(arg_required_else_help = true)]
MonitorIndexPreference(MonitorIndexPreference),
/// Set the display index preference for a monitor identified using its display name
#[clap(arg_required_else_help = true)]
DisplayIndexPreference(DisplayIndexPreference),
/// Create at least this many workspaces for the specified monitor
#[clap(arg_required_else_help = true)]
EnsureWorkspaces(EnsureWorkspaces),
Expand Down Expand Up @@ -1890,6 +1901,12 @@ Stop-Process -Name:whkd -ErrorAction SilentlyContinue
.as_bytes()?,
)?;
}
SubCommand::DisplayIndexPreference(arg) => {
send_message(
&SocketMessage::DisplayIndexPreference(arg.index_preference, arg.display)
.as_bytes()?,
)?;
}
SubCommand::EnsureWorkspaces(workspaces) => {
send_message(
&SocketMessage::EnsureWorkspaces(workspaces.monitor, workspaces.workspace_count)
Expand Down

0 comments on commit 657ac44

Please sign in to comment.