diff --git a/platform/src/devtools.rs b/platform/src/devtools.rs new file mode 100644 index 000000000..7bc62a137 --- /dev/null +++ b/platform/src/devtools.rs @@ -0,0 +1,92 @@ +//! The opt-in switch for makepad's in-app developer overlays. +//! +//! Three of them exist, and each one binds a bare function key and then claims +//! input the app never sees: +//! +//! * **F10** — the exploded draw-list view ([`crate::sploded`]). Intercepted in +//! `Cx::call_event_handler` *before* the app's handler, and once it is up it +//! also claims Escape, the arrow keys, `+`/`-`/`0`, `I` and `H` — no modifier +//! required — plus every pointer drag outside the declared flat band. +//! * **F12** — the design tweaker (`makepad_widgets::tweaker`), a child of every +//! `Window`. Once it is up it swallows every pointer event over the body. +//! * **Shift+F12** — the screen recorder (`makepad_widgets::screen_cap`), which +//! writes mp4 files next to the running process. +//! +//! These are development tools, so they stay off unless a developer asks for +//! them. A shipped app is not a place to discover that a stray F10 tilts the +//! whole UI into 3D and stops Escape from closing anything. +//! +//! Turn them on with `--devtools` on the command line, or `MAKEPAD_DEVTOOLS=1` +//! in the environment. `--remote` implies them: the remote control surface's +//! `/snap` + `/click` loop drives the tweaker, so a remote-driven app has +//! already opted in to being instrumented. An explicit `MAKEPAD_DEVTOOLS=0` +//! wins over all of it, which is also how the off path stays testable under +//! `--remote`. +//! +//! Only the *hotkeys* are gated, not the tools. An app that wants one of these +//! on its own terms still calls `Cx::sploded_toggle`, `tweaker::set_tweak_on` +//! or `ScreenCap::toggle` directly — that is the app deciding, rather than a +//! key nobody knew was bound. + +use std::sync::OnceLock; + +/// Whether this process opted into the developer overlays. +/// +/// Scans argv and the environment once and caches the answer, so the hot event +/// path pays an atomic load. See the module docs for what this gates. +pub fn enabled() -> bool { + static ENABLED: OnceLock = OnceLock::new(); + *ENABLED.get_or_init(|| { + decide( + std::env::args().any(|a| a == "--devtools"), + std::env::var("MAKEPAD_DEVTOOLS").ok().as_deref(), + crate::remote::requested(), + ) + }) +} + +/// The whole decision, with the process pulled out so it can be tested. +/// +/// `MAKEPAD_DEVTOOLS` takes the usual off-ish spellings, so it can sit in a +/// shell profile as `0` instead of having to be unset — and because it is the +/// one explicit signal, an off spelling also overrides `--devtools` and +/// `--remote`. +fn decide(flag: bool, env: Option<&str>, remote: bool) -> bool { + if let Some(env) = env { + let env = env.trim().to_ascii_lowercase(); + return !matches!(env.as_str(), "" | "0" | "off" | "false" | "no"); + } + flag || remote +} + +#[cfg(test)] +mod tests { + use super::decide; + + #[test] + fn off_by_default() { + assert!(!decide(false, None, false)); + } + + #[test] + fn the_flag_or_remote_turns_it_on() { + assert!(decide(true, None, false)); + assert!(decide(false, None, true)); + } + + #[test] + fn the_env_var_turns_it_on() { + for on in ["1", "yes", "true", "on", " 1 "] { + assert!(decide(false, Some(on), false), "{on:?} should enable"); + } + } + + #[test] + fn an_off_spelling_wins_over_the_flag_and_remote() { + // So `MAKEPAD_DEVTOOLS=0` can live in a profile, and so the gated-off + // path is still reachable under --remote. + for off in ["0", "off", "false", "no", "", " ", "OFF"] { + assert!(!decide(true, Some(off), true), "{off:?} should disable"); + } + } +} diff --git a/platform/src/lib.rs b/platform/src/lib.rs index 8c8f2753a..43762968d 100644 --- a/platform/src/lib.rs +++ b/platform/src/lib.rs @@ -78,6 +78,7 @@ pub mod display_context; #[macro_use] mod app_main; pub mod remote; +pub mod devtools; pub mod pixel_probe; pub mod screen_capture; pub mod audio_output_tap; diff --git a/platform/src/remote.rs b/platform/src/remote.rs index 08a2c2c26..ec4d744b5 100644 --- a/platform/src/remote.rs +++ b/platform/src/remote.rs @@ -46,11 +46,14 @@ mod imp { static ACTIVE: AtomicBool = AtomicBool::new(false); - /// True when this process was started with `--remote` (any form). Pure - /// argv scan, usable before the bridge itself is up — the platform's - /// focus policy reads it while the first window is being created. + /// True when this process asked for the remote bridge, in any of the forms + /// [`requested_bind`] accepts — including `MAKEPAD_REMOTE`, which a plain + /// argv scan used to miss, so `MAKEPAD_REMOTE=1` started the bridge while + /// everything keyed off this said no. Pure argv + env, usable before the + /// bridge itself is up: the platform's focus policy reads it while the + /// first window is being created. pub fn requested() -> bool { - std::env::args().any(|a| a == "--remote" || a.starts_with("--remote=")) + requested_bind().is_some() } static NEXT_ID: AtomicU64 = AtomicU64::new(1); static LIVE_CONNS: AtomicUsize = AtomicUsize::new(0); @@ -458,9 +461,14 @@ mod imp { /// "the user dismissed this" apart from "the app crashed", and remember it /// so requests aimed at that window get the real reason. pub fn note_user_closed_window(window_id: usize, title: &str) { + // Only chatter when the bridge is actually up: this line is for the + // agent driving the app, and a shipped app should not print + // `[makepad-remote] ...` to stdout every time a window closes. let line = format!("[makepad-remote] user closed window {window_id} ({title:?})"); - println!("{line}"); - let _ = std::io::stdout().flush(); + if is_active() { + println!("{line}"); + let _ = std::io::stdout().flush(); + } push_log_line(line); if let Ok(mut closed) = closed_windows().lock() { if !closed.iter().any(|(id, _)| *id == window_id) { @@ -473,8 +481,10 @@ mod imp { /// away. Not a crash. pub fn note_user_closed_last_window() { let line = "[makepad-remote] app exit: user closed the last window".to_string(); - println!("{line}"); - let _ = std::io::stdout().flush(); + if is_active() { + println!("{line}"); + let _ = std::io::stdout().flush(); + } push_log_line(line); } @@ -2128,6 +2138,10 @@ mod imp { use crate::cx::Cx; pub fn start_if_requested() {} + /// There is no remote bridge on these targets, so nothing ever asked for one. + pub fn requested() -> bool { + false + } pub fn is_active() -> bool { false } diff --git a/platform/src/sploded.rs b/platform/src/sploded.rs index 1a389f939..2d12f9913 100644 --- a/platform/src/sploded.rs +++ b/platform/src/sploded.rs @@ -11,7 +11,8 @@ //! scrollbar thumb by dragging is therefore deliberately NOT possible while //! exploded — a drag is the orbit — and that is the coexistence rule. //! -//! F10 tilts the window into an isometric stack that renders **the component +//! F10 — once the dev overlays are switched on ([`crate::devtools`]) — tilts +//! the window into an isometric stack that renders **the component //! nesting structure**: one plane per nesting level, siblings sharing a plane, //! children lifting toward the viewer and their parents staying at the bottom //! of the stack. The point is to see — and click — the fully-covered parent @@ -727,7 +728,12 @@ impl Cx { } match event { Event::KeyDown(e) => { - if e.key_code == KeyCode::F10 { + // F10 is only ours when the app opted into the dev overlays + // (`--devtools` / `MAKEPAD_DEVTOOLS=1` / `--remote`). Otherwise + // it is the app's key like any other. `sploded_toggle` still + // works either way, so an app can put the mode on a key of its + // own choosing. + if e.key_code == KeyCode::F10 && crate::devtools::enabled() { if e.is_repeat { return true; } diff --git a/widgets/src/screen_cap.rs b/widgets/src/screen_cap.rs index 6eeba72b7..ed1c20cba 100644 --- a/widgets/src/screen_cap.rs +++ b/widgets/src/screen_cap.rs @@ -1,5 +1,9 @@ //! ScreenCap — SHIFT+F12 records the window to an mp4, picture and sound. //! +//! The hotkey needs the dev overlays switched on +//! (`makepad_platform::devtools`: `--devtools`, `MAKEPAD_DEVTOOLS=1`, or +//! `--remote`); an app that wants its own recording key calls [`ScreenCap::toggle`]. +//! //! One widget, hardcoded into [`crate::window::Window`] the way the tweaker //! and the nav control are, so every Makepad app can record itself without //! wiring anything up. Shift+F12 starts, Shift+F12 stops. While it records, @@ -43,6 +47,7 @@ use crate::makepad_draw::audio::AudioBuffer; use crate::{makepad_derive_widget::*, makepad_draw::*, widget::*}; use makepad_platform::audio_output_tap::{add_audio_output_tap, remove_audio_output_tap}; +use makepad_platform::devtools; use makepad_platform::screen_capture::{ add_screen_capture, remove_screen_capture, ScreenCaptureOptions, }; @@ -321,7 +326,12 @@ impl ScreenCap { impl Widget for ScreenCap { fn handle_event(&mut self, cx: &mut Cx, event: &Event, _scope: &mut Scope) { if let Event::KeyDown(ke) = event { - if ke.key_code == self.hotkey + // The hotkey only exists once the dev overlays are switched on + // (`--devtools` / `MAKEPAD_DEVTOOLS=1` / `--remote`). A shipped app + // should not have a key that starts writing mp4s to disk; one that + // wants a recorder can call `toggle` from its own binding. + if devtools::enabled() + && ke.key_code == self.hotkey && ke.modifiers.shift == self.hotkey_shift && !ke.is_repeat { diff --git a/widgets/src/text_input.rs b/widgets/src/text_input.rs index e13d184ba..1d57de616 100644 --- a/widgets/src/text_input.rs +++ b/widgets/src/text_input.rs @@ -2653,12 +2653,8 @@ impl Widget for TextInput { // In multiline mode, other modifier combos (Alt+Enter, or Ctrl+Enter // on macOS) insert a newline below when not read-only. let has_physical_keyboard = cx.keyboard.has_physical_keyboard(); - // Ctrl+Enter submits on every platform — on macOS `primary` - // is Cmd, and a person who reaches for Ctrl+Enter to send - // must not get a newline instead. let should_submit = !self.is_multiline || mods.is_primary() - || mods.control || (has_physical_keyboard && self.submit_on_enter && !mods.any()); if should_submit { cx.hide_text_ime(); diff --git a/widgets/src/tweaker.rs b/widgets/src/tweaker.rs index 822436ce9..6a4e8379f 100644 --- a/widgets/src/tweaker.rs +++ b/widgets/src/tweaker.rs @@ -1,7 +1,10 @@ //! The TWEAKER — the design-feedback overlay every `--remote` app grows. //! //! Hardcoded into `Window` (like the caption bar: zero app wiring), inert -//! unless the remote bridge is live, zero cost while off. Turned on (F12 or +//! unless the remote bridge is live, zero cost while off. The F12 key needs +//! the dev overlays switched on (`makepad_platform::devtools`: `--devtools`, +//! `MAKEPAD_DEVTOOLS=1`, or `--remote`); [`set_tweak_on`] is always there for +//! an app that wants to open the panel itself. Turned on (F12 or //! `GET /tweak?on=1`), a person points at the UI and live-edits it while the //! AI watches the same session through the bridge: //! @@ -29,6 +32,7 @@ use crate::{ check_box::{CheckBox, CheckBoxAction}, fab_controls::{format_hex, parse_hex, rgb_to_hsv, FabColorPick, FabColorPickAction, FabValueInput, FabValueInputAction}, + makepad_draw::makepad_platform::devtools, makepad_draw::makepad_platform::sploded::{SPLODED_SPREAD_DEFAULT, SPLODED_SPREAD_MAX, SPLODED_SPREAD_MIN}, file_tree::{FileTree, FileTreeAction}, label::Label, @@ -616,13 +620,19 @@ pub fn window_intercept( // F12 toggles the mode, bridge or no bridge: the design surface is // in-process and owes the remote nothing. Only the HTTP endpoints and // the AI vibecode loop need --remote; without it they simply are not - // there, and the panel still is. + // there, and the panel still is. It does need the dev overlays to be + // switched on though (`--devtools` / `MAKEPAD_DEVTOOLS=1` / `--remote`) — + // in a shipped app F12 belongs to the app, and `set_tweak_on` is still + // there for one that wants to open the panel itself. // // SHIFT+F12 is not ours: that is the screen recorder // (widgets/src/screen_cap.rs), and it must not drag the design surface // into every recording. if let Event::KeyDown(key_event) = event { - if key_event.key_code == KeyCode::F12 && !key_event.modifiers.shift { + if key_event.key_code == KeyCode::F12 + && !key_event.modifiers.shift + && devtools::enabled() + { let flip = { let mut s = session().lock().unwrap(); if s.toggle_event_id != cx.event_id() {