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

web clipboard handling #178

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
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
6 changes: 6 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[alias]
run-wasm = ["run", "--release", "--package", "run-wasm", "--"]
Copy link
Contributor Author

@Vrixyz Vrixyz May 22, 2023

Choose a reason for hiding this comment

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

I think cargo run-wasm is quite handy to easily test a wasm version ; I can make another PR if interested


# Using unstable APIs is required for writing to clipboard
[target.wasm32-unknown-unknown]
rustflags = ["--cfg=web_sys_unstable_apis"]
26 changes: 25 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ default_fonts = ["egui/default_fonts"]
serde = ["egui/serde"]

[dependencies]
bevy = { version = "0.10", default-features = false, features = ["bevy_render", "bevy_asset"] }
bevy = { version = "0.10", default-features = false, features = [
"bevy_render",
"bevy_asset",
] }
egui = { version = "0.21.0", default-features = false, features = ["bytemuck"] }
webbrowser = { version = "0.8.2", optional = true }

Expand All @@ -39,3 +42,24 @@ bevy = { version = "0.10", default-features = false, features = [
"bevy_pbr",
"bevy_core_pipeline",
] }

[target.'cfg(target_arch = "wasm32")'.dependencies]
winit = "0.28"
web-sys = { version = "*", features = [
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved
"Clipboard",
"ClipboardEvent",
"DataTransfer",
'Document',
'EventTarget',
"Window",
"Navigator",
] }
js-sys = "0.3.63"
wasm-bindgen = "0.2.84"
wasm-bindgen-futures = "0.4.36"
console_log = "1.0.0"
log = "0.4"
crossbeam-channel = "0.5.8"
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved

[workspace]
members = ["run-wasm"]
8 changes: 7 additions & 1 deletion examples/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ fn main() {
.insert_resource(ClearColor(Color::rgb(0.0, 0.0, 0.0)))
.insert_resource(Msaa::Sample4)
.init_resource::<UiState>()
.add_plugins(DefaultPlugins)
.add_plugins(DefaultPlugins.set(WindowPlugin {
primary_window: Some(Window {
prevent_default_event_handling: false,
..default()
}),
..default()
}))
.add_plugin(EguiPlugin)
.add_startup_system(configure_visuals_system)
.add_startup_system(configure_ui_state_system)
Expand Down
9 changes: 9 additions & 0 deletions run-wasm/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "run-wasm"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
cargo-run-wasm = "0.2.0"
3 changes: 3 additions & 0 deletions run-wasm/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
cargo_run_wasm::run_wasm_with_css("body { margin: 0px; }");
}
56 changes: 49 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ pub mod systems;
/// Egui render node.
pub mod egui_node;

/// Clipboard management for web
#[cfg(all(feature = "manage_clipboard", target_arch = "wasm32"))]
pub mod web_clipboard;

pub use egui;
#[cfg(all(feature = "manage_clipboard", target_arch = "wasm32"))]
use web_clipboard::{WebEventCopy, WebEventCut, WebEventPaste};

use crate::{
egui_node::{EguiPipeline, EGUI_SHADER_HANDLE},
Expand Down Expand Up @@ -138,6 +144,10 @@ impl Default for EguiSettings {
#[derive(Component, Clone, Debug, Default, Deref, DerefMut)]
pub struct EguiInput(pub egui::RawInput);

/// A resource to check if we're on a mac, correctly detects on web too.
#[derive(Resource)]
pub struct IsMac(pub bool);
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved

/// A resource for accessing clipboard.
///
/// The resource is available only if `manage_clipboard` feature is enabled.
Expand All @@ -146,8 +156,15 @@ pub struct EguiInput(pub egui::RawInput);
pub struct EguiClipboard {
#[cfg(not(target_arch = "wasm32"))]
clipboard: ThreadLocal<Option<RefCell<Clipboard>>>,
/// for copy events.
#[cfg(target_arch = "wasm32")]
pub web_copy: web_clipboard::WebChannel<WebEventCopy>,
/// for copy events.
#[cfg(target_arch = "wasm32")]
pub web_cut: web_clipboard::WebChannel<WebEventCut>,
/// for paste events, only supporting strings.
#[cfg(target_arch = "wasm32")]
clipboard: String,
pub web_paste: web_clipboard::WebChannel<WebEventPaste>,
}

#[cfg(feature = "manage_clipboard")]
Expand All @@ -159,12 +176,20 @@ impl EguiClipboard {

/// Gets clipboard contents. Returns [`None`] if clipboard provider is unavailable or returns an error.
#[must_use]
pub fn get_contents(&self) -> Option<String> {
#[cfg(not(target_arch = "wasm32"))]
pub fn get_contents(&mut self) -> Option<String> {
self.get_contents_impl()
}

/// Gets clipboard contents. Returns [`None`] if clipboard provider is unavailable or returns an error.
#[must_use]
#[cfg(target_arch = "wasm32")]
pub fn get_contents(&mut self) -> Option<String> {
self.get_contents_impl()
}

#[cfg(not(target_arch = "wasm32"))]
fn set_contents_impl(&self, contents: &str) {
fn set_contents_impl(&mut self, contents: &str) {
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved
if let Some(mut clipboard) = self.get() {
if let Err(err) = clipboard.set_text(contents.to_owned()) {
log::error!("Failed to set clipboard contents: {:?}", err);
Expand All @@ -174,11 +199,11 @@ impl EguiClipboard {

#[cfg(target_arch = "wasm32")]
fn set_contents_impl(&mut self, contents: &str) {
self.clipboard = contents.to_owned();
web_clipboard::clipboard_copy(contents.to_owned());
}

#[cfg(not(target_arch = "wasm32"))]
fn get_contents_impl(&self) -> Option<String> {
fn get_contents_impl(&mut self) -> Option<String> {
if let Some(mut clipboard) = self.get() {
match clipboard.get_text() {
Ok(contents) => return Some(contents),
Expand All @@ -190,8 +215,8 @@ impl EguiClipboard {

#[cfg(target_arch = "wasm32")]
#[allow(clippy::unnecessary_wraps)]
fn get_contents_impl(&self) -> Option<String> {
Some(self.clipboard.clone())
fn get_contents_impl(&mut self) -> Option<String> {
self.web_paste.try_read_clipboard_event().map(|e| e.0)
}

#[cfg(not(target_arch = "wasm32"))]
Expand Down Expand Up @@ -532,7 +557,24 @@ impl Plugin for EguiPlugin {
world.init_resource::<EguiClipboard>();
world.init_resource::<EguiUserTextures>();
world.init_resource::<EguiMousePosition>();
if !world.contains_resource::<IsMac>() {
let mut is_mac = cfg!(target_os = "macos");
#[cfg(target_arch = "wasm32")]
{
let window = web_sys::window().expect("window");

let nav = window.navigator();
let platform = nav.platform();
Copy link
Owner

Choose a reason for hiding this comment

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

Hm.. It looks like this property is deprecated: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/platform

Copy link
Owner

Choose a reason for hiding this comment

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

Let's just read from the user agent string for now (by testing whether it contains "Mac"), and I'll keep my eye on https://developer.mozilla.org/en-US/docs/Web/API/NavigatorUAData (which is experimental for now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with user_agent.contains("Macintosh;");, I don't know if any user agent might include "Machine", I wanted to err on the side of precaution.

if let Ok(platform) = platform {
is_mac = platform.starts_with("Mac");
}
}
log::info!(is_mac);
world.insert_resource(IsMac(is_mac));
}

#[cfg(all(feature = "manage_clipboard", target_arch = "wasm32"))]
app.add_startup_system(web_clipboard::startup_setup_web_events);
app.add_startup_systems(
(
setup_new_windows_system,
Expand Down
60 changes: 42 additions & 18 deletions src/systems.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
EguiContext, EguiContextQuery, EguiInput, EguiMousePosition, EguiSettings, WindowSize,
EguiContext, EguiContextQuery, EguiInput, EguiMousePosition, EguiSettings, IsMac, WindowSize,
};
#[cfg(feature = "open_url")]
use bevy::log;
Expand Down Expand Up @@ -54,7 +54,7 @@ impl<'w, 's> InputEvents<'w, 's> {
#[derive(SystemParam)]
pub struct InputResources<'w, 's> {
#[cfg(feature = "manage_clipboard")]
pub egui_clipboard: Res<'w, crate::EguiClipboard>,
pub egui_clipboard: ResMut<'w, crate::EguiClipboard>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

see #178 (comment) ; to be noted we can have mut only for wasm if we want.

pub keyboard_input: Res<'w, Input<KeyCode>>,
#[system_param(ignore)]
_marker: PhantomData<&'s ()>,
Expand All @@ -71,8 +71,9 @@ pub struct ContextSystemParams<'w, 's> {

/// Processes Bevy input and feeds it to Egui.
pub fn process_input_system(
mut is_mac: Res<IsMac>,
mut input_events: InputEvents,
input_resources: InputResources,
mut input_resources: InputResources,
mut context_params: ContextSystemParams,
egui_settings: Res<EguiSettings>,
mut egui_mouse_position: ResMut<EguiMousePosition>,
Expand Down Expand Up @@ -101,12 +102,8 @@ pub fn process_input_system(
let win = input_resources.keyboard_input.pressed(KeyCode::LWin)
|| input_resources.keyboard_input.pressed(KeyCode::RWin);

let mac_cmd = if cfg!(target_os = "macos") {
win
} else {
false
};
let command = if cfg!(target_os = "macos") { win } else { ctrl };
let mac_cmd = if is_mac.0 { win } else { false };
let command = if is_mac.0 { win } else { ctrl };

let modifiers = egui::Modifiers {
alt,
Expand Down Expand Up @@ -250,20 +247,47 @@ pub fn process_input_system(
// We also check that it's an `ButtonState::Pressed` event, as we don't want to
// copy, cut or paste on the key release.
#[cfg(feature = "manage_clipboard")]
if command && pressed {
match key {
egui::Key::C => {
{
#[cfg(not(target_arch = "wasm32"))]
if command && pressed {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

⚠️ on wasm mac, this is wrong, because it maps to ctrl key, but it should map to command key...

(to be clear, this is not a but introduced in this PR)

Copy link
Owner

Choose a reason for hiding this comment

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

Oh, good catch. This makes me wonder how we can detect running on MacOS in wasm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eframe uses events for all those, deferring the responsibility to the browser: https://github.com/emilk/egui/blob/307565efa55158cfa6b82d2e8fdc4c4914b954ed/crates/eframe/src/web/events.rs#L184

Copy link
Owner

@vladbat00 vladbat00 May 28, 2023

Choose a reason for hiding this comment

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

It feels like we should adopt the same approach. Let's keep the whole block enabled only for non-wasm targets, whereas for wasm we'll just read from channels (which in turn will get messages on web_sys events).

Copy link
Owner

Choose a reason for hiding this comment

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

I think this is important to fix, as atm on MacOS, users need to do Ctrl+C to copy and Cmd+V to paste. I'd also be happy if we added the Select All (Ctrl/Cmd+A) support for WASM, to avoid the same inconsistency issue. That's extending the scope of this PR a bit, but hotkey consistency feels important here.

Copy link
Contributor Author

@Vrixyz Vrixyz May 31, 2023

Choose a reason for hiding this comment

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

I feel like it means we would bypass winit/bevy key inputs in wasm :/ could this possibly fixed upstream 🤔 ?

for the record, ctrl-A works (kinda) on wasm; on azerty it's mapped to ctrl-Q though, and on mac to ctrl where we'd want it to be cmd.

In all hindsight, I'd suggest a simpler approach where we detect if we run on mac or not through https://docs.rs/web-sys/latest/web_sys/struct.Navigator.html#method.platform, and adapt our meta key from there in case of cfg!(target_os = "wasm32") to consolidate following code: https://github.com/mvlabat/bevy_egui/blob/c51fc5640e2e431fa2169c98e6d252914c614904/src/systems.rs#L104-L110

Is it ok for you?

match key {
egui::Key::C => {
focused_input.events.push(egui::Event::Copy);
}
egui::Key::X => {
focused_input.events.push(egui::Event::Cut);
}
egui::Key::V => {
if let Some(contents) =
input_resources.egui_clipboard.get_contents()
{
focused_input.events.push(egui::Event::Text(contents))
}
}
_ => {}
}
}
#[cfg(target_arch = "wasm32")]
{
if input_resources
.egui_clipboard
.web_copy
.try_read_clipboard_event()
.is_some()
{
focused_input.events.push(egui::Event::Copy);
}
egui::Key::X => {
if input_resources
.egui_clipboard
.web_cut
.try_read_clipboard_event()
.is_some()
{
focused_input.events.push(egui::Event::Cut);
}
egui::Key::V => {
if let Some(contents) = input_resources.egui_clipboard.get_contents() {
focused_input.events.push(egui::Event::Text(contents))
}
if let Some(contents) = input_resources.egui_clipboard.get_contents() {
focused_input.events.push(egui::Event::Text(contents));
}
_ => {}
}
}
}
Expand Down
Loading