Rework loading images from files (#7088)

This PR is a follow-up to #7084, where I noted that I wasn't satisfied
with using `SharedUri` to represent both URIs and paths on the local
filesystem:

> I'm still not entirely happy with this naming, as the file paths that
we can store in here are not _really_ URIs, as they are lacking a
protocol.
>
> I want to explore changing `SharedUri` / `SharedUrl` back to alway
storing a URL and treat local filepaths differently, as it seems we're
conflating two different concerns under the same umbrella, at the
moment.

`SharedUri` has now been reverted to just containing a `SharedString`
with a URI.

`ImageSource` now has a new `File` variant that is used to load an image
from a `PathBuf`.

Release Notes:

- N/A
This commit is contained in:
Marshall Bowers 2024-01-30 11:26:02 -05:00 committed by GitHub
parent 6d4fe8098b
commit 2980f0508c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 163 additions and 166 deletions

View File

@ -707,7 +707,7 @@ impl User {
Arc::new(User {
id: message.id,
github_login: message.github_login,
avatar_uri: SharedUri::network(message.avatar_url),
avatar_uri: message.avatar_url.into(),
})
}
}

View File

@ -9,7 +9,7 @@ use fs::{repository::GitFileStatus, FakeFs, Fs as _, RemoveOptions};
use futures::StreamExt as _;
use gpui::{
px, size, AppContext, BackgroundExecutor, Model, Modifiers, MouseButton, MouseDownEvent,
SharedUri, TestAppContext,
TestAppContext,
};
use language::{
language_settings::{AllLanguageSettings, Formatter},
@ -1828,7 +1828,7 @@ async fn test_active_call_events(
owner: Arc::new(User {
id: client_a.user_id().unwrap(),
github_login: "user_a".to_string(),
avatar_uri: SharedUri::network("avatar_a"),
avatar_uri: "avatar_a".into(),
}),
project_id: project_a_id,
worktree_root_names: vec!["a".to_string()],
@ -1846,7 +1846,7 @@ async fn test_active_call_events(
owner: Arc::new(User {
id: client_b.user_id().unwrap(),
github_login: "user_b".to_string(),
avatar_uri: SharedUri::network("avatar_b"),
avatar_uri: "avatar_b".into(),
}),
project_id: project_b_id,
worktree_root_names: vec!["b".to_string()]

View File

@ -714,7 +714,7 @@ fn format_timestamp(
#[cfg(test)]
mod tests {
use super::*;
use gpui::{HighlightStyle, SharedUri};
use gpui::HighlightStyle;
use pretty_assertions::assert_eq;
use rich_text::Highlight;
use time::{Date, OffsetDateTime, Time, UtcOffset};
@ -730,7 +730,7 @@ mod tests {
timestamp: OffsetDateTime::now_utc(),
sender: Arc::new(client::User {
github_login: "fgh".into(),
avatar_uri: SharedUri::network("avatar_fgh"),
avatar_uri: "avatar_fgh".into(),
id: 103,
}),
nonce: 5,

View File

@ -365,7 +365,7 @@ impl Render for MessageEditor {
mod tests {
use super::*;
use client::{Client, User, UserStore};
use gpui::{SharedUri, TestAppContext};
use gpui::TestAppContext;
use language::{Language, LanguageConfig};
use rpc::proto;
use settings::SettingsStore;
@ -392,7 +392,7 @@ mod tests {
user: Arc::new(User {
github_login: "a-b".into(),
id: 101,
avatar_uri: SharedUri::network("avatar_a-b"),
avatar_uri: "avatar_a-b".into(),
}),
kind: proto::channel_member::Kind::Member,
role: proto::ChannelRole::Member,
@ -401,7 +401,7 @@ mod tests {
user: Arc::new(User {
github_login: "C_D".into(),
id: 102,
avatar_uri: SharedUri::network("avatar_C_D"),
avatar_uri: "avatar_C_D".into(),
}),
kind: proto::channel_member::Kind::Member,
role: proto::ChannelRole::Member,

View File

@ -1,4 +1,4 @@
use gpui::{prelude::*, SharedUri};
use gpui::prelude::*;
use story::{StoryContainer, StoryItem, StorySection};
use ui::prelude::*;
@ -19,7 +19,7 @@ impl Render for CollabNotificationStory {
"Incoming Call Notification",
window_container(400., 72.).child(
CollabNotification::new(
SharedUri::network("https://avatars.githubusercontent.com/u/1486634?v=4"),
"https://avatars.githubusercontent.com/u/1486634?v=4",
Button::new("accept", "Accept"),
Button::new("decline", "Decline"),
)
@ -36,7 +36,7 @@ impl Render for CollabNotificationStory {
"Project Shared Notification",
window_container(400., 72.).child(
CollabNotification::new(
SharedUri::network("https://avatars.githubusercontent.com/u/1714999?v=4"),
"https://avatars.githubusercontent.com/u/1714999?v=4",
Button::new("open", "Open"),
Button::new("dismiss", "Dismiss"),
)

View File

@ -1,12 +1,25 @@
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::Arc;
use gpui::*;
#[derive(IntoElement)]
struct ImageFromResource {
struct ImageContainer {
text: SharedString,
resource: SharedUri,
src: ImageSource,
}
impl RenderOnce for ImageFromResource {
impl ImageContainer {
pub fn new(text: impl Into<SharedString>, src: impl Into<ImageSource>) -> Self {
Self {
text: text.into(),
src: src.into(),
}
}
}
impl RenderOnce for ImageContainer {
fn render(self, _: &mut WindowContext) -> impl IntoElement {
div().child(
div()
@ -14,13 +27,13 @@ impl RenderOnce for ImageFromResource {
.size_full()
.gap_4()
.child(self.text)
.child(img(self.resource).w(px(512.0)).h(px(512.0))),
.child(img(self.src).w(px(512.0)).h(px(512.0))),
)
}
}
struct ImageShowcase {
local_resource: SharedUri,
local_resource: Arc<PathBuf>,
remote_resource: SharedUri,
}
@ -34,14 +47,14 @@ impl Render for ImageShowcase {
.items_center()
.gap_8()
.bg(rgb(0xFFFFFF))
.child(ImageFromResource {
text: "Image loaded from a local file".into(),
resource: self.local_resource.clone(),
})
.child(ImageFromResource {
text: "Image loaded from a remote resource".into(),
resource: self.remote_resource.clone(),
})
.child(ImageContainer::new(
"Image loaded from a local file",
self.local_resource.clone(),
))
.child(ImageContainer::new(
"Image loaded from a remote resource",
self.remote_resource.clone(),
))
}
}
@ -51,8 +64,10 @@ fn main() {
App::new().run(|cx: &mut AppContext| {
cx.open_window(WindowOptions::default(), |cx| {
cx.new_view(|_cx| ImageShowcase {
local_resource: SharedUri::file("../zed/resources/app-icon.png"),
remote_resource: SharedUri::network("https://picsum.photos/512/512"),
local_resource: Arc::new(
PathBuf::from_str("crates/zed/resources/app-icon.png").unwrap(),
),
remote_resource: "https://picsum.photos/512/512".into(),
})
});
});

View File

@ -1,9 +1,10 @@
use std::path::PathBuf;
use std::sync::Arc;
use crate::{
point, size, Bounds, DevicePixels, Element, ElementContext, ImageData, InteractiveElement,
InteractiveElementState, Interactivity, IntoElement, LayoutId, Pixels, SharedUri, Size,
StyleRefinement, Styled,
StyleRefinement, Styled, UriOrPath,
};
use futures::FutureExt;
use media::core_video::CVImageBuffer;
@ -14,6 +15,8 @@ use util::ResultExt;
pub enum ImageSource {
/// Image content will be loaded from provided URI at render time.
Uri(SharedUri),
/// Image content will be loaded from the provided file at render time.
File(Arc<PathBuf>),
/// Cached image data
Data(Arc<ImageData>),
// TODO: move surface definitions into mac platform module
@ -27,6 +30,24 @@ impl From<SharedUri> for ImageSource {
}
}
impl From<&'static str> for ImageSource {
fn from(uri: &'static str) -> Self {
Self::Uri(uri.into())
}
}
impl From<String> for ImageSource {
fn from(uri: String) -> Self {
Self::Uri(uri.into())
}
}
impl From<Arc<PathBuf>> for ImageSource {
fn from(value: Arc<PathBuf>) -> Self {
Self::File(value)
}
}
impl From<Arc<ImageData>> for ImageSource {
fn from(value: Arc<ImageData>) -> Self {
Self::Data(value)
@ -91,8 +112,14 @@ impl Element for Img {
let corner_radii = style.corner_radii.to_pixels(bounds.size, cx.rem_size());
cx.with_z_index(1, |cx| {
match source {
ImageSource::Uri(uri) => {
let image_future = cx.image_cache.get(uri.clone(), cx);
ImageSource::Uri(_) | ImageSource::File(_) => {
let uri_or_path: UriOrPath = match source {
ImageSource::Uri(uri) => uri.into(),
ImageSource::File(path) => path.into(),
_ => unreachable!(),
};
let image_future = cx.image_cache.get(uri_or_path.clone(), cx);
if let Some(data) = image_future
.clone()
.now_or_never()

View File

@ -3,6 +3,7 @@ use collections::HashMap;
use futures::{future::Shared, AsyncReadExt, FutureExt, TryFutureExt};
use image::ImageError;
use parking_lot::Mutex;
use std::path::PathBuf;
use std::sync::Arc;
use thiserror::Error;
use util::http::{self, HttpClient};
@ -41,7 +42,25 @@ impl From<ImageError> for Error {
pub(crate) struct ImageCache {
client: Arc<dyn HttpClient>,
images: Arc<Mutex<HashMap<SharedUri, FetchImageTask>>>,
images: Arc<Mutex<HashMap<UriOrPath, FetchImageTask>>>,
}
#[derive(Debug, PartialEq, Eq, Hash, Clone)]
pub enum UriOrPath {
Uri(SharedUri),
Path(Arc<PathBuf>),
}
impl From<SharedUri> for UriOrPath {
fn from(value: SharedUri) -> Self {
Self::Uri(value)
}
}
impl From<Arc<PathBuf>> for UriOrPath {
fn from(value: Arc<PathBuf>) -> Self {
Self::Path(value)
}
}
type FetchImageTask = Shared<Task<Result<Arc<ImageData>, Error>>>;
@ -54,11 +73,11 @@ impl ImageCache {
}
}
pub fn get(&self, uri: impl Into<SharedUri>, cx: &AppContext) -> FetchImageTask {
let uri = uri.into();
pub fn get(&self, uri_or_path: impl Into<UriOrPath>, cx: &AppContext) -> FetchImageTask {
let uri_or_path = uri_or_path.into();
let mut images = self.images.lock();
match images.get(&uri) {
match images.get(&uri_or_path) {
Some(future) => future.clone(),
None => {
let client = self.client.clone();
@ -66,14 +85,14 @@ impl ImageCache {
.background_executor()
.spawn(
{
let uri = uri.clone();
let uri_or_path = uri_or_path.clone();
async move {
match uri {
SharedUri::File(uri) => {
match uri_or_path {
UriOrPath::Path(uri) => {
let image = image::open(uri.as_ref())?.into_bgra8();
Ok(Arc::new(ImageData::new(image)))
}
SharedUri::Network(uri) => {
UriOrPath::Uri(uri) => {
let mut response =
client.get(uri.as_ref(), ().into(), true).await?;
let mut body = Vec::new();
@ -96,16 +115,16 @@ impl ImageCache {
}
}
.map_err({
let uri = uri.clone();
let uri_or_path = uri_or_path.clone();
move |error| {
log::log!(log::Level::Error, "{:?} {:?}", &uri, &error);
log::log!(log::Level::Error, "{:?} {:?}", &uri_or_path, &error);
error
}
}),
)
.shared();
images.insert(uri, future.clone());
images.insert(uri_or_path, future.clone());
future
}
}

View File

@ -1,65 +1,25 @@
use std::ops::{Deref, DerefMut};
use derive_more::{Deref, DerefMut};
use crate::SharedString;
/// A URI stored in a [`SharedString`].
#[derive(PartialEq, Eq, Hash, Clone)]
pub enum SharedUri {
/// A path to a local file.
File(SharedString),
/// A URL to a remote resource.
Network(SharedString),
}
impl SharedUri {
/// Creates a [`SharedUri`] pointing to a local file.
pub fn file<S: Into<SharedString>>(s: S) -> Self {
Self::File(s.into())
}
/// Creates a [`SharedUri`] pointing to a remote resource.
pub fn network<S: Into<SharedString>>(s: S) -> Self {
Self::Network(s.into())
}
}
impl Default for SharedUri {
fn default() -> Self {
Self::Network(SharedString::default())
}
}
impl Deref for SharedUri {
type Target = SharedString;
fn deref(&self) -> &Self::Target {
match self {
Self::File(s) => s,
Self::Network(s) => s,
}
}
}
impl DerefMut for SharedUri {
fn deref_mut(&mut self) -> &mut Self::Target {
match self {
Self::File(s) => s,
Self::Network(s) => s,
}
}
}
/// A [`SharedString`] containing a URI.
#[derive(Deref, DerefMut, Default, PartialEq, Eq, Hash, Clone)]
pub struct SharedUri(SharedString);
impl std::fmt::Debug for SharedUri {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::File(s) => write!(f, "File({:?})", s),
Self::Network(s) => write!(f, "Network({:?})", s),
}
self.0.fmt(f)
}
}
impl std::fmt::Display for SharedUri {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.as_ref())
write!(f, "{}", self.0.as_ref())
}
}
impl<T: Into<SharedString>> From<T> for SharedUri {
fn from(value: T) -> Self {
Self(value.into())
}
}

View File

@ -1,4 +1,4 @@
use gpui::{div, img, px, IntoElement, ParentElement, Render, SharedUri, Styled, ViewContext};
use gpui::{div, img, px, IntoElement, ParentElement, Render, Styled, ViewContext};
use story::Story;
use crate::{ActiveTheme, PlayerColors};
@ -53,12 +53,10 @@ impl Render for PlayerStory {
.border_2()
.border_color(player.cursor)
.child(
img(SharedUri::network(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))
.rounded_full()
.size_6()
.bg(gpui::red()),
img("https://avatars.githubusercontent.com/u/1714999?v=4")
.rounded_full()
.size_6()
.bg(gpui::red()),
)
}),
))
@ -84,12 +82,10 @@ impl Render for PlayerStory {
.border_color(player.background)
.size(px(28.))
.child(
img(SharedUri::network(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
img("https://avatars.githubusercontent.com/u/1714999?v=4")
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
),
)
.child(
@ -102,12 +98,10 @@ impl Render for PlayerStory {
.border_color(player.background)
.size(px(28.))
.child(
img(SharedUri::network(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
img("https://avatars.githubusercontent.com/u/1714999?v=4")
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
),
)
.child(
@ -120,12 +114,10 @@ impl Render for PlayerStory {
.border_color(player.background)
.size(px(28.))
.child(
img(SharedUri::network(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
img("https://avatars.githubusercontent.com/u/1714999?v=4")
.rounded_full()
.size(px(24.))
.bg(gpui::red()),
),
)
}),

View File

@ -1,4 +1,4 @@
use gpui::{Render, SharedUri};
use gpui::Render;
use story::{StoryContainer, StoryItem, StorySection};
use crate::{prelude::*, AudioStatus, Availability, AvatarAvailabilityIndicator};
@ -13,66 +13,50 @@ impl Render for AvatarStory {
StorySection::new()
.child(StoryItem::new(
"Default",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/1714999?v=4",
)),
Avatar::new("https://avatars.githubusercontent.com/u/1714999?v=4"),
))
.child(StoryItem::new(
"Default",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
)),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4"),
)),
)
.child(
StorySection::new()
.child(StoryItem::new(
"With free availability indicator",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.indicator(AvatarAvailabilityIndicator::new(Availability::Free)),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.indicator(AvatarAvailabilityIndicator::new(Availability::Free)),
))
.child(StoryItem::new(
"With busy availability indicator",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.indicator(AvatarAvailabilityIndicator::new(Availability::Busy)),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.indicator(AvatarAvailabilityIndicator::new(Availability::Busy)),
)),
)
.child(
StorySection::new()
.child(StoryItem::new(
"With info border",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.border_color(cx.theme().status().info_border),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.border_color(cx.theme().status().info_border),
))
.child(StoryItem::new(
"With error border",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.border_color(cx.theme().status().error_border),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.border_color(cx.theme().status().error_border),
)),
)
.child(
StorySection::new()
.child(StoryItem::new(
"With muted audio indicator",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.indicator(AvatarAudioStatusIndicator::new(AudioStatus::Muted)),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.indicator(AvatarAudioStatusIndicator::new(AudioStatus::Muted)),
))
.child(StoryItem::new(
"With deafened audio indicator",
Avatar::new(SharedUri::network(
"https://avatars.githubusercontent.com/u/326587?v=4",
))
.indicator(AvatarAudioStatusIndicator::new(AudioStatus::Deafened)),
Avatar::new("https://avatars.githubusercontent.com/u/326587?v=4")
.indicator(AvatarAudioStatusIndicator::new(AudioStatus::Deafened)),
)),
)
}

View File

@ -1,4 +1,4 @@
use gpui::{Render, SharedUri};
use gpui::Render;
use story::Story;
use crate::{prelude::*, Avatar};
@ -45,17 +45,17 @@ impl Render for ListItemStory {
.child(
ListItem::new("with_start slot avatar")
.child("Hello, world!")
.start_slot(Avatar::new(SharedUri::network(
.start_slot(Avatar::new(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))),
)),
)
.child(Story::label("With end slot"))
.child(
ListItem::new("with_left_avatar")
.child("Hello, world!")
.end_slot(Avatar::new(SharedUri::network(
.end_slot(Avatar::new(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))),
)),
)
.child(Story::label("With end hover slot"))
.child(
@ -64,25 +64,25 @@ impl Render for ListItemStory {
.end_slot(
h_flex()
.gap_2()
.child(Avatar::new(SharedUri::network(
.child(Avatar::new(
"https://avatars.githubusercontent.com/u/1789?v=4",
)))
.child(Avatar::new(SharedUri::network(
))
.child(Avatar::new(
"https://avatars.githubusercontent.com/u/1789?v=4",
)))
.child(Avatar::new(SharedUri::network(
))
.child(Avatar::new(
"https://avatars.githubusercontent.com/u/1789?v=4",
)))
.child(Avatar::new(SharedUri::network(
))
.child(Avatar::new(
"https://avatars.githubusercontent.com/u/1789?v=4",
)))
.child(Avatar::new(SharedUri::network(
))
.child(Avatar::new(
"https://avatars.githubusercontent.com/u/1789?v=4",
))),
)),
)
.end_hover_slot(Avatar::new(SharedUri::network(
.end_hover_slot(Avatar::new(
"https://avatars.githubusercontent.com/u/1714999?v=4",
))),
)),
)
.child(Story::label("With `on_click`"))
.child(