-
Notifications
You must be signed in to change notification settings - Fork 16
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
Implement Metadata/Information display for files (and preview) #184
Conversation
Hi, thanks for the kind words and your interest in the project! With the PR #170 it is now already possible to render your own ui in the right panel without having to change anything in the file dialog code. This would allow you to actually implement the previews directly in your crate instead of a fork. However, I think it is a good idea if the file dialog already shows a default UI with file information in the right panel and possibly also an image preview. I've already skimmed through your code and here are some first thoughts:
Let me know when you have continued working on the PR and I should take another look at it. Overall, I think the idea is great :) |
awesome, thanks a lot for looking at this! egui_extras = { version = "0.29", features = ["all_loaders"] }
image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } I'm currently thinking of implementing a function We would probably still need to store the image and file content in the file dialog code or |
I would use the methods completely internally. So I would do it like this: by default the file dialog itself calls the method But there should then be the config option |
Mhh, I think I'll change my mind and an exposed We could solve this by default, if enabled, the |
What do you think of this: self.file_dialog.update_with_right_panel_ui(ctx, &mut Information_panel::new().ui); then one could change things in this struct, by for example: self.file_dialog.update_with_right_panel_ui(ctx, &mut Information_panel::new().show_file_modified().ui); I'm not sure how we could add the possibility for users to easily add other file-type readers / displays. |
I like the idea. A preview of custom file types can then be implemented relatively easily as follows: self.file_dialog.update_with_right_panel_ui(ctx, |ui, dialog| {
// Show custom preview for unsupported file formats
if dialog.active_entry().path().extension() == "csv" {
ui.label("My custom preview for csv files!");
}
// Show default file information below the custom preview
InformationPanel::new().ui(ui, dialog);
}); I think this is the easiest and most flexible way. Quick note: The panel should be named in Camel Case: |
Ah yeah, sorry, I first had the Panel as a function, I see an issue with the implementation above: self.file_dialog.update_with_right_panel_ui(ctx, |ui, dialog| {
// Show default file information below the custom preview
InformationPanel::new().add_file_preview("csv", |ui| {
ui.label("My custom preview for csv files!");}
).ui(ui, dialog);
}); This would also allow overriding existing previews. |
That's a good idea! Then we can implement default file previews but also still offer the option of overwriting the defaults. |
But I think the constructor should be called in the constructor of the app, and not inside the loop? struct MyApp {
file_dialog: FileDialog,
information_panel: InformationPanel,
selected_file: Option<PathBuf>,
}
impl MyApp {
pub fn new(_cc: &eframe::CreationContext) -> Self {
Self {
file_dialog: FileDialog::new(),
information_panel: InformationPanel::new()
.add_file_preview("csv", |ui| {
ui.label("my csv preview.");
}),
selected_file: None,
}
}
}
impl eframe::App for MyApp {
fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
egui::CentralPanel::default().show(ctx, |ui| {
if ui.button("Select file").clicked() {
self.file_dialog.select_file();
}
self.file_dialog.update_with_right_panel_ui(ctx, &mut |ui, dia| {
self.information_panel.ui(ui, dia);
});
ui.label(format!("Selected file: {:?}", self.selected_file));
});
}
}
fn main() -> eframe::Result<()> {
eframe::run_native(
"File dialog example",
eframe::NativeOptions::default(),
Box::new(|ctx| {
egui_extras::install_image_loaders(&ctx.egui_ctx);
Ok(Box::new(MyApp::new(ctx)))
}),
)
}
|
Yep, that's probably better. But the app that uses the info panel can decide for itself. Shouldn't change anything in the implementation of the feature :) |
Okay, I reverted the changes from before and came up with a structure. Still a lot of TODOs.
The 'renderers' are now set in a HashMap: pub fn add_file_preview(
mut self,
extension: &str,
add_contents: impl FnMut(&mut Ui, Option<String>, Option<RgbaImage>) + 'static,
) -> Self {
self.supported_files
.insert(extension.to_string(), Box::new(add_contents));
self
} and can have access text (file content) and image (preview image). Not sure if the thumbnail-image can not be extracted from the metadata though... We will see if this is still needed later. information_panel: InformationPanel::new().add_file_preview(
"csv",
|ui, text, image| {
ui.label("CSV preview:");
if let Some(content) = text {
egui::ScrollArea::vertical()
.max_height(100.0)
.show(ui, |ui| {
ui.add(
egui::TextEdit::multiline(&mut content.clone()).code_editor(),
);
});
}
},
), Changes to
|
f6e0b01
to
2de8627
Compare
Thanks in advance for implementing this! Here are already a few points that caught my attention:
This is a really big feature, thank you for taking the time to do this work :) |
Thanks for the feedback!
ui.image(format!("file://{}",active_entry.as_path().display()));
|
But that shouldn't be the case with the images. According to the documentation (https://docs.rs/egui/latest/egui/load/index.html) and my experience, the images are only loaded once and then automatically stored inside the context using the image loaders. If the performance is not good, something else is wrong. I have to try it out myself later today |
Yeah, I see that, I'm also puzzled why the images do not seem to be cached... maybe I need to profile it to see where it uses all the resources.. |
What do you mean by opening the thumbnail from the exif data? Are you able to share your test code including the image you are trying to display? Then I can try it out for myself and see what the current performance is like. 16 MB image files are or larger are relatively common, so we should try to get the performance as good as possible. I'm just thinking about it... Maybe at some point we want to show preview images in the central view. Then we would need that in addition to the meta information for each image in the directory 🤔 |
to my knowledge, JPEG files contain a thumbnail in the EXIF. This allows to read only the metadata and not the entire file, and then subsequently resizing it to display the preview. For PNGs we would still have to do that though... My test code is as is on my branch, but with lines 41-68 in supported_files.insert(
"png".to_string(),
Box::new(
|ui: &mut Ui, text: Option<String>, image: Option<RgbaImage>, item: &DirectoryEntry| {
ui.label("Image");
ui.image(format!("file://{}", item.as_path().display()));
},
) as Box<dyn FnMut(&mut Ui, Option<String>, Option<RgbaImage>, &DirectoryEntry)>, The test image is in the root directory: |
I now found the time to test the However, the problem is not with the let image = if self.load_image_content {
image::open(path)
.map(|img| {
let new_width = 100;
let (original_width, original_height) = img.dimensions();
let new_height = (original_height as f32 * new_width as f32
/ original_width as f32)
as u32;
// Resize the image to the new dimensions (100px width)
let img = img.resize(
new_width,
new_height,
image::imageops::FilterType::Lanczos3,
);
img.into_rgba8()
})
.ok()
} else {
None
}; in However, with such a large image there is another problem: It seems as if the But I think this is exclusively a problem with the |
Yep, this works! |
Co-authored-by: Jannis <[email protected]>
Co-authored-by: Jannis <[email protected]>
@hacknus is the PR ready for another review? |
@fluxxcode Yes, I'm through now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last few cleanup threads
Co-authored-by: Jannis <[email protected]>
Co-authored-by: Jannis <[email protected]>
Thank you very much for the work on this great project!
I'm currently implementing preview functionality for this crate.
It is still WIP, I'm mostly doing this for a custom file preview implementation for my project, so I'm not sure how much I will be pushing this further on (depends a bit on the responses here).
I currently implemented an image preview and a text-preview for text files, csv, etc...
The image preview is very slow in dev mode but in release mode, it is almost not noticeable on my machine (Apple M2, SSD), not sure how this could be improved.
Overall, this is a rather 'heavy' thing, so it would probably be best to have it as a feature for the crate, not enabled per default.