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.

Sign up for GitHub

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

Add ViewportCommand::RequestCut, RequestCopy and RequestPaste to trigger clipboard actions #4035

Merged
merged 6 commits into from
Apr 22, 2024
Merged

Add ViewportCommand::RequestCut, RequestCopy and RequestPaste to trigger clipboard actions #4035

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a743f6c
Add `ViewportCommand::RequestCut`, `ViewportCommand::RequestCopy` and…
bu5hm4nn Feb 12, 2024
5a20351
Merge remote-tracking branch 'upstream/master' into feature/let-app-t…
bu5hm4nn Mar 9, 2024
92a21f8
Elaborate docstring for new Clipboard Viewport commands
bu5hm4nn Mar 26, 2024
8663178
improve docstring
emilk Apr 21, 2024
24a0aac
Apply suggestions from code review
bu5hm4nn Apr 21, 2024
52fc424
Merge remote-tracking branch 'upstream/master' into feature/let-app-t…
bu5hm4nn Apr 21, 2024
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
58 changes: 40 additions & 18 deletions crates/eframe/src/native/glow_integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use std::{cell::RefCell, rc::Rc, sync::Arc, time::Instant};

use egui_winit::ActionRequested;
use glutin::{
config::GlConfig,
context::NotCurrentGlContext,
Expand All @@ -22,9 +23,9 @@ use winit::{
};

use egui::{
epaint::ahash::HashMap, DeferredViewportUiCallback, ImmediateViewport, NumExt as _,
ViewportBuilder, ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet,
ViewportInfo, ViewportOutput,
ahash::{HashMap, HashSet, HashSetExt},
DeferredViewportUiCallback, ImmediateViewport, NumExt as _, ViewportBuilder, ViewportClass,
ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo, ViewportOutput,
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
Expand Down Expand Up @@ -104,7 +105,7 @@ struct Viewport {
class: ViewportClass,
builder: ViewportBuilder,
info: ViewportInfo,
screenshot_requested: bool,
actions_requested: HashSet<egui_winit::ActionRequested>,

/// The user-callback that shows the ui.
/// None for immediate viewports.
Expand Down Expand Up @@ -663,17 +664,38 @@ impl GlowWinitRunning {
);

{
let screenshot_requested = std::mem::take(&mut viewport.screenshot_requested);
if screenshot_requested {
let screenshot = painter.read_screen_rgba(screen_size_in_pixels);
egui_winit
.egui_input_mut()
.events
.push(egui::Event::Screenshot {
viewport_id,
image: screenshot.into(),
});
for action in viewport.actions_requested.drain() {
match action {
ActionRequested::Screenshot => {
let screenshot = painter.read_screen_rgba(screen_size_in_pixels);
egui_winit
.egui_input_mut()
.events
.push(egui::Event::Screenshot {
viewport_id,
image: screenshot.into(),
});
}
ActionRequested::Cut => {
egui_winit.egui_input_mut().events.push(egui::Event::Cut);
}
ActionRequested::Copy => {
egui_winit.egui_input_mut().events.push(egui::Event::Copy);
}
ActionRequested::Paste => {
if let Some(contents) = egui_winit.clipboard_text() {
let contents = contents.replace("\r\n", "\n");
if !contents.is_empty() {
egui_winit
.egui_input_mut()
.events
.push(egui::Event::Paste(contents));
}
}
}
}
}

integration.post_rendering(&window);
}

Expand Down Expand Up @@ -986,7 +1008,7 @@ impl GlutinWindowContext {
class: ViewportClass::Root,
builder: viewport_builder,
info,
screenshot_requested: false,
actions_requested: HashSetExt::new(),
bu5hm4nn marked this conversation as resolved.
Show resolved Hide resolved
viewport_ui_cb: None,
gl_surface: None,
window: window.map(Arc::new),
Expand Down Expand Up @@ -1238,7 +1260,7 @@ impl GlutinWindowContext {
commands,
window,
is_viewport_focused,
&mut viewport.screenshot_requested,
&mut viewport.actions_requested,
);
}
}
Expand Down Expand Up @@ -1283,7 +1305,7 @@ fn initialize_or_update_viewport<'vp>(
class,
builder,
info: Default::default(),
screenshot_requested: false,
actions_requested: HashSet::new(),
viewport_ui_cb,
window: None,
egui_winit: None,
Expand Down Expand Up @@ -1317,7 +1339,7 @@ fn initialize_or_update_viewport<'vp>(
delta_commands,
window,
is_viewport_focused,
&mut viewport.screenshot_requested,
&mut viewport.actions_requested,
);
}

Expand Down
47 changes: 38 additions & 9 deletions crates/eframe/src/native/wgpu_integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use std::{cell::RefCell, rc::Rc, sync::Arc, time::Instant};

use egui_winit::ActionRequested;
use parking_lot::Mutex;
use raw_window_handle::{HasDisplayHandle as _, HasWindowHandle as _};
use winit::{
Expand All @@ -15,9 +16,9 @@ use winit::{
};

use egui::{
ahash::HashMap, DeferredViewportUiCallback, FullOutput, ImmediateViewport, ViewportBuilder,
ViewportClass, ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo,
ViewportOutput,
ahash::{HashMap, HashSet, HashSetExt},
DeferredViewportUiCallback, FullOutput, ImmediateViewport, ViewportBuilder, ViewportClass,
ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo, ViewportOutput,
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
Expand Down Expand Up @@ -77,7 +78,7 @@ pub struct Viewport {
class: ViewportClass,
builder: ViewportBuilder,
info: ViewportInfo,
screenshot_requested: bool,
actions_requested: HashSet<ActionRequested>,

/// `None` for sync viewports.
viewport_ui_cb: Option<Arc<DeferredViewportUiCallback>>,
Expand Down Expand Up @@ -286,7 +287,7 @@ impl WgpuWinitApp {
maximized: Some(window.is_maximized()),
..Default::default()
},
screenshot_requested: false,
actions_requested: HashSetExt::new(),
bu5hm4nn marked this conversation as resolved.
Show resolved Hide resolved
viewport_ui_cb: None,
window: Some(window),
egui_winit: Some(egui_winit),
Expand Down Expand Up @@ -670,7 +671,10 @@ impl WgpuWinitRunning {

let clipped_primitives = egui_ctx.tessellate(shapes, pixels_per_point);

let screenshot_requested = std::mem::take(&mut viewport.screenshot_requested);
let screenshot_requested = viewport
.actions_requested
.take(&ActionRequested::Screenshot)
.is_some();
let (vsync_secs, screenshot) = painter.paint_and_update_textures(
viewport_id,
pixels_per_point,
Expand All @@ -689,6 +693,31 @@ impl WgpuWinitRunning {
});
}

for action in viewport.actions_requested.drain() {
match action {
ActionRequested::Screenshot => {
// already handled above
}
ActionRequested::Cut => {
egui_winit.egui_input_mut().events.push(egui::Event::Cut);
}
ActionRequested::Copy => {
egui_winit.egui_input_mut().events.push(egui::Event::Copy);
}
ActionRequested::Paste => {
if let Some(contents) = egui_winit.clipboard_text() {
let contents = contents.replace("\r\n", "\n");
if !contents.is_empty() {
egui_winit
.egui_input_mut()
.events
.push(egui::Event::Paste(contents));
}
}
}
}
}

integration.post_rendering(window);

let active_viewports_ids: ViewportIdSet = viewport_output.keys().copied().collect();
Expand Down Expand Up @@ -1054,7 +1083,7 @@ fn handle_viewport_output(
commands,
window,
is_viewport_focused,
&mut viewport.screenshot_requested,
&mut viewport.actions_requested,
);
}
}
Expand Down Expand Up @@ -1087,7 +1116,7 @@ fn initialize_or_update_viewport<'vp>(
class,
builder,
info: Default::default(),
screenshot_requested: false,
actions_requested: HashSet::new(),
viewport_ui_cb,
window: None,
egui_winit: None,
Expand Down Expand Up @@ -1120,7 +1149,7 @@ fn initialize_or_update_viewport<'vp>(
delta_commands,
window,
is_viewport_focused,
&mut viewport.screenshot_requested,
&mut viewport.actions_requested,
);
}

Expand Down
28 changes: 23 additions & 5 deletions crates/egui-winit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ pub use accesskit_winit;
pub use egui;
#[cfg(feature = "accesskit")]
use egui::accesskit;
use egui::{Pos2, Rect, Vec2, ViewportBuilder, ViewportCommand, ViewportId, ViewportInfo};
use egui::{
ahash::HashSet, Pos2, Rect, Vec2, ViewportBuilder, ViewportCommand, ViewportId, ViewportInfo,
};
pub use winit;

pub mod clipboard;
Expand Down Expand Up @@ -1254,14 +1256,21 @@ fn translate_cursor(cursor_icon: egui::CursorIcon) -> Option<winit::window::Curs

// Helpers for egui Viewports
// ---------------------------------------------------------------------------
#[derive(PartialEq, Eq, Hash, Debug)]
pub enum ActionRequested {
Screenshot,
Cut,
Copy,
Paste,
}

pub fn process_viewport_commands(
egui_ctx: &egui::Context,
info: &mut ViewportInfo,
commands: impl IntoIterator<Item = ViewportCommand>,
window: &Window,
is_viewport_focused: bool,
screenshot_requested: &mut bool,
actions_requested: &mut HashSet<ActionRequested>,
) {
for command in commands {
process_viewport_command(
Expand All @@ -1270,7 +1279,7 @@ pub fn process_viewport_commands(
command,
info,
is_viewport_focused,
screenshot_requested,
actions_requested,
);
}
}
Expand All @@ -1281,7 +1290,7 @@ fn process_viewport_command(
command: ViewportCommand,
info: &mut ViewportInfo,
is_viewport_focused: bool,
screenshot_requested: &mut bool,
actions_requested: &mut HashSet<ActionRequested>,
) {
crate::profile_function!();

Expand Down Expand Up @@ -1462,7 +1471,16 @@ fn process_viewport_command(
}
}
ViewportCommand::Screenshot => {
*screenshot_requested = true;
actions_requested.insert(ActionRequested::Screenshot);
}
ViewportCommand::RequestCut => {
actions_requested.insert(ActionRequested::Cut);
}
ViewportCommand::RequestCopy => {
actions_requested.insert(ActionRequested::Copy);
}
ViewportCommand::RequestPaste => {
actions_requested.insert(ActionRequested::Paste);
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions crates/egui/src/viewport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,15 @@ pub enum ViewportCommand {
///
/// The results are returned in `crate::Event::Screenshot`.
Screenshot,

/// Request a cut from the clipboard
RequestCut,

/// Request a copy from the clipboard
RequestCopy,

/// Request a paste from the clipboard
RequestPaste,
Copy link
Owner

Choose a reason for hiding this comment

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

These needs better docstrings, because I don't get it. Are these effectively injecting events in the next frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are doing the equivalent of pressing CTRL + C, CTRL + X and CTRL + V (or the CMD equivalent on macOS). The reason we had to introduce this is because adding a second clipboard instance broke clipboard interaction for Wayland users. So in order to offer users a context menu with these actions, we needed a way to trigger them. Since we cannot programmatically emit keypresses currently, this seemed like a viable solution.

So maybe the docstring could say Trigger a CMD + X cut or something like that

Copy link
Owner

Choose a reason for hiding this comment

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

I see.

Perhaps "Reads text from the system clipboard and pastes it in the next frame"

}

impl ViewportCommand {
Expand Down
13 changes: 8 additions & 5 deletions crates/egui_glow/src/winit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
pub use egui_winit;
pub use egui_winit::EventResponse;

use egui::{ViewportId, ViewportOutput};
use egui::{
ahash::{HashSet, HashSetExt},
ViewportId, ViewportOutput,
};
use egui_winit::winit;

use crate::shader_version::ShaderVersion;
Expand Down Expand Up @@ -79,17 +82,17 @@ impl EguiGlow {
log::warn!("Multiple viewports not yet supported by EguiGlow");
}
for (_, ViewportOutput { commands, .. }) in viewport_output {
let mut screenshot_requested = false;
let mut actions_requested: HashSet<egui_winit::ActionRequested> = HashSetExt::new();
bu5hm4nn marked this conversation as resolved.
Show resolved Hide resolved
egui_winit::process_viewport_commands(
&self.egui_ctx,
&mut self.viewport_info,
commands,
window,
true,
&mut screenshot_requested,
&mut actions_requested,
);
if screenshot_requested {
log::warn!("Screenshot not yet supported by EguiGlow");
for action in actions_requested {
log::warn!("{:?} not yet supported by EguiGlow", action);
}
}

Expand Down
Loading