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.

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

Screen Module, window titlebar, position, size, DPI #1037

Merged
merged 21 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 druid-shell/src/platform/gtk/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ use crate::screen::Monitor;

pub(crate) fn get_monitors() -> Vec<Monitor> {
log::warn!("Screen::get_monitors() is currently unimplemented for gtk.");
Vec::<Monitor>::new()
Vec::new()
}
11 changes: 5 additions & 6 deletions druid-shell/src/platform/gtk/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,14 @@ use crate::keyboard::{KbKey, KeyState, KeyEvent, Modifiers};
use crate::mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
use crate::scale::{Scale, Scalable, ScaledArea};
use crate::window::{IdleToken, Text, TimerToken, WinHandler};
use crate::window;

use super::application::Application;
use super::dialog;
use super::keycodes;
use super::menu::Menu;
use super::util;

use crate::window::WindowState as WindowSizeState; // Avoid name conflict.

/// The platform target DPI.
///
/// GTK considers 96 the default value which represents a 1.0 scale factor.
Expand Down Expand Up @@ -162,7 +161,7 @@ impl WindowBuilder {
log::warn!("WindowBuilder::set_position is currently unimplemented for gtk.");
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
log::warn!("WindowBuilder::set_window_state is currently unimplemented for gtk.");
}

Expand Down Expand Up @@ -581,13 +580,13 @@ impl WindowHandle {
Size::new(0.0, 0.0)
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
log::warn!("WindowHandle::set_window_state is currently unimplemented for gtk.");
}

pub fn get_window_state(&self) -> WindowSizeState {
pub fn get_window_state(&self) -> window::WindowState {
log::warn!("WindowHandle::get_window_state is currently unimplemented for gtk.");
WindowSizeState::RESTORED
window::WindowState::RESTORED
}

pub fn handle_titlebar(&self, _val: bool) {
Expand Down
2 changes: 1 addition & 1 deletion druid-shell/src/platform/mac/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ use crate::screen::Monitor;

pub(crate) fn get_monitors() -> Vec<Monitor> {
log::warn!("Screen::get_monitors() is currently unimplemented for Mac.");
Vec::<Monitor>::new()
Vec::new()
}
10 changes: 5 additions & 5 deletions druid-shell/src/platform/mac/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ use crate::keyboard_types::KeyState;
use crate::mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
use crate::scale::Scale;
use crate::window::{IdleToken, Text, TimerToken, WinHandler};
use crate::window;
use crate::Error;

use crate::window::WindowState as WindowSizeState; // Avoid name conflict.

#[allow(non_upper_case_globals)]
const NSWindowDidBecomeKeyNotification: &str = "NSWindowDidBecomeKeyNotification";
Expand Down Expand Up @@ -153,7 +153,7 @@ impl WindowBuilder {
log::warn!("WindowBuilder::set_position is currently unimplemented for mac platforms.");
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
log::warn!("WindowBuilder::set_window_state is currently unimplemented for mac platforms.");
}

Expand Down Expand Up @@ -866,13 +866,13 @@ impl WindowHandle {
Size::new(0.0, 0.0)
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
log::warn!("WindowHandle::set_window_state is currently unimplemented for Mac.");
}

pub fn get_window_state(&self) -> WindowSizeState {
pub fn get_window_state(&self) -> window::WindowState {
log::warn!("WindowHandle::get_window_state is currently unimplemented for Mac.");
WindowSizeState::RESTORED
window::WindowState::RESTORED
}

pub fn handle_titlebar(&self, _val: bool) {
Expand Down
2 changes: 1 addition & 1 deletion druid-shell/src/platform/web/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ use crate::screen::Monitor;

pub(crate) fn get_monitors() -> Vec<Monitor> {
log::warn!("Screen::get_monitors() is not implemented for web.");
Vec::<Monitor>::new()
Vec::new()
}
11 changes: 5 additions & 6 deletions druid-shell/src/platform/web/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ use crate::scale::{Scale, ScaledArea};
use crate::keyboard::{KbKey, KeyState, Modifiers};
use crate::mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
use crate::window::{IdleToken, Text, TimerToken, WinHandler};

use crate::window::WindowState as WindowSizeState; // Avoid name conflict.
use crate::window;

// This is a macro instead of a function since KeyboardEvent and MouseEvent has identical functions
// to query modifier key states.
Expand Down Expand Up @@ -351,7 +350,7 @@ impl WindowBuilder {
// Ignored
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
// Ignored
}

Expand Down Expand Up @@ -458,13 +457,13 @@ impl WindowHandle {
Size::new(0.0, 0.0)
}

pub fn set_window_state(&self, _state: WindowSizeState) {
pub fn set_window_state(&self, _state: window::WindowState) {
log::warn!("WindowHandle::set_window_state unimplemented for web.");
}

pub fn get_window_state(&self) -> WindowSizeState {
pub fn get_window_state(&self) -> window::WindowState {
log::warn!("WindowHandle::get_window_state unimplemented for web.");
WindowSizeState::RESTORED
window::WindowState::RESTORED
}

pub fn handle_titlebar(&self, _val: bool) {
Expand Down
12 changes: 10 additions & 2 deletions druid-shell/src/platform/windows/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use winapi::shared::ntdef::LPCWSTR;
use winapi::shared::windef::{HCURSOR, HWND, DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2};
use winapi::shared::winerror::HRESULT_FROM_WIN32;
use winapi::um::errhandlingapi::GetLastError;
use winapi::um::shellscalingapi::PROCESS_PER_MONITOR_DPI_AWARE;
use winapi::um::winuser::{
DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostMessageW, PostQuitMessage,
RegisterClassW, TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG,
Expand All @@ -37,7 +38,7 @@ use super::accels;
use super::clipboard::Clipboard;
use super::error::Error;
use super::util::{self, ToWide, CLASS_NAME, OPTIONAL_FUNCTIONS};
use super::window::{self, DS_REQUEST_DESTROY, DS_HANDLE_DROPPED};
use super::window::{self, DS_REQUEST_DESTROY, DS_HANDLE_DEFERRED};

#[derive(Clone)]
pub(crate) struct Application {
Expand All @@ -49,6 +50,8 @@ struct State {
windows: HashSet<HWND>,
}



impl Application {
pub fn new() -> Result<Application, Error> {
Application::init()?;
Expand All @@ -69,6 +72,11 @@ impl Application {
func(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);
}
}
else if let Some(func) = OPTIONAL_FUNCTIONS.SetProcessDpiAwareness {
unsafe {
func(PROCESS_PER_MONITOR_DPI_AWARE);
}
}
unsafe {
let class_name = CLASS_NAME.to_wide();
let icon = LoadIconW(0 as HINSTANCE, IDI_APPLICATION);
Expand Down Expand Up @@ -106,7 +114,7 @@ impl Application {
loop {
if let Ok(state) = self.state.try_borrow() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My experience is that putting any additional functionality in the runloop is problematic. The logic doesn't get invoked when the system is using its own runloop, which happens on live resize and also while a file dialog is open. Also, if and when we ever run as a guest (for example as a VST plugin), then we'll also be at the mercy of the host's runloop. But if we use the function sparingly, maybe we're ok.

Also, I think the name "DS_HANDLE_DROPPED" is confusing and should be changed to "BLOCKED" to be consistent with other functions.

See below where I suggest a comment to add.

for hwnd in &state.windows {
SendMessageW(*hwnd, DS_HANDLE_DROPPED, 0,0);
SendMessageW(*hwnd, DS_HANDLE_DEFERRED, 0,0);
}
}
let mut msg = mem::MaybeUninit::uninit();
Expand Down
14 changes: 6 additions & 8 deletions druid-shell/src/platform/windows/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ use std::mem::size_of;
use crate::screen::Monitor;
use crate::kurbo::Rect;

static mut MONITORS : Vec<Monitor> = Vec::new();

unsafe extern "system" fn monitorenumproc(hmonitor : HMONITOR, _hdc : HDC, _lprect : LPRECT, _lparam : LPARAM) -> BOOL {
let rect = RECT { left: 0, top: 0, right: 0, bottom: 0};
let mut info = MONITORINFO { cbSize : size_of::<MONITORINFO>() as u32, rcMonitor : rect, rcWork : rect, dwFlags : 0};
Expand All @@ -41,21 +39,21 @@ unsafe extern "system" fn monitorenumproc(hmonitor : HMONITOR, _hdc : HDC, _lpre
let primary = info.dwFlags == MONITORINFOF_PRIMARY;
let rect = Rect::new(info.rcMonitor.left as f64, info.rcMonitor.top as f64, info.rcMonitor.right as f64, info.rcMonitor.bottom as f64);
let work_rect = Rect::new(info.rcWork.left as f64, info.rcWork.top as f64, info.rcWork.right as f64, info.rcWork.bottom as f64);
let m = Monitor::new(primary, rect, work_rect);
MONITORS.push(m);
let monitors = _lparam as *mut Vec::<Monitor>;
(*monitors).push(Monitor::new(primary, rect, work_rect));
TRUE
}


pub(crate) fn get_monitors() -> Vec<Monitor> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there is a safety issue here due to the use of static mutable global state, and the fact that this function is exposed up to the user through the Screen interface, which is Send and Sync. One solution is to be more threadsafe by using the dwData mechanism to pass in a pointer to the Vec being mutated, another thing to consider is ensuring that Screen can't be used across thread boundaries.

Another possibility for the safety concern is to use a Mutex. But that in turn requires once_cell (or lazy_static, we have deps on both), so it's a little long-winded.

unsafe {
MONITORS = Vec::new();
if EnumDisplayMonitors(null_mut(), null_mut(), Some(monitorenumproc), 0) == 0{
let monitors = Vec::<Monitor>::new();
let ptr = &monitors as *const Vec::<Monitor>;
if EnumDisplayMonitors(null_mut(), null_mut(), Some(monitorenumproc), ptr as isize) == 0{
warn!(
"Failed to Enumerate Display Monitors: {}",
Error::Hr(HRESULT_FROM_WIN32(GetLastError()))
);
};
MONITORS.clone()
monitors
}
}
5 changes: 5 additions & 0 deletions druid-shell/src/platform/windows/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ pub(crate) fn recti_to_rect(rect: RECT) -> Rect {
type GetDpiForSystem = unsafe extern "system" fn() -> UINT;
type GetDpiForWindow = unsafe extern "system" fn(HWND) -> UINT;
type SetProcessDpiAwarenessContext = unsafe extern "system" fn(winapi::shared::windef::DPI_AWARENESS_CONTEXT) -> BOOL;
type GetSystemMetricsForDpi = unsafe extern "system" fn(winapi::ctypes::c_int, UINT) -> winapi::ctypes::c_int;
// from shcore.dll
type GetDpiForMonitor = unsafe extern "system" fn(HMONITOR, MONITOR_DPI_TYPE, *mut UINT, *mut UINT);
type SetProcessDpiAwareness = unsafe extern "system" fn(PROCESS_DPI_AWARENESS) -> HRESULT;
Expand All @@ -150,6 +151,7 @@ pub struct OptionalFunctions {
pub SetProcessDpiAwarenessContext: Option<SetProcessDpiAwarenessContext>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest cleaning this up a bit, but now I'm not sure. We don't currently use GetDpiForMonitor, but I think it is the function we want to use for compatibility on Windows 8.1. Similar for SetProcessDpiAwareness.

pub GetDpiForMonitor: Option<GetDpiForMonitor>,
pub SetProcessDpiAwareness: Option<SetProcessDpiAwareness>,
pub GetSystemMetricsForDpi: Option<GetSystemMetricsForDpi>,
pub DCompositionCreateDevice2: Option<DCompositionCreateDevice2>,
pub CreateDXGIFactory2: Option<CreateDXGIFactory2>,
}
Expand Down Expand Up @@ -205,6 +207,7 @@ fn load_optional_functions() -> OptionalFunctions {
let mut GetDpiForWindow = None;
let mut SetProcessDpiAwarenessContext = None;
let mut SetProcessDpiAwareness = None;
let mut GetSystemMetricsForDpi = None;
let mut DCompositionCreateDevice2 = None;
let mut CreateDXGIFactory2 = None;

Expand All @@ -221,6 +224,7 @@ fn load_optional_functions() -> OptionalFunctions {
load_function!(user32, GetDpiForSystem, "10");
load_function!(user32, GetDpiForWindow, "10");
load_function!(user32, SetProcessDpiAwarenessContext, "10");
load_function!(user32, GetSystemMetricsForDpi, "10");
}

if !dcomp.is_null() {
Expand All @@ -237,6 +241,7 @@ fn load_optional_functions() -> OptionalFunctions {
SetProcessDpiAwarenessContext,
GetDpiForMonitor,
SetProcessDpiAwareness,
GetSystemMetricsForDpi,
DCompositionCreateDevice2,
CreateDXGIFactory2,
}
Expand Down
Loading