skyler
/
egui
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Enable `clippy::iter_over_hash_type` lint (#7421) 

This helped discover a few things that _might_ have been buggy.
This commit is contained in:
Emil Ernerfeldt 2025-08-06 13:55:53 +02:00 committed by GitHub
parent d42ea3800f
commit 36a4981f29
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 114 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock
View File

@ -1290,7 +1290,6 @@ name = "egui-winit"
version = "0.32.0"
dependencies = [
"accesskit_winit",
"ahash",
"arboard",
"bytemuck",
"document-features",
@ -1374,7 +1373,6 @@ dependencies = [
name = "egui_glow"
version = "0.32.0"
dependencies = [
"ahash",
"bytemuck",
"document-features",
"egui",

2
Cargo.toml
View File

@ -190,6 +190,7 @@ iter_filter_is_some = "warn"
iter_not_returning_iterator = "warn"
iter_on_empty_collections = "warn"
iter_on_single_items = "warn"
iter_over_hash_type = "warn"
iter_without_into_iter = "warn"
large_digit_groups = "warn"
large_include_file = "warn"
@ -296,7 +297,6 @@ zero_sized_map_values = "warn"
# TODO(emilk): maybe enable more of these lints?
iter_over_hash_type = "allow"
should_panic_without_expect = "allow"
too_many_lines = "allow"
unwrap_used = "allow" # TODO(emilk): We really wanna warn on this one

30
crates/eframe/src/native/glow_integration.rs
View File

@ -23,10 +23,10 @@ use winit::{
window::{Window, WindowId},
};
use ahash::{HashMap, HashSet};
use ahash::HashMap;
use egui::{
DeferredViewportUiCallback, ImmediateViewport, ViewportBuilder, ViewportClass, ViewportId,
ViewportIdMap, ViewportIdPair, ViewportInfo, ViewportOutput,
DeferredViewportUiCallback, ImmediateViewport, OrderedViewportIdMap, ViewportBuilder,
ViewportClass, ViewportId, ViewportIdPair, ViewportInfo, ViewportOutput,
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
@ -94,9 +94,9 @@ struct GlutinWindowContext {
current_gl_context: Option<glutin::context::PossiblyCurrentContext>,
not_current_gl_context: Option<glutin::context::NotCurrentContext>,
viewports: ViewportIdMap<Viewport>,
viewports: OrderedViewportIdMap<Viewport>,
viewport_from_window: HashMap<WindowId, ViewportId>,
window_from_viewport: ViewportIdMap<WindowId>,
window_from_viewport: OrderedViewportIdMap<WindowId>,
focused_viewport: Option<ViewportId>,
}
@ -107,7 +107,7 @@ struct Viewport {
builder: ViewportBuilder,
deferred_commands: Vec<egui::viewport::ViewportCommand>,
info: ViewportInfo,
actions_requested: HashSet<egui_winit::ActionRequested>,
actions_requested: Vec<egui_winit::ActionRequested>,
/// The user-callback that shows the ui.
/// None for immediate viewports.
@ -671,7 +671,7 @@ impl GlowWinitRunning<'_> {
);
{
for action in viewport.actions_requested.drain() {
for action in viewport.actions_requested.drain(..) {
match action {
ActionRequested::Screenshot(user_data) => {
let screenshot = painter.read_screen_rgba(screen_size_in_pixels);
@ -1031,7 +1031,7 @@ impl GlutinWindowContext {
let not_current_gl_context = Some(gl_context);
let mut viewport_from_window = HashMap::default();
let mut window_from_viewport = ViewportIdMap::default();
let mut window_from_viewport = OrderedViewportIdMap::default();
let mut info = ViewportInfo::default();
if let Some(window) = &window {
viewport_from_window.insert(window.id(), ViewportId::ROOT);
@ -1039,7 +1039,7 @@ impl GlutinWindowContext {
egui_winit::update_viewport_info(&mut info, egui_ctx, window, true);
}
let mut viewports = ViewportIdMap::default();
let mut viewports = OrderedViewportIdMap::default();
viewports.insert(
ViewportId::ROOT,
Viewport {
@ -1265,7 +1265,7 @@ impl GlutinWindowContext {
pub(crate) fn remove_viewports_not_in(
&mut self,
viewport_output: &ViewportIdMap<ViewportOutput>,
viewport_output: &OrderedViewportIdMap<ViewportOutput>,
) {
// GC old viewports
self.viewports
@ -1280,7 +1280,7 @@ impl GlutinWindowContext {
&mut self,
event_loop: &ActiveEventLoop,
egui_ctx: &egui::Context,
viewport_output: &ViewportIdMap<ViewportOutput>,
viewport_output: &OrderedViewportIdMap<ViewportOutput>,
) {
profiling::function_scope!();
@ -1337,7 +1337,7 @@ impl GlutinWindowContext {
}
fn initialize_or_update_viewport(
viewports: &mut ViewportIdMap<Viewport>,
viewports: &mut OrderedViewportIdMap<Viewport>,
ids: ViewportIdPair,
class: ViewportClass,
mut builder: ViewportBuilder,
@ -1345,6 +1345,8 @@ fn initialize_or_update_viewport(
) -> &mut Viewport {
profiling::function_scope!();
use std::collections::btree_map::Entry;
if builder.icon.is_none() {
// Inherit icon from parent
builder.icon = viewports
@ -1353,7 +1355,7 @@ fn initialize_or_update_viewport(
}
match viewports.entry(ids.this) {
std::collections::hash_map::Entry::Vacant(entry) => {
Entry::Vacant(entry) => {
// New viewport:
log::debug!("Creating new viewport {:?} ({:?})", ids.this, builder.title);
entry.insert(Viewport {
@ -1370,7 +1372,7 @@ fn initialize_or_update_viewport(
})
}
std::collections::hash_map::Entry::Occupied(mut entry) => {
Entry::Occupied(mut entry) => {
// Patch an existing viewport:
let viewport = entry.get_mut();

29
crates/eframe/src/native/wgpu_integration.rs
View File

@ -15,10 +15,11 @@ use winit::{
window::{Window, WindowId},
};
use ahash::{HashMap, HashSet, HashSetExt as _};
use ahash::HashMap;
use egui::{
DeferredViewportUiCallback, FullOutput, ImmediateViewport, ViewportBuilder, ViewportClass,
ViewportId, ViewportIdMap, ViewportIdPair, ViewportIdSet, ViewportInfo, ViewportOutput,
DeferredViewportUiCallback, FullOutput, ImmediateViewport, OrderedViewportIdMap,
ViewportBuilder, ViewportClass, ViewportId, ViewportIdPair, ViewportIdSet, ViewportInfo,
ViewportOutput,
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
@ -72,7 +73,7 @@ pub struct SharedState {
focused_viewport: Option<ViewportId>,
}
pub type Viewports = ViewportIdMap<Viewport>;
pub type Viewports = egui::OrderedViewportIdMap<Viewport>;
pub struct Viewport {
ids: ViewportIdPair,
@ -80,7 +81,7 @@ pub struct Viewport {
builder: ViewportBuilder,
deferred_commands: Vec<egui::viewport::ViewportCommand>,
info: ViewportInfo,
actions_requested: HashSet<ActionRequested>,
actions_requested: Vec<ActionRequested>,
/// `None` for sync viewports.
viewport_ui_cb: Option<Arc<DeferredViewportUiCallback>>,
@ -679,7 +680,7 @@ impl WgpuWinitRunning<'_> {
screenshot_commands,
);
for action in viewport.actions_requested.drain() {
for action in viewport.actions_requested.drain(..) {
match action {
ActionRequested::Screenshot { .. } => {
// already handled above
@ -1034,10 +1035,10 @@ fn render_immediate_viewport(
}
pub(crate) fn remove_viewports_not_in(
viewports: &mut ViewportIdMap<Viewport>,
viewports: &mut Viewports,
painter: &mut egui_wgpu::winit::Painter,
viewport_from_window: &mut HashMap<WindowId, ViewportId>,
viewport_output: &ViewportIdMap<ViewportOutput>,
viewport_output: &OrderedViewportIdMap<ViewportOutput>,
) {
let active_viewports_ids: ViewportIdSet = viewport_output.keys().copied().collect();
@ -1050,8 +1051,8 @@ pub(crate) fn remove_viewports_not_in(
/// Add new viewports, and update existing ones:
fn handle_viewport_output(
egui_ctx: &egui::Context,
viewport_output: &ViewportIdMap<ViewportOutput>,
viewports: &mut ViewportIdMap<Viewport>,
viewport_output: &OrderedViewportIdMap<ViewportOutput>,
viewports: &mut Viewports,
painter: &mut egui_wgpu::winit::Painter,
viewport_from_window: &mut HashMap<WindowId, ViewportId>,
) {
@ -1111,6 +1112,8 @@ fn initialize_or_update_viewport<'a>(
viewport_ui_cb: Option<Arc<dyn Fn(&egui::Context) + Send + Sync>>,
painter: &mut egui_wgpu::winit::Painter,
) -> &'a mut Viewport {
use std::collections::btree_map::Entry;
profiling::function_scope!();
if builder.icon.is_none() {
@ -1121,7 +1124,7 @@ fn initialize_or_update_viewport<'a>(
}
match viewports.entry(ids.this) {
std::collections::hash_map::Entry::Vacant(entry) => {
Entry::Vacant(entry) => {
// New viewport:
log::debug!("Creating new viewport {:?} ({:?})", ids.this, builder.title);
entry.insert(Viewport {
@ -1130,14 +1133,14 @@ fn initialize_or_update_viewport<'a>(
builder,
deferred_commands: vec![],
info: Default::default(),
actions_requested: HashSet::new(),
actions_requested: Vec::new(),
viewport_ui_cb,
window: None,
egui_winit: None,
})
}
std::collections::hash_map::Entry::Occupied(mut entry) => {
Entry::Occupied(mut entry) => {
// Patch an existing viewport:
let viewport = entry.get_mut();

2
crates/eframe/src/web/events.rs
View File

@ -286,6 +286,8 @@ pub(crate) fn on_keyup(event: web_sys::KeyboardEvent, runner: &mut AppRunner) {
// See https://github.com/emilk/egui/issues/4724
let keys_down = runner.egui_ctx().input(|i| i.keys_down.clone());
#[expect(clippy::iter_over_hash_type)]
for key in keys_down {
let egui_event = egui::Event::Key {
key,

1
crates/egui-winit/Cargo.toml
View File

@ -57,7 +57,6 @@ x11 = ["winit/x11", "bytemuck"]
[dependencies]
egui = { workspace = true, default-features = false, features = ["log"] }
ahash.workspace = true
log.workspace = true
profiling.workspace = true
raw-window-handle.workspace = true

13
crates/egui-winit/src/lib.rs
View File

@ -22,7 +22,6 @@ mod window_settings;
pub use window_settings::WindowSettings;
use ahash::HashSet;
use raw_window_handle::HasDisplayHandle;
use winit::{
@ -1334,7 +1333,7 @@ pub fn process_viewport_commands(
info: &mut ViewportInfo,
commands: impl IntoIterator<Item = ViewportCommand>,
window: &Window,
actions_requested: &mut HashSet<ActionRequested>,
actions_requested: &mut Vec<ActionRequested>,
) {
for command in commands {
process_viewport_command(egui_ctx, window, command, info, actions_requested);
@ -1346,7 +1345,7 @@ fn process_viewport_command(
window: &Window,
command: ViewportCommand,
info: &mut ViewportInfo,
actions_requested: &mut HashSet<ActionRequested>,
actions_requested: &mut Vec<ActionRequested>,
) {
profiling::function_scope!();
@ -1539,16 +1538,16 @@ fn process_viewport_command(
}
}
ViewportCommand::Screenshot(user_data) => {
actions_requested.insert(ActionRequested::Screenshot(user_data));
actions_requested.push(ActionRequested::Screenshot(user_data));
}
ViewportCommand::RequestCut => {
actions_requested.insert(ActionRequested::Cut);
actions_requested.push(ActionRequested::Cut);
}
ViewportCommand::RequestCopy => {
actions_requested.insert(ActionRequested::Copy);
actions_requested.push(ActionRequested::Copy);
}
ViewportCommand::RequestPaste => {
actions_requested.insert(ActionRequested::Paste);
actions_requested.push(ActionRequested::Paste);
}
}
}

3
crates/egui/src/context.rs
View File

@ -2149,6 +2149,7 @@ impl Context {
self.write(|ctx| {
if ctx.memory.options.zoom_factor != zoom_factor {
ctx.new_zoom_factor = Some(zoom_factor);
#[expect(clippy::iter_over_hash_type)]
for viewport_id in ctx.all_viewport_ids() {
ctx.request_repaint(viewport_id, cause.clone());
}
@ -2275,6 +2276,8 @@ impl Context {
/// Called at the end of the pass.
#[cfg(debug_assertions)]
fn debug_painting(&self) {
#![expect(clippy::iter_over_hash_type)] // ok to be sloppy in debug painting
let paint_widget = |widget: &WidgetRect, text: &str, color: Color32| {
let rect = widget.interact_rect;
if rect.is_positive() {

8
crates/egui/src/data/input.rs
View File

@ -3,7 +3,7 @@
use epaint::ColorImage;
use crate::{
Key, Theme, ViewportId, ViewportIdMap,
Key, OrderedViewportIdMap, Theme, ViewportId, ViewportIdMap,
emath::{Pos2, Rect, Vec2},
};
@ -1132,7 +1132,11 @@ impl RawInput {
} = self;
ui.label(format!("Active viewport: {viewport_id:?}"));
for (id, viewport) in viewports {
let ordered_viewports = viewports
.iter()
.map(|(id, value)| (*id, value))
.collect::<OrderedViewportIdMap<_>>();
for (id, viewport) in ordered_viewports {
ui.group(|ui| {
ui.label(format!("Viewport {id:?}"));
ui.push_id(id, |ui| {

10
crates/egui/src/data/output.rs
View File

@ -1,6 +1,6 @@
//! All the data egui returns to the backend at the end of each frame.
use crate::{RepaintCause, ViewportIdMap, ViewportOutput, WidgetType};
use crate::{OrderedViewportIdMap, RepaintCause, ViewportOutput, WidgetType};
/// What egui emits each frame from [`crate::Context::run`].
///
@ -32,12 +32,14 @@ pub struct FullOutput {
///
/// It is up to the integration to spawn a native window for each viewport,
/// and to close any window that no longer has a viewport in this map.
pub viewport_output: ViewportIdMap<ViewportOutput>,
pub viewport_output: OrderedViewportIdMap<ViewportOutput>,
}
impl FullOutput {
/// Add on new output.
pub fn append(&mut self, newer: Self) {
use std::collections::btree_map::Entry;
let Self {
platform_output,
textures_delta,
@ -53,10 +55,10 @@ impl FullOutput {
for (id, new_viewport) in viewport_output {
match self.viewport_output.entry(id) {
std::collections::hash_map::Entry::Vacant(entry) => {
Entry::Vacant(entry) => {
entry.insert(new_viewport);
}
std::collections::hash_map::Entry::Occupied(mut entry) => {
Entry::Occupied(mut entry) => {
entry.get_mut().append(new_viewport);
}
}

9
crates/egui/src/layers.rs
View File

@ -240,8 +240,7 @@ impl GraphicLayers {
if let Some(list) = order_map.get_mut(&layer_id.id) {
if let Some(to_global) = to_global.get(layer_id) {
for clipped_shape in &mut list.0 {
clipped_shape.clip_rect = *to_global * clipped_shape.clip_rect;
clipped_shape.shape.transform(*to_global);
clipped_shape.transform(*to_global);
}
}
all_shapes.append(&mut list.0);
@ -250,13 +249,15 @@ impl GraphicLayers {
}
// Also draw areas that are missing in `area_order`:
// NOTE: We don't think we end up here in normal situations.
// This is just a safety net in case we have some bug somewhere.
#[expect(clippy::iter_over_hash_type)]
for (id, list) in order_map {
let layer_id = LayerId::new(order, *id);
if let Some(to_global) = to_global.get(&layer_id) {
for clipped_shape in &mut list.0 {
clipped_shape.clip_rect = *to_global * clipped_shape.clip_rect;
clipped_shape.shape.transform(*to_global);
clipped_shape.transform(*to_global);
}
}

15
crates/egui/src/memory/mod.rs
View File

@ -711,6 +711,8 @@ impl Focus {
let mut best_score = f32::INFINITY;
let mut best_id = None;
// iteration order should only matter in case of a tie, and that should be very rare
#[expect(clippy::iter_over_hash_type)]
for (candidate_id, candidate_rect) in &self.focus_widgets_cache {
if *candidate_id == current_focused.id {
continue;
@ -959,6 +961,7 @@ impl Memory {
/// Forget window positions, sizes etc.
/// Can be used to auto-layout windows.
pub fn reset_areas(&mut self) {
#[expect(clippy::iter_over_hash_type)]
for area in self.areas.values_mut() {
*area = Default::default();
}
@ -1324,12 +1327,14 @@ impl Areas {
wants_to_be_on_top.clear();
// For all layers with sublayers, put the sublayers directly after the parent layer:
let sublayers = std::mem::take(sublayers);
for (parent, children) in sublayers {
let mut moved_layers = vec![parent];
// (it doesn't matter in which order we replace parents with their children)
#[expect(clippy::iter_over_hash_type)]
for (parent, children) in std::mem::take(sublayers) {
let mut moved_layers = vec![parent]; // parent first…
order.retain(|l| {
if children.contains(l) {
moved_layers.push(*l);
moved_layers.push(*l); // …followed by children
false
} else {
true
@ -1338,7 +1343,7 @@ impl Areas {
let Some(parent_pos) = order.iter().position(|l| l == &parent) else {
continue;
};
order.splice(parent_pos..=parent_pos, moved_layers);
order.splice(parent_pos..=parent_pos, moved_layers); // replace the parent with itself and its children
}
self.order_map = self

2
crates/egui/src/util/id_type_map.rs
View File

@ -574,6 +574,8 @@ struct PersistedMap(Vec<(u64, SerializedElement)>);
#[cfg(feature = "persistence")]
impl PersistedMap {
fn from_map(map: &IdTypeMap) -> Self {
#![expect(clippy::iter_over_hash_type)] // the serialized order doesn't matter
profiling::function_scope!();
use std::collections::BTreeMap;

17
crates/egui/src/viewport.rs
View File

@ -113,6 +113,20 @@ pub enum ViewportClass {
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct ViewportId(pub Id);
// We implement `PartialOrd` and `Ord` so we can use `ViewportId` in a `BTreeMap`,
// which allows predicatable iteration order, frame-to-frame.
impl PartialOrd for ViewportId {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
Some(self.cmp(other))
}
}
impl Ord for ViewportId {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.0.value().cmp(&other.0.value())
}
}
impl Default for ViewportId {
#[inline]
fn default() -> Self {
@ -151,6 +165,9 @@ pub type ViewportIdSet = nohash_hasher::IntSet<ViewportId>;
/// A fast hash map from [`ViewportId`] to `T`.
pub type ViewportIdMap<T> = nohash_hasher::IntMap<ViewportId, T>;
/// An order map from [`ViewportId`] to `T`.
pub type OrderedViewportIdMap<T> = std::collections::BTreeMap<ViewportId, T>;
// ----------------------------------------------------------------------------
/// Image data for an application icon.

1
crates/egui/src/widget_rect.rs
View File

@ -129,6 +129,7 @@ impl WidgetRects {
infos,
} = self;
#[expect(clippy::iter_over_hash_type)]
for rects in by_layer.values_mut() {
rects.clear();
}

6
crates/egui_demo_lib/src/demo/extra_viewport.rs
View File

@ -54,7 +54,11 @@ fn viewport_content(ui: &mut egui::Ui, ctx: &egui::Context, open: &mut bool) {
egui::ScrollArea::vertical().show(ui, |ui| {
let viewports = ui.input(|i| i.raw.viewports.clone());
for (id, viewport) in viewports {
let ordered_viewports = viewports
.iter()
.map(|(id, viewport)| (*id, viewport.clone()))
.collect::<egui::OrderedViewportIdMap<_>>();
for (id, viewport) in ordered_viewports {
ui.group(|ui| {
ui.label(format!("viewport {id:?}"));
ui.push_id(id, |ui| {

1
crates/egui_glow/Cargo.toml
View File

@ -53,7 +53,6 @@ x11 = ["winit?/x11"]
egui = { workspace = true, default-features = false, features = ["bytemuck"] }
egui-winit = { workspace = true, optional = true, default-features = false }
ahash.workspace = true
bytemuck.workspace = true
glow.workspace = true
log.workspace = true

1
crates/egui_glow/src/painter.rs
View File

@ -698,6 +698,7 @@ impl Painter {
unsafe fn destroy_gl(&self) {
unsafe {
self.gl.delete_program(self.program);
#[expect(clippy::iter_over_hash_type)]
for tex in self.textures.values() {
self.gl.delete_texture(*tex);
}

7
crates/egui_glow/src/winit.rs
View File

@ -1,7 +1,6 @@
use ahash::HashSet;
pub use egui_winit::{self, EventResponse};
use egui::{ViewportId, ViewportOutput};
pub use egui_winit;
pub use egui_winit::EventResponse;
use egui_winit::winit;
use crate::shader_version::ShaderVersion;
@ -81,7 +80,7 @@ impl EguiGlow {
log::warn!("Multiple viewports not yet supported by EguiGlow");
}
for (_, ViewportOutput { commands, .. }) in viewport_output {
let mut actions_requested: HashSet<egui_winit::ActionRequested> = Default::default();
let mut actions_requested = Default::default();
egui_winit::process_viewport_commands(
&self.egui_ctx,
&mut self.viewport_info,

12
crates/epaint/src/lib.rs
View File

@ -127,6 +127,18 @@ pub struct ClippedShape {
pub shape: Shape,
}
impl ClippedShape {
/// Transform (move/scale) the shape in-place.
///
/// If using a [`PaintCallback`], note that only the rect is scaled as opposed
/// to other shapes where the stroke is also scaled.
pub fn transform(&mut self, transform: emath::TSTransform) {
let Self { clip_rect, shape } = self;
*clip_rect = transform * *clip_rect;
shape.transform(transform);
}
}
/// A [`Mesh`] or [`PaintCallback`] within a clip rectangle.
///
/// Everything is using logical points.

2
crates/epaint/src/shapes/shape.rs
View File

@ -416,7 +416,7 @@ impl Shape {
self.transform(TSTransform::from_translation(delta));
}
/// Move the shape by this many points, in-place.
/// Transform (move/scale) the shape in-place.
///
/// If using a [`PaintCallback`], note that only the rect is scaled as opposed
/// to other shapes where the stroke is also scaled.

1
tests/test_viewports/src/main.rs
View File

@ -367,6 +367,7 @@ fn drag_and_drop_test(ui: &mut egui::Ui) {
assert!(col <= COLS, "The col should be less then: {COLS}");
// Should be a better way to do this!
#[expect(clippy::iter_over_hash_type)]
for container_data in self.containers_data.values_mut() {
for ids in container_data {
ids.retain(|i| *i != id);