Skip to content

Commit

Permalink
egui_extras: Fixed handling of file:// protocol for images (#4107)
Browse files Browse the repository at this point in the history
* Remove the leading slash from the path if the target OS is Windows.

This is because Windows paths are not supposed to start with a slash.
For example, `file:///C:/path/to/file` is a valid URI, but
`/C:/path/to/file` is not a valid path.

* Use the input URI consistently as the cache key.

Currently, the cache key is inconsistently set as either the path or the
URI, while the forget key is always the URI. This inconsistency should
be resolved.

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
  • Loading branch information
varphone and emilk authored Mar 8, 2024
1 parent 4d776fd commit 1f414c0
Showing 1 changed file with 19 additions and 6 deletions.
25 changes: 19 additions & 6 deletions crates/egui_extras/src/loaders/file_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,32 @@ impl FileLoader {

const PROTOCOL: &str = "file://";

/// Remove the leading slash from the path if the target OS is Windows.
///
/// This is because Windows paths are not supposed to start with a slash.
/// For example, `file:///C:/path/to/file` is a valid URI, but `/C:/path/to/file` is not a valid path.
#[inline]
fn trim_extra_slash(s: &str) -> &str {
if cfg!(target_os = "windows") {
s.trim_start_matches('/')
} else {
s
}
}

impl BytesLoader for FileLoader {
fn id(&self) -> &str {
Self::ID
}

fn load(&self, ctx: &egui::Context, uri: &str) -> BytesLoadResult {
// File loader only supports the `file` protocol.
let Some(path) = uri.strip_prefix(PROTOCOL) else {
let Some(path) = uri.strip_prefix(PROTOCOL).map(trim_extra_slash) else {
return Err(LoadError::NotSupported);
};

let mut cache = self.cache.lock();
if let Some(entry) = cache.get(path).cloned() {
if let Some(entry) = cache.get(uri).cloned() {
// `path` has either begun loading, is loaded, or has failed to load.
match entry {
Poll::Ready(Ok(file)) => Ok(BytesPoll::Ready {
Expand All @@ -54,7 +67,7 @@ impl BytesLoader for FileLoader {

// Set the file to `pending` until we finish loading it.
let path = path.to_owned();
cache.insert(path.clone(), Poll::Pending);
cache.insert(uri.to_owned(), Poll::Pending);
drop(cache);

// Spawn a thread to read the file, so that we don't block the render for too long.
Expand All @@ -63,7 +76,7 @@ impl BytesLoader for FileLoader {
.spawn({
let ctx = ctx.clone();
let cache = self.cache.clone();
let _uri = uri.to_owned();
let uri = uri.to_owned();
move || {
let result = match std::fs::read(&path) {
Ok(bytes) => {
Expand All @@ -82,10 +95,10 @@ impl BytesLoader for FileLoader {
}
Err(err) => Err(err.to_string()),
};
let prev = cache.lock().insert(path, Poll::Ready(result));
let prev = cache.lock().insert(uri.clone(), Poll::Ready(result));
assert!(matches!(prev, Some(Poll::Pending)));
ctx.request_repaint();
log::trace!("finished loading {_uri:?}");
log::trace!("finished loading {uri:?}");
}
})
.expect("failed to spawn thread");
Expand Down

0 comments on commit 1f414c0

Please sign in to comment.