From b9f9cab9145cebc68d34fc30f25f0600c28afc43 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 5 Sep 2023 13:03:37 +0200 Subject: [PATCH 1/4] Update webpki 0.22.0 -> 0.22.1 (#3305) --- Cargo.lock | 4 ++-- deny.toml | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b83be68c8..cc604a480 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4083,9 +4083,9 @@ dependencies = [ [[package]] name = "webpki" -version = "0.22.0" +version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f095d78192e208183081cc07bc5515ef55216397af48b873e5edcd72637fa1bd" +checksum = "f0e74f82d49d545ad128049b7e88f6576df2da6b02e9ce565c6f533be576957e" dependencies = [ "ring", "untrusted", diff --git a/deny.toml b/deny.toml index fa0280809..3c1f21d8c 100644 --- a/deny.toml +++ b/deny.toml @@ -22,7 +22,6 @@ unmaintained = "warn" yanked = "deny" ignore = [ "RUSTSEC-2020-0071", # https://rustsec.org/advisories/RUSTSEC-2020-0071 - chrono/time: Potential segfault in the time crate - "RUSTSEC-2023-0052", # https://rustsec.org/advisories/RUSTSEC-2023-0052 - can be fixed by `cargo update -p ureq`, but then we run into duplicate crates: https://github.com/algesten/ureq/issues/653 ] [bans] From 436996b79e9f1b260e9499c696985cdf8862e650 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 5 Sep 2023 14:11:07 +0200 Subject: [PATCH 2/4] Update web-time to 0.2 (#3308) --- Cargo.lock | 4 ++-- crates/egui-winit/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc604a480..eaaa83aec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4055,9 +4055,9 @@ dependencies = [ [[package]] name = "web-time" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa75ec260dcf59cc310827bae1d7f629809b173dbfe808a633e19754dd4282a5" +checksum = "19353897b48e2c4d849a2d73cb0aeb16dc2be4e00c565abfc11eb65a806e47de" dependencies = [ "js-sys", "once_cell", diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index 1496149ab..e7ec1a27f 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -61,7 +61,7 @@ egui = { version = "0.22.0", path = "../egui", default-features = false, feature ] } log = { version = "0.4", features = ["std"] } raw-window-handle = "0.5.0" -web-time = { version = "0.1" } # We use web-time so we can (maybe) compile for web +web-time = { version = "0.2" } # We use web-time so we can (maybe) compile for web winit = { version = "0.28", default-features = false } #! ### Optional dependencies From 67168be069b2290d48f958cb57324146f3d7c35b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 5 Sep 2023 14:11:22 +0200 Subject: [PATCH 3/4] Improve clippy, and add more docs (#3306) * Silence a few clippy warnings * Use named threads * Remove some deprecated functions * Document Context and Ui fully * Use `parking_lot::Mutex` in `eframe` * Expand clippy.toml files * build fix --- Cargo.lock | 1 + clippy.toml | 61 +++++++++++++++++++++ crates/eframe/Cargo.toml | 3 +- crates/eframe/src/native/run.rs | 9 +-- crates/egui/src/context.rs | 9 ++- crates/egui/src/os.rs | 3 + crates/egui/src/ui.rs | 15 ++--- crates/egui_demo_app/src/backend_panel.rs | 11 ++-- crates/epaint/src/bezier.rs | 2 + crates/epaint/src/mutex.rs | 4 ++ crates/epaint/src/text/text_layout_types.rs | 1 + scripts/clippy_wasm/clippy.toml | 23 +++++++- 12 files changed, 119 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eaaa83aec..6b62a62a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1152,6 +1152,7 @@ dependencies = [ "js-sys", "log", "objc", + "parking_lot", "percent-encoding", "pollster", "puffin", diff --git a/clippy.toml b/clippy.toml index 38feddcf0..6dbb62aad 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,7 +1,68 @@ # There is also a scripts/clippy_wasm/clippy.toml which forbids some mthods that are not available in wasm. +# ----------------------------------------------------------------------------- +# Section identical to scripts/clippy_wasm/clippy.toml: + msrv = "1.67" +allow-unwrap-in-tests = true + +# https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#avoid-breaking-exported-api +# We want suggestions, even if it changes public API. +avoid-breaking-exported-api = false + +max-fn-params-bools = 2 # TODO(emilk): decrease this to 1 + +# https://rust-lang.github.io/rust-clippy/master/index.html#/large_include_file +max-include-file-size = 100000 + +too-many-lines-threshold = 100 + +# ----------------------------------------------------------------------------- + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros +disallowed-macros = [ + 'dbg', + 'std::unimplemented', + + # TODO(emilk): consider forbidding these to encourage the use of proper log stream, and then explicitly allow legitimate uses + # 'std::eprint', + # 'std::eprintln', + # 'std::print', + # 'std::println', +] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods +disallowed-methods = [ + "std::env::temp_dir", # Use the tempdir crate instead + + # There are many things that aren't allowed on wasm, + # but we cannot disable them all here (because of e.g. https://github.com/rust-lang/rust-clippy/issues/10406) + # so we do that in `clipppy_wasm.toml` instead. + + "std::thread::spawn", # Use `std::thread::Builder` and name the thread + + "sha1::Digest::new", # SHA1 is cryptographically broken + + "std::panic::catch_unwind", # We compile with `panic = "abort"` +] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names +disallowed-names = [] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types +disallowed-types = [ + # Use the faster & simpler non-poisonable primitives in `parking_lot` instead + "std::sync::Mutex", + "std::sync::RwLock", + "std::sync::Condvar", + # "std::sync::Once", # enabled for now as the `log_once` macro uses it internally + + "ring::digest::SHA1_FOR_LEGACY_USE_ONLY", # SHA1 is cryptographically broken +] + +# ----------------------------------------------------------------------------- + # Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown doc-valid-idents = [ # You must also update the same list in the root `clippy.toml`! diff --git a/crates/eframe/Cargo.toml b/crates/eframe/Cargo.toml index 782905201..70772f2f6 100644 --- a/crates/eframe/Cargo.toml +++ b/crates/eframe/Cargo.toml @@ -96,6 +96,8 @@ egui = { version = "0.22.0", path = "../egui", default-features = false, feature "log", ] } log = { version = "0.4", features = ["std"] } +parking_lot = "0.12" +static_assertions = "1.1.0" thiserror.workspace = true #! ### Optional dependencies @@ -106,7 +108,6 @@ egui_glow = { version = "0.22.0", path = "../egui_glow", optional = true, defaul glow = { version = "0.12", optional = true } ron = { version = "0.8", optional = true, features = ["integer128"] } serde = { version = "1", optional = true, features = ["derive"] } -static_assertions = "1.1.0" # ------------------------------------------- # native: diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 6a315d1aa..986a86d3f 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -1092,6 +1092,8 @@ pub use glow_integration::run_glow; mod wgpu_integration { use std::sync::Arc; + use parking_lot::Mutex; + use super::*; /// State that is initialized when the application is first starts running via @@ -1104,7 +1106,7 @@ mod wgpu_integration { } struct WgpuWinitApp { - repaint_proxy: Arc>>, + repaint_proxy: Arc>>, app_name: String, native_options: epi::NativeOptions, app_creator: Option, @@ -1130,7 +1132,7 @@ mod wgpu_integration { ); Self { - repaint_proxy: Arc::new(std::sync::Mutex::new(event_loop.create_proxy())), + repaint_proxy: Arc::new(Mutex::new(event_loop.create_proxy())), app_name: app_name.to_owned(), native_options, running: None, @@ -1215,7 +1217,7 @@ mod wgpu_integration { ); #[cfg(feature = "accesskit")] { - integration.init_accesskit(&window, self.repaint_proxy.lock().unwrap().clone()); + integration.init_accesskit(&window, self.repaint_proxy.lock().clone()); } let theme = system_theme.unwrap_or(self.native_options.default_theme); integration.egui_ctx.set_visuals(theme.egui_visuals()); @@ -1232,7 +1234,6 @@ mod wgpu_integration { let frame_nr = info.current_frame_nr; event_loop_proxy .lock() - .unwrap() .send_event(UserEvent::RequestRepaint { when, frame_nr }) .ok(); }); diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 231783fde..8e47f8d61 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1,4 +1,5 @@ -// #![warn(missing_docs)] +#![warn(missing_docs)] // Let's keep `Context` well-documented. + use std::sync::Arc; use crate::{ @@ -1552,6 +1553,7 @@ impl Context { } impl Context { + /// Show a ui for settings (style and tessellation options). pub fn settings_ui(&self, ui: &mut Ui) { use crate::containers::*; @@ -1574,6 +1576,7 @@ impl Context { }); } + /// Show the state of egui, including its input and output. pub fn inspection_ui(&self, ui: &mut Ui) { use crate::containers::*; crate::trace!(ui); @@ -1703,6 +1706,7 @@ impl Context { }); } + /// Shows the contents of [`Self::memory`]. pub fn memory_ui(&self, ui: &mut crate::Ui) { if ui .button("Reset all") @@ -1803,6 +1807,7 @@ impl Context { } impl Context { + /// Edit the active [`Style`]. pub fn style_ui(&self, ui: &mut Ui) { let mut style: Style = (*self.style()).clone(); style.ui(ui); @@ -1818,6 +1823,8 @@ impl Context { /// the function is still called, but with no other effect. /// /// No locks are held while the given closure is called. + #[allow(clippy::unused_self)] + #[inline] pub fn with_accessibility_parent(&self, _id: Id, f: impl FnOnce()) { // TODO(emilk): this isn't thread-safe - another thread can call this function between the push/pop calls #[cfg(feature = "accesskit")] diff --git a/crates/egui/src/os.rs b/crates/egui/src/os.rs index 70f49f905..93db910de 100644 --- a/crates/egui/src/os.rs +++ b/crates/egui/src/os.rs @@ -1,3 +1,5 @@ +/// An `enum` of common operating systems. +#[allow(clippy::upper_case_acronyms)] // `Ios` looks too ugly #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum OperatingSystem { /// Unknown OS - could be wasm @@ -26,6 +28,7 @@ impl Default for OperatingSystem { } impl OperatingSystem { + /// Uses the compile-time `target_arch` to identify the OS. pub const fn from_target_os() -> Self { if cfg!(target_arch = "wasm32") { Self::Unknown diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 686d7a66e..6219fa556 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -1,4 +1,4 @@ -// #![warn(missing_docs)] +#![warn(missing_docs)] // Let's keep `Ui` well-documented. use std::hash::Hash; use std::sync::Arc; @@ -249,11 +249,6 @@ impl Ui { self.painter.is_visible() } - #[deprecated = "Renamed is_visible"] - pub fn visible(&self) -> bool { - self.painter.is_visible() - } - /// Calling `set_visible(false)` will cause all further widgets to be invisible, /// yet still allocate space. /// @@ -281,6 +276,7 @@ impl Ui { } } + /// Read the [`Layout`]. #[inline] pub fn layout(&self) -> &Layout { self.placer.layout() @@ -611,6 +607,7 @@ impl Ui { Id::new(self.next_auto_id_source) } + /// Same as `ui.next_auto_id().with(id_source)` pub fn auto_id_with(&self, id_source: IdSource) -> Id where IdSource: Hash, @@ -618,6 +615,7 @@ impl Ui { Id::new(self.next_auto_id_source).with(id_source) } + /// Pretend like `count` widgets have been allocated. pub fn skip_ahead_auto_ids(&mut self, count: usize) { self.next_auto_id_source = self.next_auto_id_source.wrapping_add(count as u64); } @@ -2038,11 +2036,6 @@ impl Ui { InnerResponse::new(inner, self.interact(rect, child_ui.id, Sense::hover())) } - #[deprecated = "Use ui.vertical_centered or ui.centered_and_justified"] - pub fn centered(&mut self, add_contents: impl FnOnce(&mut Self) -> R) -> InnerResponse { - self.vertical_centered(add_contents) - } - /// This will make the next added widget centered and justified in the available space. /// /// Only one widget may be added to the inner `Ui`! diff --git a/crates/egui_demo_app/src/backend_panel.rs b/crates/egui_demo_app/src/backend_panel.rs index a8222aff2..ebef75f43 100644 --- a/crates/egui_demo_app/src/backend_panel.rs +++ b/crates/egui_demo_app/src/backend_panel.rs @@ -451,10 +451,13 @@ impl EguiWindows { #[cfg(not(target_arch = "wasm32"))] fn call_after_delay(delay: std::time::Duration, f: impl FnOnce() + Send + 'static) { - std::thread::spawn(move || { - std::thread::sleep(delay); - f(); - }); + std::thread::Builder::new() + .name("call_after_delay".to_owned()) + .spawn(move || { + std::thread::sleep(delay); + f(); + }) + .unwrap(); } #[cfg(target_arch = "wasm32")] diff --git a/crates/epaint/src/bezier.rs b/crates/epaint/src/bezier.rs index d379a17a8..3fa1aa793 100644 --- a/crates/epaint/src/bezier.rs +++ b/crates/epaint/src/bezier.rs @@ -1,4 +1,6 @@ #![allow(clippy::many_single_char_names)] +#![allow(clippy::wrong_self_convention)] // False positives + use std::ops::Range; use crate::{shape::Shape, Color32, PathShape, Stroke}; diff --git a/crates/epaint/src/mutex.rs b/crates/epaint/src/mutex.rs index 050fe9dfa..73b54ca89 100644 --- a/crates/epaint/src/mutex.rs +++ b/crates/epaint/src/mutex.rs @@ -374,6 +374,8 @@ where #[cfg(test)] mod tests { + #![allow(clippy::disallowed_methods)] // Ok for tests + use crate::mutex::Mutex; use std::time::Duration; @@ -414,6 +416,8 @@ mod tests { #[cfg(feature = "deadlock_detection")] #[cfg(test)] mod tests_rwlock { + #![allow(clippy::disallowed_methods)] // Ok for tests + use crate::mutex::RwLock; use std::time::Duration; diff --git a/crates/epaint/src/text/text_layout_types.rs b/crates/epaint/src/text/text_layout_types.rs index 96475b278..2cb7bb1df 100644 --- a/crates/epaint/src/text/text_layout_types.rs +++ b/crates/epaint/src/text/text_layout_types.rs @@ -1,4 +1,5 @@ #![allow(clippy::derive_hash_xor_eq)] // We need to impl Hash for f32, but we don't implement Eq, which is fine +#![allow(clippy::wrong_self_convention)] // We use `from_` to indicate conversion direction. It's non-diomatic, but makes sense in this context. use std::ops::Range; use std::sync::Arc; diff --git a/scripts/clippy_wasm/clippy.toml b/scripts/clippy_wasm/clippy.toml index 436e1fb84..09359a065 100644 --- a/scripts/clippy_wasm/clippy.toml +++ b/scripts/clippy_wasm/clippy.toml @@ -3,8 +3,26 @@ # We cannot forbid all these methods in the main `clippy.toml` because of # https://github.com/rust-lang/rust-clippy/issues/10406 +# ----------------------------------------------------------------------------- +# Section identical to the root clippy.toml: + msrv = "1.67" +allow-unwrap-in-tests = true + +# https://doc.rust-lang.org/nightly/clippy/lint_configuration.html#avoid-breaking-exported-api +# We want suggestions, even if it changes public API. +avoid-breaking-exported-api = false + +max-fn-params-bools = 2 # TODO(emilk): decrease this to 1 + +# https://rust-lang.github.io/rust-clippy/master/index.html#/large_include_file +max-include-file-size = 100000 + +too-many-lines-threshold = 100 + +# ----------------------------------------------------------------------------- + # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods disallowed-methods = [ "std::time::Instant::now", # use `instant` crate instead for wasm/web compatibility @@ -17,8 +35,9 @@ disallowed-methods = [ # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types disallowed-types = [ - # Cannot spawn threads on wasm: - "std::thread::Builder", + { path = "instant::SystemTime", reason = "Known bugs. Use web-time." }, + { path = "std::thread::Builder", reason = "Cannot spawn threads on wasm" }, + # { path = "std::path::PathBuf", reason = "Can't read/write files on web" }, // TODO(emilk): consider banning Path on wasm ] # Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown From 707ca04c081c1fa7d00c9d8d88f33ad5bb218714 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 5 Sep 2023 14:11:29 +0200 Subject: [PATCH 4/4] Add opt-in `puffin` feature to `egui-extras` (#3307) * Add opt-in `puffin` feature to `egui-extras` Image loading can be slow. Related to https://github.com/emilk/egui/pull/3297 * Silence warning --- Cargo.lock | 1 + crates/egui_extras/Cargo.toml | 13 ++++++++++--- crates/egui_extras/src/image.rs | 2 ++ crates/egui_extras/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b62a62a3..047903a51 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1265,6 +1265,7 @@ dependencies = [ "egui", "image", "log", + "puffin", "resvg", "serde", "tiny-skia", diff --git a/crates/egui_extras/Cargo.toml b/crates/egui_extras/Cargo.toml index fe3bf8876..23583a22d 100644 --- a/crates/egui_extras/Cargo.toml +++ b/crates/egui_extras/Cargo.toml @@ -29,12 +29,17 @@ default = [] ## Enable [`DatePickerButton`] widget. datepicker = ["chrono"] -## Support loading svg images. -svg = ["resvg", "tiny-skia", "usvg"] - ## Log warnings using [`log`](https://docs.rs/log) crate. log = ["dep:log", "egui/log"] +## Enable profiling with the [`puffin`](https://docs.rs/puffin) crate. +## +## Only enabled on native, because of the low resolution (1ms) of clocks in browsers. +puffin = ["dep:puffin", "egui/puffin"] + +## Support loading svg images. +svg = ["resvg", "tiny-skia", "usvg"] + [dependencies] egui = { version = "0.22.0", path = "../egui", default-features = false } @@ -65,6 +70,8 @@ image = { version = "0.24", optional = true, default-features = false } # feature "log" log = { version = "0.4", optional = true, features = ["std"] } +puffin = { version = "0.16", optional = true } + # svg feature resvg = { version = "0.28", optional = true, default-features = false } tiny-skia = { version = "0.8", optional = true, default-features = false } # must be updated in lock-step with resvg diff --git a/crates/egui_extras/src/image.rs b/crates/egui_extras/src/image.rs index bec11d1ab..9304f2b80 100644 --- a/crates/egui_extras/src/image.rs +++ b/crates/egui_extras/src/image.rs @@ -203,6 +203,7 @@ use egui::ColorImage; /// On invalid image or unsupported image format. #[cfg(feature = "image")] pub fn load_image_bytes(image_bytes: &[u8]) -> Result { + crate::profile_function!(); let image = image::load_from_memory(image_bytes).map_err(|err| err.to_string())?; let size = [image.width() as _, image.height() as _]; let image_buffer = image.to_rgba8(); @@ -235,6 +236,7 @@ pub fn load_svg_bytes_with_size( svg_bytes: &[u8], fit_to: FitTo, ) -> Result { + crate::profile_function!(); let opt = usvg::Options::default(); let rtree = usvg::Tree::from_data(svg_bytes, &opt).map_err(|err| err.to_string())?; diff --git a/crates/egui_extras/src/lib.rs b/crates/egui_extras/src/lib.rs index 243127f24..29475c14d 100644 --- a/crates/egui_extras/src/lib.rs +++ b/crates/egui_extras/src/lib.rs @@ -28,6 +28,38 @@ pub use crate::sizing::Size; pub use crate::strip::*; pub use crate::table::*; +// --------------------------------------------------------------------------- + +mod profiling_scopes { + #![allow(unused_macros)] + #![allow(unused_imports)] + + /// Profiling macro for feature "puffin" + macro_rules! profile_function { + ($($arg: tt)*) => { + #[cfg(not(target_arch = "wasm32"))] // Disabled on web because of the coarse 1ms clock resolution there. + #[cfg(feature = "puffin")] + puffin::profile_function!($($arg)*); + }; + } + pub(crate) use profile_function; + + /// Profiling macro for feature "puffin" + macro_rules! profile_scope { + ($($arg: tt)*) => { + #[cfg(not(target_arch = "wasm32"))] // Disabled on web because of the coarse 1ms clock resolution there. + #[cfg(feature = "puffin")] + puffin::profile_scope!($($arg)*); + }; + } + pub(crate) use profile_scope; +} + +#[allow(unused_imports)] +pub(crate) use profiling_scopes::*; + +// --------------------------------------------------------------------------- + /// Log an error with either `log` or `eprintln` macro_rules! log_err { ($fmt: literal, $($arg: tt)*) => {{