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'.

@cla-bot check

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

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

1. I would remove the Dimension: and Image size:
2. 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:

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?

Here's what i have

@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.

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.

@danilo-leal, hi 👋🏼

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

@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.

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.

@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.

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)

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.

@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.

Perhaps, you could point me in the right direction?

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:

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!

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:

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!

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!

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

@kaf-lamed-beyt Just tried this locally. Looks good!

• I'd like to make this not a button, but I think it is easier for me to do that after this merges, as I should probably ship a variant of ButtonLike that matches the style of a button but is none-interactive (so the style stays consistent.) So no change here for now.

• I'm not too familiar with how GIFs work, but is it correct that we should see PNG (32-bit color) with a GIF open (maybe this refers to the format of each frame?)

Thanks for contributing, and sorry for the delay! Nothing blocking on the design side for this to merge.

Thanks @iamnbutler!

I'd like to make this not a button, but I think it is easier for me to do that after this merges, as I should probably ship a variant of ButtonLike that matches the style of a button but is none-interactive (so the style stays consistent.) So no change here for now.

This is great! shipping a different variant of Button. I used it like so intending to add a tooltip — during the early stage of the PR instead of putting the info in a div.

I'm not too familiar with how GIFs work, but is it correct that we should see PNG (32-bit color) with a GIF open (maybe this refers to the format of each frame?)

And yes, you're right about this. Showing a format-specific description like PNG for a GIF for the color type Rgba8 and Rgba16 is misleading. GIFs can also fall within this color type, so inferring that it is PNG isn't good. My bad.

Instead, we can leave it as RGBA for both. Thanks for the detailed breakdown, @jansol!

I pushed a commit addressing this now

Ok great! No blockers from me to merge this 👍

Instead, we can leave it as RGBA for both.

That's still problematic:

• a single-channel image can be "Grayscale" or "Luminance" or "Red" which are supposed to be treated differently
• a two-channel image could be "grayscale with alpha".... or it could be "red and green" (commonly used for non-color data e.g. in games)
• three channels could be RGB or YUV
• four channels could be RGBA, YUV with alpha or CMYK.
• there can be more channels than four (see "Deep Pixels" in OpenEXR)

(YUV is commonly used in video compression and occasionally still frames taken from videos may be kept in this format. JPEG files with CMYK color are used for professional printing. The Windows Imaging Component documentation has a detailed summary.)

ColorType is not a suitable data source since it silently expands 1, 2 and 4 bit formats to 8 bits. ExtendedColorType keeps the number of bits and adds an extra case for 8bpc CMYK and 8bpc alpha masks but still loses information about other types. (it also can't handle >4 channel images but I don't know if image-rs can even read such images for now)

TL;DR: the only thing image-rs somewhat reliably tells us is the number of bits per pixel and the number of channels, so that's what I'd display in the status item, rather than claiming incorrect things about the meanings of the channels in an image.

I see.

So now, if I get you correctly, @jansol, you're suggesting we switch to ExtendedColorType instead of ColorType. am I right?

Then, I'll have something like this: GIF - 4 channels, 32 bits per pixel?

See the attached image.

My concern now is how do we make this descriptive enough ("grayscale" and whatnot)? I may be wrong, but I doubt many people would grasp this at first.

Then, I'll have something like this: GIF - 4 channels, 32 bits per pixel?

Precisely.

My concern now is how do we make this descriptive enough ("grayscale" and whatnot)? I may be wrong, but I doubt many people would grasp this at first.

For now, not at all. If people aren't familiar with the concept of image channels making them look it up once is a much smaller problem in my book than making people who do understand it double-check every single image with some other tool because they can't trust what Zed tells them.

In the longer term I'd love to see image-rs to add a proper file metadata API so we can also know stuff like whether the image is encoded in sRGB or DCI-P3 or AdobeRGB or whatnot. But we should under no circumstances make up information that we don't have.

That said, ExtendedColorType does have special cases for CMYK (Cmyk8) and 8-bit alpha masks (A8) that could potentially be used to provide an extra bit of info for those cases with reasonable confidence that it'll be correct, but for others I wouldn't since that simply won't be reliable.

FWIW Here's what file and ffprobe respectively say about a random PNG I have lying around in my home directory:

file: PNG image data, 926 x 871, 8-bit/color RGB, non-interlaced
ffprobe: png, rgb24(pc, gbr/unknown/unknown), 926x871

And a JPEG right next to it:

file: JPEG image data, JFIF standard 1.01, resolution (DPI), density 72x72, segment length 16, progressive, precision 8, 415x424, components 3
ffprobe: mjpeg (Progressive), yuvj444p(pc, bt470bg/unknown/unknown), 415x424

As you can see, "RGB" is misleading here, even though both are in fact color images with 3 channels and image-rs does silently convert both to RGB after decoding them. But the point of the info view is to show information about the file on disk and not about what Zed is rendering to the screen.

Yes, it is indeed misleading @jansol! thanks again for taking the time to elaborate on the concept behind the image channels/color type.

I've also included the image format now.

@jansol :
Can you create a repository with only test images, that you consider suitable and that cover all the fields?
To understand how Webstorm behaves.

I would put all this information in a left side menu that can be opened and closed, like xcode does.

CI passes now.
cc: @mikayla-maki, @iamnbutler

hey @kaf-lamed-beyt, silly question but can you confirm after your latest changes the image information still shows up for you? I don't see it anymore.

Oh! Lemme do that now.

so, on startup, when I opened it, there was an image currently open, the image info didn't show up.

but, when I selected another one, it showed the image info. any idea what could be causing this @iamnbutler? Is there a way to ensure metadata loads on startup

I haven't looked through your implementation yet, but @mikayla-maki might be better at giving feedback here.

From what I can see, any image that was already open doesn't get image info until it has been closed and re-opened.

So my naive assumption is some event isn't being handled (workspace opened/updated?) or something is happening when the buffer opens, and you need to check for any open buffers after the workspace loads.

I can take a look after my meetings today if someone else doesn't get to it!

Yes. You're right. It is related to an image event not being called. Perhaps, I could try adding a new value MetadataUpdated or somn' in the ImageItemEvent enum.

I'll try tinkering and let you know what I find.

@iamnbutler, I tried the ImageItemEvent approach, but it didn't yield positive results.

After a while, I realized it was pretty much the same thing that goes into the ProjectItem implementation and since the new method in ImageView is responsible for image loading on initial render, I called ImageItem::image_info in that block.

It works. But, I don't know if it is optimal. Please take a look, thanks!