Add image dimension and file size information #21675

kaf-lamed-beyt · 2024-12-07T02:53:37Z

@jansol, kindly take a look when you're free.

Release Notes:

Add image dimension and file size information

cla-bot · 2024-12-07T02:53:41Z

We require contributors to sign our Contributor License Agreement, and we don't have @kaf-lamed-beyt on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

kaf-lamed-beyt · 2024-12-07T02:56:50Z

@cla-bot check

cla-bot · 2024-12-07T02:56:54Z

The cla-bot has been summoned, and re-checked this pull request!

Angelk90 · 2024-12-07T10:18:34Z

@kaf-lamed-beyt :
Great job, personally I would do this:

I would remove the Dimension: and Image size:
I would show this information here below on the right at the beginning of these icons

You can take a screenshot of the image located at the following path: ./crates/zed/resources/app-icon-dev.png

I don't understand why webstorm and vscode give two different weight dimensions.

Vscode:

Webstorm:

kaf-lamed-beyt · 2024-12-07T10:48:04Z

Alright! Thank you for the feedback @Angelk90. I'll take the screenshot and share it here.

Quick question though... which crate would I need to update to show this image data in the location you asked me to move it? I'm thinking this one: crates/zed/src/zed/app_menus.rs?

Perhaps, using the StatusItemView?

kaf-lamed-beyt · 2024-12-07T10:57:43Z

Here's what i have

Angelk90 · 2024-12-07T11:46:57Z

@kaf-lamed-beyt : Maybe I figured out why the size of webstorm is different, it is measured using kB and not KB.

kB = 1,000 bytes (Decimal system, preferred in commercial and in the most recent specifications).
KB = 1,024 bytes (Binary system, more traditional in computer science).

But the calculations do not work:
189.48 KB × 1,024 = 194,355.72 bytes
194,355.72 bytes / 1,000 = 194.36 kB

194,360 bytes (webstorm size) − 194,030 bytes = 330 bytes

Anyway, an important point in my opinion is the convention of whether to use a dot or a comma to define decimals.

Webstorm uses a comma, VScode uses a dot from what I see.
I think this is an important thing to define correctly.

To understand where best to put this information I think you need to ask @danilo-leal.

Personally I don't like the file path being visible, if it were possible from the settings I would remove it from mine.

@kaf-lamed-beyt : If you can show this information for the images too that would be great.

kaf-lamed-beyt · 2024-12-07T14:11:55Z

Oh! Now I understand the reason for the disparity in file size. So what do you recommend we use in this case @Angelk90? the binary or decimal system?

We can decide to accommodate both of them though. If that's okay

@kaf-lamed-beyt : If you can show this information for the images too that would be great.

cool I'll include this too.

kaf-lamed-beyt · 2024-12-07T14:41:38Z

@danilo-leal, hi 👋🏼

any idea how I can include the image metadata in the status bar?

Angelk90 · 2024-12-07T16:23:56Z

@kaf-lamed-beyt :
I would let the user choose which unit of measurement he prefers, for example in the default settings I would create a key called I don't know for example:

https://github.com/zed-industries/zed/blob/main/assets/settings/default.json

{
  "system_type": "binary"
}

default binary, another possible choice decimal, If a value other than the two or incorrect is defined, use the default one

{
  "decimal_separator": "dot", // dot or comma
}

Same thing as above.

example code:

fn format_size(size: u64, base: f64, units: &[&str], separator: char) -> String {
    let mut size_in_units = size as f64;
    let mut unit_index = 0;

    while size_in_units >= base && unit_index < units.len() - 1 {
        size_in_units /= base;
        unit_index += 1;
    }

    let formatted_size = format!("{:.2}", size_in_units);
    
    let formatted_size = match separator {
        ',' => formatted_size.replace('.', ","),
        _ => formatted_size
    };

    format!("{} {}", formatted_size, units[unit_index])
}

fn format_size_decimal(size: u64, separator: char) -> String {
    let units = ["B", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"];
    format_size(size, 1000.0, &units, separator)
}

fn format_size_binary(size: u64, separator: char) -> String {
    let units = ["B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"];
    format_size(size, 1024.0, &units, separator)
}

We'll have to see what the maintainers think of these choices.

jansol · 2024-12-07T17:58:29Z

That was fast! Nice work so far!

Anyway, an important point in my opinion is the convention of whether to use a dot or a comma to define decimals.

~~The decimal separator is defined by the user's locale. We should not try to be clever about it.~~ nvm apparently format! does not do this

any idea how I can include the image metadata in the status bar?

Don't. That won't work well if someone opens multiple images side by side. Also there is very limited space in the status bar and I would very much like to see a lot more metadata (file format; color format - channels, bit depth, subsampling, etc; physical size/DPI; comment; whether the file has color profile; ...)

As far as I've seen, most applications tend to put metadata in an overlay or a side bar. I think @iamnbutler is the one that should be consulted on that, as he is the main designer in the Zed team AIUI. You might want to arrange a collab call with him to decide on the UI design portion of this.

Angelk90 · 2024-12-07T18:12:51Z

@jansol :

The decimal separator is defined by the user's locale.

Where and how?
In the settings where is this setting found?

That won't work well if someone opens multiple images side by side.

Depending on which image the tab focus is on, information is displayed just like vscode does.

However, I agree with the choice of having a side menu for file information, as xcode does.

kaf-lamed-beyt · 2024-12-07T18:57:33Z

That was fast! Nice work so far!

Thank you, @jansol!

Don't. That won't work well if someone opens multiple images side by side. Also there is very limited space in the status bar and I would very much like to see a lot more metadata (file format; color format - channels, bit depth, subsampling, etc; physical size/DPI; comment; whether the file has color profile; ...)

Oh! That's good then. This is a valid reason.

I'd check if the image crate can provide all these metadata you mentioned.

What's the best way to reach @iamnbutler? Email? Twitter? Discord (I may need the discord handle)

mikayla-maki

Personally, I prefer the VSCode style with the size and dimensions in the status bar. You can define a new status bar item in the image_viewer crate, and then add it after the cursor position indicator here:

zed/crates/zed/src/zed.rs

Lines 214 to 222 in d7ea3dd

    
           workspace.status_bar().update(cx, |status_bar, cx| { 
        
               status_bar.add_left_item(diagnostic_summary, cx); 
        
               status_bar.add_left_item(activity_indicator, cx); 
        
               status_bar.add_right_item(inline_completion_button, cx); 
        
               status_bar.add_right_item(active_buffer_language, cx); 
        
                           status_bar.add_right_item(active_toolchain_language, cx); 
        
               status_bar.add_right_item(vim_mode_indicator, cx); 
        
               status_bar.add_right_item(cursor_position, cx); 
        
           });

mikayla-maki · 2024-12-13T00:25:18Z

crates/image_viewer/Cargo.toml

@@ -23,3 +23,4 @@ theme.workspace = true
 ui.workspace = true
 util.workspace = true
 workspace.workspace = true
+image = "0.25.5"


Let's elevate this dependency, and the one in GPUI, to a workspace dependency

crates/image_viewer/src/image_viewer.rs

Angelk90 · 2024-12-13T09:34:07Z

I would very much like to see a lot more metadata (file format; color format - channels, bit depth, subsampling, etc; physical size/DPI; comment; whether the file has color profile; ...)

@mikayla-maki : what @jansol proposed cannot be done.

Let's see what @iamnbutler says.

kaf-lamed-beyt · 2024-12-14T23:43:43Z

@mikayla-maki am i on the right path?

I've managed to get a UI element to render in the status bar. But, I do not think the load_metadata fn here works as it should. I even tried logging to the console so I could see if the data was retrieved or not.

zed/crates/project/src/image_store.rs

Lines 135 to 157 in b27afc4

    
           pub async fn load_metadata<F: Fs>( 
        
               &mut self, 
        
               project: &Project, 
        
               cx: &AppContext, 
        
               fs: &F, 
        
           ) -> Result<(), String> { 
        
               println!("Loading metadata for image: {:?}", self.path()); 
        
               match Self::try_open(self, project, cx, fs).await { 
        
                   Ok(metadata) => { 
        
                       self.width = Some(metadata.0); 
        
                       self.height = Some(metadata.1); 
        
                       self.file_size = Some(metadata.2); 
        
                       self.color_type = Some(metadata.3); 
        
                       println!("Metadata loaded: {:?}", metadata); 
        
                       Ok(()) 
        
                   } 
        
                   Err(err) => { 
        
                       println!("Failed to load metadata: {}", err); 
        
                       Err(err) 
        
                   } 
        
               } 
        
           }

Perhaps, you could point me in the right direction?

iamnbutler · 2024-12-17T14:08:41Z

Hey, sorry I'm terrible at keeping up with activity here.

My first instinct is that the bottom right of the toolbar should be used to show this metadata.

In buffers, that is cursor position, selected lines, characters, etc. So for images it would make sense for it to show file size, dimensions, etc.

There is the downside that @jansol mentions, that you can't do comparisons in this way.

I wouldn't be opposed to us exploring other options such as overlays, but I would not put this info in the toolbar, as it is intended for controls.

If we had sub-splits already (in-buffer sidebars) I would suggest adding a i icon that opens a panel in the buffer where we can show information, similar to finder:

kaf-lamed-beyt · 2024-12-17T16:50:41Z

Thanks for the feedback @iamnbutler! The approach I'm taking is in line with what you mentioned here

My first instinct is that the bottom right of the toolbar should be used to show this metadata.
In buffers, that is cursor position, selected lines, characters, etc. So for images it would make sense for it to show file size, dimensions, etc.

But, I have difficulty getting the image metadata to show up there. I successfully got a UI element to render in the status(tool)bar:a placeholder.

So any help, reviewing what I'm doing wrong, would be very much appreciated.

If we decide to have a sub-split like you the image description you shared, where or which crates should I be focusing on for this implementation?

This is my first "real" rodeo with Rust, so you'll please excuse my questions 👐🏼. Thanks!

mikayla-maki · 2024-12-22T10:06:34Z

But, I have difficulty getting the image metadata to show up there.

You'll have to figure it out if you want to ship this PR :). I'd suggest looking at the cursor position UI, and learning how it's hooked into the rest of Zed. The key trait you'll want to understand is StatusItemView, and you can see how it's implemented for the CursorPosition struct here:

zed/crates/go_to_line/src/cursor_position.rs

Lines 212 to 232 in 2ff06ce

    
           impl StatusItemView for CursorPosition { 
        
               fn set_active_pane_item( 
        
                   &mut self, 
        
                   active_pane_item: Option<&dyn ItemHandle>, 
        
                   cx: &mut ViewContext<Self>, 
        
               ) { 
        
                   if let Some(editor) = active_pane_item.and_then(|item| item.act_as::<Editor>(cx)) { 
        
                       self._observe_active_editor = 
        
                           Some(cx.observe(&editor, |cursor_position, editor, cx| { 
        
                               Self::update_position(cursor_position, editor, Some(UPDATE_DEBOUNCE), cx) 
        
                           })); 
        
                       self.update_position(editor, None, cx); 
        
                   } else { 
        
                       self.position = None; 
        
                       self._observe_active_editor = None; 
        
                   } 
        
                   cx.notify(); 
        
               } 
        
           }

I'm going to mark this as a draft for now, feel free to mark it as ready once you have a new implementation utilizing the status bar working.

Good luck and happy holidays!

kaf-lamed-beyt · 2024-12-22T11:06:13Z

Thank you @mikayla-maki! I've been going through the CursorPosition implementation, actually.

But, I'll go with your suggestion and approach everything carefully now.

I'll make sure to mark this PR as ready when I'm set. Happy holidays!

The standard fs crate does filesystem operations synchronously which may lead to blocking the main thread when trying to access the image metadata, specifically fs.metadata to obtain the file size. This PR addresses that issue, and also renders other image information like the dimensions (width & height), and the image type (PNG|JPG etc)

kaf-lamed-beyt · 2024-12-29T15:12:41Z

@mikayla-maki, kindly take a look when you get the chance. Thanks!

Angelk90 · 2024-12-30T17:53:48Z

@kaf-lamed-beyt : There seems to be too much space between the text and the copilot icon and I would then add a separation as seen in the second image.

For units of measurement some prefer 1024 others 1000, I would let the user choose which one to use via settings, I would set the default to 1024.

kaf-lamed-beyt · 2024-12-30T18:08:50Z

Oh! I'll look into the spacing issue and that divider in the UI too.

I'd take a look at the settings crate to see how I can make the file meta unit customizable.

kaf-lamed-beyt · 2024-12-31T12:40:36Z

People should now be able to set their preferred unit types.

The image below uses the "decimal" unit
. You can compare it with the previous screenshot I attached.

Here's how:

For the spacing issue @Angelk90, it is there by default.

See an example of what it looks like in the current Zed version I have on my machine. The space between the CursorPosition and the language buffer is pretty much the same as what's in my first screenshot.

For the vertical divider... I couldn't find how it was implemented.

…image sizes

mikayla-maki

Making a lot of progress! There's a couple things that need to be re-arranged though. You'll also need to get CI passing before we can merge this :)

That said, the feature itself is looking really good! Excited to have this.

crates/image_viewer/src/image_info.rs

crates/project/src/image_store.rs

crates/image_viewer/src/image_info.rs

crates/project/src/image_store.rs

mikayla-maki · 2025-01-03T01:06:21Z

crates/project/src/image_store.rs

+                if let Ok(()) = image_model.update(&mut cx, |image, cx| {
+                    let project_ref = project_clone.read(cx);
+
+                    if let Ok(metadata) = futures::executor::block_on(image.load_metadata(


This shouldn't be blocked on. Rather, return it out of the .update(), then .await it, then call update() again to assign the results.

oh! thanks, i'll try this now

@mikayla-maki, I did some cleanups. Kindly review and let me know what you think.

The only issue I have is in this block. When I try to do it like you said — regarding this executor::block_on — I run into Rust lifetime issues several times.

Perhaps a little nudge in the right direction? thanks!

mikayla-maki · 2025-01-03T01:07:03Z

crates/settings/src/settings.rs

@@ -66,6 +66,8 @@ pub fn init(cx: &mut AppContext) {
        .set_default_settings(&default_settings(), cx)
        .unwrap();
    cx.set_global(settings);
+
+    ImageFileSizeUnitType::register(cx);


This shouldn't be registered here. This should be in an init() function in the image_info file.

So for this scenario... i tried having an exact init function in ImageItem But I kept on running into an error. This one specifically.

Thread "main" panicked with "unregistered setting type image_viewer::image_info::ImageFileSizeUnitType" at /oss/zed/crates/settings/src/settings_store.rs:341:32 0: zed::reliability::init_panic_hook::{{closure}}

So to avoid it, I ended up using call_once from std::sync like this 👇🏼 so it doesn't call ::register() everytime the statusItem mounts

pub fn new(_workspace: &Workspace, cx: &mut AppContext) -> Self { static INIT: std::sync::Once = std::sync::Once::new(); INIT.call_once(|| { ImageFileSizeUnitType::register(cx); });

Lemme know if this is okay.

crates/settings/src/settings_store.rs

kaf-lamed-beyt · 2025-01-03T06:59:25Z

Making a lot of progress! There's a couple things that need to be re-arranged though. You'll also need to get CI passing before we can merge this :)

That said, the feature itself is looking really good! Excited to have this.

Ayy! I'm happy to hear this @mikayla-maki!

I'll go over the changes you've suggested and push tthem

- create a standalaone type (ImageItemMeta) to hold image metadata - add smol for async I/O actions when retrieving image properties - move ImageFileSizeUnitType into image_info.rs - used a call_once method from std::sync to register file size unit setting

… worktree-abspath

mikayla-maki · 2025-01-04T00:08:58Z

Awesome! I'll mark this as draft for now. Let me know when it's next ready for review :)

fix block on

kaf-lamed-beyt · 2025-01-15T11:43:51Z

@mikayla-maki, can you take a look now?

kaf-lamed-beyt added 2 commits December 7, 2024 03:49

chore: show image info

392e6f8

Merge branch 'main' of github.com:kaf-lamed-beyt/zed

5874639

cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 7, 2024

danilo-leal changed the title ~~chore: show image information~~ Add image dimension and file size information Dec 7, 2024

mikayla-maki assigned iamnbutler and mikayla-maki Dec 13, 2024

mikayla-maki requested changes Dec 13, 2024

View reviewed changes

kaf-lamed-beyt added 2 commits December 15, 2024 00:36

implement image_info StatusItemView

da13757

minor

b27afc4

mikayla-maki marked this pull request as draft December 22, 2024 10:06

Merge branch 'main' into worktree-abspath

c221619

kaf-lamed-beyt marked this pull request as ready for review December 30, 2024 07:29

kaf-lamed-beyt added 2 commits December 31, 2024 13:54

feat: allow users to chose their preferred units of measurements for …

5511e2f

…image sizes

Merge branch 'main' into worktree-abspath

dc1eb2f

mikayla-maki requested changes Jan 3, 2025

View reviewed changes

kaf-lamed-beyt and others added 3 commits January 3, 2025 19:57

refactor

1a6229f

- create a standalaone type (ImageItemMeta) to hold image metadata - add smol for async I/O actions when retrieving image properties - move ImageFileSizeUnitType into image_info.rs - used a call_once method from std::sync to register file size unit setting

Merge branch 'main' into worktree-abspath

9d65395

Merge branch 'worktree-abspath' of github.com:kaf-lamed-beyt/zed into…

a0dcce0

… worktree-abspath

mikayla-maki marked this pull request as draft January 4, 2025 00:09

kaf-lamed-beyt and others added 5 commits January 8, 2025 12:37

get fs from project instead of creating a new pointer

ad8627e

update cargo lock

00f076b

Merge branch 'main' into worktree-abspath

aceb577

fix block on

120732f

Merge pull request #1 from 0xtimsb/fix-block-on

7805335

fix block on

kaf-lamed-beyt marked this pull request as ready for review January 15, 2025 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image dimension and file size information #21675

Add image dimension and file size information #21675

kaf-lamed-beyt commented Dec 7, 2024 •

edited by danilo-leal

Loading

cla-bot bot commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

cla-bot bot commented Dec 7, 2024

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024 •

edited

Loading

kaf-lamed-beyt commented Dec 7, 2024

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

Angelk90 commented Dec 7, 2024 •

edited

Loading

jansol commented Dec 7, 2024 •

edited

Loading

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

mikayla-maki left a comment

mikayla-maki Dec 13, 2024

Angelk90 commented Dec 13, 2024

kaf-lamed-beyt commented Dec 14, 2024

iamnbutler commented Dec 17, 2024

kaf-lamed-beyt commented Dec 17, 2024

mikayla-maki commented Dec 22, 2024 •

edited

Loading

kaf-lamed-beyt commented Dec 22, 2024

kaf-lamed-beyt commented Dec 29, 2024

Angelk90 commented Dec 30, 2024

kaf-lamed-beyt commented Dec 30, 2024 •

edited

Loading

kaf-lamed-beyt commented Dec 31, 2024 •

edited

Loading

mikayla-maki left a comment •

edited

Loading

mikayla-maki Jan 3, 2025

kaf-lamed-beyt Jan 3, 2025

kaf-lamed-beyt Jan 11, 2025 •

edited

Loading

mikayla-maki Jan 3, 2025

kaf-lamed-beyt Jan 3, 2025 •

edited

Loading

kaf-lamed-beyt commented Jan 3, 2025

mikayla-maki commented Jan 4, 2025

kaf-lamed-beyt commented Jan 15, 2025

	workspace.status_bar().update(cx, \|status_bar, cx\| {
	status_bar.add_left_item(diagnostic_summary, cx);
	status_bar.add_left_item(activity_indicator, cx);
	status_bar.add_right_item(inline_completion_button, cx);
	status_bar.add_right_item(active_buffer_language, cx);
	status_bar.add_right_item(active_toolchain_language, cx);
	status_bar.add_right_item(vim_mode_indicator, cx);
	status_bar.add_right_item(cursor_position, cx);
	});

Add image dimension and file size information #21675

Are you sure you want to change the base?

Add image dimension and file size information #21675

Conversation

kaf-lamed-beyt commented Dec 7, 2024 • edited by danilo-leal Loading

cla-bot bot commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

cla-bot bot commented Dec 7, 2024

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024 • edited Loading

kaf-lamed-beyt commented Dec 7, 2024

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

Angelk90 commented Dec 7, 2024 • edited Loading

jansol commented Dec 7, 2024 • edited Loading

Angelk90 commented Dec 7, 2024

kaf-lamed-beyt commented Dec 7, 2024

mikayla-maki left a comment

Choose a reason for hiding this comment

mikayla-maki Dec 13, 2024

Choose a reason for hiding this comment

Angelk90 commented Dec 13, 2024

kaf-lamed-beyt commented Dec 14, 2024

iamnbutler commented Dec 17, 2024

kaf-lamed-beyt commented Dec 17, 2024

mikayla-maki commented Dec 22, 2024 • edited Loading

kaf-lamed-beyt commented Dec 22, 2024

kaf-lamed-beyt commented Dec 29, 2024

Angelk90 commented Dec 30, 2024

kaf-lamed-beyt commented Dec 30, 2024 • edited Loading

kaf-lamed-beyt commented Dec 31, 2024 • edited Loading

mikayla-maki left a comment • edited Loading

Choose a reason for hiding this comment

mikayla-maki Jan 3, 2025

Choose a reason for hiding this comment

kaf-lamed-beyt Jan 3, 2025

Choose a reason for hiding this comment

kaf-lamed-beyt Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

mikayla-maki Jan 3, 2025

Choose a reason for hiding this comment

kaf-lamed-beyt Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

kaf-lamed-beyt commented Jan 3, 2025

mikayla-maki commented Jan 4, 2025

kaf-lamed-beyt commented Jan 15, 2025

kaf-lamed-beyt commented Dec 7, 2024 •

edited by danilo-leal

Loading

kaf-lamed-beyt commented Dec 7, 2024 •

edited

Loading

Angelk90 commented Dec 7, 2024 •

edited

Loading

jansol commented Dec 7, 2024 •

edited

Loading

mikayla-maki commented Dec 22, 2024 •

edited

Loading

kaf-lamed-beyt commented Dec 30, 2024 •

edited

Loading

kaf-lamed-beyt commented Dec 31, 2024 •

edited

Loading

mikayla-maki left a comment •

edited

Loading

kaf-lamed-beyt Jan 11, 2025 •

edited

Loading

kaf-lamed-beyt Jan 3, 2025 •

edited

Loading