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

Fix panic when dragging window between monitors of different pixels_per_point #4088

Closed
wants to merge 35 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
89d9511
Update context.rs
rustbasic Feb 22, 2024
43657e5
Merge branch 'emilk:master' into patch15
rustbasic Feb 28, 2024
a3c774f
Merge branch 'emilk:master' into patch15
rustbasic Mar 1, 2024
482dd36
Merge branch 'emilk:master' into patch15
rustbasic Mar 5, 2024
ff93318
Merge branch 'emilk:master' into patch15
rustbasic Mar 8, 2024
1f5d8ca
Merge branch 'emilk:master' into patch15
rustbasic Mar 11, 2024
98b6ae4
Merge branch 'emilk:master' into patch15
rustbasic Mar 12, 2024
e55e977
Merge branch 'emilk:master' into patch15
rustbasic Mar 14, 2024
ace1a39
Merge branch 'emilk:master' into patch15
rustbasic Mar 15, 2024
6093dfe
Update layers.rs
rustbasic Mar 16, 2024
4c87831
Merge branch 'emilk:master' into patch15
rustbasic Mar 18, 2024
4f00087
Merge branch 'emilk:master' into patch15
rustbasic Mar 20, 2024
ccb94bf
Merge branch 'emilk:master' into patch15
rustbasic Mar 20, 2024
4aafa01
Update glow_integration.rs
rustbasic Mar 20, 2024
15f4eba
Update wgpu_integration.rs
rustbasic Mar 20, 2024
19c89a3
Merge branch 'emilk:master' into patch15
rustbasic Mar 21, 2024
4763eba
Merge branch 'emilk:master' into patch15
rustbasic Mar 22, 2024
311e64f
Update layers.rs
rustbasic Mar 23, 2024
420c689
Update layers.rs
rustbasic Mar 23, 2024
ac73bbb
Update layers.rs
rustbasic Mar 23, 2024
c66b390
Merge branch 'emilk:master' into patch15
rustbasic Mar 25, 2024
360d864
Merge branch 'emilk:master' into patch15
rustbasic Mar 26, 2024
13acfef
Merge branch 'emilk:master' into patch15
rustbasic Mar 27, 2024
3e09f8f
Merge branch 'emilk:master' into patch15
rustbasic Mar 28, 2024
a24be8c
Merge branch 'emilk:master' into patch15
rustbasic Mar 29, 2024
abf356e
Merge branch 'emilk:master' into patch15
rustbasic Mar 30, 2024
d2ba18e
Update epi_integration.rs
rustbasic Mar 30, 2024
cf3c4a0
Update epi_integration.rs
rustbasic Mar 30, 2024
a742a27
Update epi_integration.rs
rustbasic Mar 30, 2024
e5f328f
Merge branch 'emilk:master' into patch15
rustbasic Mar 30, 2024
ed54385
Merge branch 'emilk:master' into patch15
rustbasic Mar 30, 2024
0d32392
Merge branch 'master' into patch15
rustbasic Mar 31, 2024
5ad0819
Merge branch 'emilk:master' into patch15
rustbasic Apr 1, 2024
8af5e26
Merge branch 'emilk:master' into patch15
rustbasic Apr 1, 2024
015cfdf
Merge branch 'emilk:master' into patch15
rustbasic Apr 1, 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
15 changes: 9 additions & 6 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2171,12 +2171,15 @@ impl Context {

self.write(|ctx| {
let tessellation_options = ctx.memory.options.tessellation_options;
let texture_atlas = ctx
.fonts
.get(&pixels_per_point.into())
.expect("tessellate called with a different pixels_per_point than the font atlas was created with. \
You should use egui::FullOutput::pixels_per_point when tessellating.")
.texture_atlas();
let texture_atlas = match ctx.fonts.get(&pixels_per_point.into()) {
Some(fonts) => fonts.texture_atlas(),
None => {
ctx.fonts.iter().next()
Copy link
Owner

@emilk emilk Mar 20, 2024

Choose a reason for hiding this comment

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

This will fall back to a texture atlas for the wrong pixels_per_point, which will likely look bad. The existing check is there to help users that write egui integration to find bugs in their code.

I'd rather figure out the root cause of the bug than mask it :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will fall back to a texture atlas for the wrong pixels_per_point, which will likely look bad. The existing check is there to help users that write egui integration to find bugs in their code.

I'd rather figure out the root cause of the bug than mask it :/

Don't worry, the texture atlas will be read for good pixel_per_point right away.
However, if you don't do this, a panic will occur before the texture atlas for a good pixel_per_point is read.
How to reproduce the panic is the same as the sequence shown above.

This also eliminates the root cause of the bug.
The root cause of another bug was fixed in #4128, so I didn't find it.

.expect("tessellate called with a different pixels_per_point than the font atlas was created with. \
You should use egui::FullOutput::pixels_per_point when tessellating.")
.1.texture_atlas()
}
};
let (font_tex_size, prepared_discs) = {
let atlas = texture_atlas.lock();
(atlas.size(), atlas.prepared_discs())
Expand Down
4 changes: 4 additions & 0 deletions crates/egui/src/layers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ impl PaintList {
/// and then later setting it using `paint_list.set(idx, cr, frame);`.
#[inline(always)]
pub fn set(&mut self, idx: ShapeIdx, clip_rect: Rect, shape: Shape) {
if self.0.len() <= idx.0 {
self.add(clip_rect, Shape::Noop);
}
Comment on lines +152 to +154
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't be needed unless the user does something wrong, and even then this only works if it is exactly off-by-one

Copy link
Contributor Author

@rustbasic rustbasic Mar 20, 2024

Choose a reason for hiding this comment

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

This shouldn't be needed unless the user does something wrong, and even then this only works if it is exactly off-by-one

I experienced a panic occurring when self.0.len() was 0, and there is no problem if I use this. glow_integration and wgpu_integration must also be changed. I will add that part to this commit as well.

How to reproduce panic:

  1. Run examples/test_viewports.
  2. Open all viewports.
  3. Close “DD -> Top D -> SS -> Top S”. (order required)

Copy link
Owner

Choose a reason for hiding this comment

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

I fail to reproduce.

In any case: try to find the root of the problem instead of adding defensive solutions

Copy link
Owner

Choose a reason for hiding this comment

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

And use RUST_BACKTRACE=1 to get stack traces


self.0[idx.0] = ClippedShape { clip_rect, shape };
}

Expand Down
Loading