Skip to content

Commit

Permalink
Address review
Browse files Browse the repository at this point in the history
  • Loading branch information
daxpedda committed Jul 23, 2024
1 parent a3f956d commit 5c39ab7
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 101 deletions.
20 changes: 8 additions & 12 deletions src/platform_impl/web/event_loop/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,19 +220,15 @@ impl Shared {
}

pub(crate) fn start(&self, event_handler: Box<EventHandler>) {
let start = {
let mut runner = self.0.runner.borrow_mut();
assert!(matches!(*runner, RunnerEnum::Pending));
if self.0.monitor.is_initializing() {
*runner = RunnerEnum::Initializing(Runner::new(event_handler));
false
} else {
*runner = RunnerEnum::Running(Runner::new(event_handler));
true
}
};
let mut runner = self.0.runner.borrow_mut();
assert!(matches!(*runner, RunnerEnum::Pending));
if self.0.monitor.is_initializing() {
*runner = RunnerEnum::Initializing(Runner::new(event_handler));
} else {
*runner = RunnerEnum::Running(Runner::new(event_handler));

drop(runner);

if start {
self.init();
self.set_listener();
}
Expand Down
154 changes: 79 additions & 75 deletions src/platform_impl/web/monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,41 +397,8 @@ impl MonitorHandler {
} else {
// If permission is denied we listen for changes so we can catch external
// permission granting.
let handle = EventListenerHandle::new(
permission.clone(),
"change",
Closure::new({
let runner = runner.weak();
let permission = permission.clone();
move || {
if let PermissionState::Granted = permission.state() {
let future = JsFuture::from(window.screen_details());

let runner = runner.clone();
wasm_bindgen_futures::spawn_local(async move {
let screen_details = match future.await {
Ok(screen_details) => {
screen_details.unchecked_into()
},
Err(error) => unreachable_error(
&error,
"getting screen details failed even though \
permission was granted",
),
};

if let Some(runner) = runner.upgrade() {
// We drop the event listener handle here, which
// doesn't drop it while we are running it, because
// we are in a `spawn_local()` context.
*runner.monitor().state.borrow_mut() =
State::Detailed(screen_details);
}
});
}
}
}),
);
let handle =
Self::setup_listener(runner.weak(), window, permission.clone());
State::Permission { permission, _handle: handle }
};

Expand All @@ -448,6 +415,40 @@ impl MonitorHandler {
Self { state: RefCell::new(state), main_thread, window, engine, screen }
}

fn setup_listener(
runner: WeakShared,
window: WindowExt,
permission: PermissionStatus,
) -> EventListenerHandle<dyn Fn()> {
EventListenerHandle::new(
permission.clone(),
"change",
Closure::new(move || {
if let PermissionState::Granted = permission.state() {
let future = JsFuture::from(window.screen_details());

let runner = runner.clone();
wasm_bindgen_futures::spawn_local(async move {
let screen_details = match future.await {
Ok(screen_details) => screen_details.unchecked_into(),
Err(error) => unreachable_error(
&error,
"getting screen details failed even though permission was granted",
),
};

if let Some(runner) = runner.upgrade() {
// We drop the event listener handle here, which
// doesn't drop it while we are running it, because
// we are in a `spawn_local()` context.
*runner.monitor().state.borrow_mut() = State::Detailed(screen_details);
}
});
}
}),
)
}

pub fn is_extended(&self) -> Option<bool> {
self.screen.is_extended()
}
Expand Down Expand Up @@ -611,6 +612,47 @@ pub(crate) enum MonitorPermissionFuture {
Ready(Option<Result<(), MonitorPermissionError>>),
}

impl MonitorPermissionFuture {
fn upgrade(&mut self) {
let notifier = Notifier::new();
let notified = notifier.notified();
let Self::Initialize { runner, .. } = mem::replace(self, Self::Upgrade(notified.clone()))
else {
unreachable!()
};

runner.dispatch(|(shared, window)| {
let future = JsFuture::from(window.screen_details());

if let Some(shared) = shared.upgrade() {
*shared.monitor().state.borrow_mut() = State::Upgrade(notified);
}

let shared = shared.clone();
wasm_bindgen_futures::spawn_local(async move {
match future.await {
Ok(details) => {
// Notifying `Future`s is not dependant on the lifetime
// of
// the runner, because
// they can outlive it.
notifier.notify(Ok(()));

if let Some(shared) = shared.upgrade() {
*shared.monitor().state.borrow_mut() =
State::Detailed(details.unchecked_into())
}
},
Err(error) => unreachable_error(
&error,
"getting screen details failed even though permission was granted",
),
}
});
});
}
}

impl Future for MonitorPermissionFuture {
type Output = Result<(), MonitorPermissionError>;

Expand All @@ -625,46 +667,8 @@ impl Future for MonitorPermissionFuture {
Poll::Ready(Err(error))
},
MonitorPermissionError::Prompt => {
let notifier = Notifier::new();
let notified = notifier.notified();
let Self::Initialize { runner, .. } =
mem::replace(this, Self::Upgrade(notified.clone()))
else {
unreachable!()
};

runner.queue(|(shared, window)| {
let future = JsFuture::from(window.screen_details());

if let Some(shared) = shared.upgrade() {
*shared.monitor().state.borrow_mut() = State::Upgrade(notified);
}

let shared = shared.clone();
wasm_bindgen_futures::spawn_local(async move {
match future.await {
Ok(details) => {
// Notifying `Future`s is not dependant on the lifetime
// of
// the runner, because
// they can outlive it.
notifier.notify(Ok(()));

if let Some(shared) = shared.upgrade() {
*shared.monitor().state.borrow_mut() =
State::Detailed(details.unchecked_into())
}
},
Err(error) => unreachable_error(
&error,
"getting screen details failed even though permission \
was granted",
),
}
});

Poll::Pending
})
this.upgrade();
Poll::Pending
},
}
} else {
Expand Down
29 changes: 15 additions & 14 deletions src/platform_impl/web/web_sys/fullscreen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,23 @@ pub(crate) fn request_fullscreen(
match fullscreen {
Fullscreen::Exclusive(_) => error!("Exclusive full screen mode is not supported"),
Fullscreen::Borderless(Some(monitor)) => {
if monitor::has_screen_details_support(window) {
if let Some(monitor) = monitor.detailed(main_thread) {
let options: FullscreenOptions = Object::new().unchecked_into();
options.set_screen(&monitor);
REJECT_HANDLER.with(|handler| {
let _ = canvas.request_fullscreen_with_options(&options).catch(handler);
});
} else {
error!(
"Selecting a specific screen for fullscreen mode requires a detailed \
screen. See `MonitorHandleExtWeb::is_detailed()`."
)
}
} else {
if !monitor::has_screen_details_support(window) {
error!(
"Fullscreen mode selecting a specific screen is not supported by this browser"
);
return;
}

if let Some(monitor) = monitor.detailed(main_thread) {
let options: FullscreenOptions = Object::new().unchecked_into();
options.set_screen(&monitor);
REJECT_HANDLER.with(|handler| {
let _ = canvas.request_fullscreen_with_options(&options).catch(handler);
});
} else {
error!(
"Selecting a specific screen for fullscreen mode requires a detailed screen. \
See `MonitorHandleExtWeb::is_detailed()`."
)
}
},
Expand Down

0 comments on commit 5c39ab7

Please sign in to comment.