Fix preview of images with non srgb icc profile #3358

leiserfg · 2025-11-23T17:44:58Z

Which issue does this PR resolve?

Currently yazi can't correctly preview images that are not srgb, this PRs transforms the colorspace while making the cache entry so they work fine.

Rationale of this PR

It reads the icc of the image, checks if it's not srgb and if that's the case, makes it into srgb before writing to the cache folder.

This is an example of an image that works fine after the change and didn't before:

leiserfg · 2025-11-23T17:46:53Z

By the way, it was a headache to deal with jemalloc in nix-shell, no idea why it does not compile. I had to disable it while making the PR and then enable it again before pushing.
Edit: added another commit to solve for that.

sxyazi · 2025-11-24T00:33:33Z

This change looks reasonable to me, thank you for the patch!

I don't know Nix well, so would like someone else to review the Nix-related change some, would you like to pull it out to a separate PR so it won't block this one?

sxyazi

Nit

sxyazi · 2025-11-24T00:34:05Z

yazi-adapter/src/image.rs

 impl Image {
 	pub async fn precache(src: PathBuf, cache: &Path) -> Result<()> {
-		let (mut img, orientation, icc) = Self::decode_from(src).await?;
+		let (mut img, orientation, icc) = Self::decode_from(src.clone()).await?;


Suggested change

let (mut img, orientation, icc) = Self::decode_from(src.clone()).await?;

let (mut img, orientation, icc) = Self::decode_from(src).await?;

sxyazi · 2025-11-24T00:34:25Z

yazi-adapter/src/image.rs

-				icc.map(|b| encoder.set_icc_profile(b));
-				encoder.encode_image(&img.into_rgb8())?;
+
+				let mut data = img.to_rgb8().into_raw();


Suggested change

let mut data = img.to_rgb8().into_raw();

let mut data = img.into_rgb8().into_raw();

sxyazi · 2025-11-24T00:36:54Z

yazi-adapter/src/image.rs

+				// If ICC profile is present and it's not srgb, perform color transformation
+				if let Some(ref icc_profile) = icc
+					&& let Some(input) = Profile::new_from_slice(icc_profile, false)
+					&& !input.is_sRGB()
+				{
+					let mut output = Profile::new_sRGB();
+					output.precache_output_transform();
+
+					let xfm = Transform::new(&input, &output, qcms::DataType::RGB8, qcms::Intent::default())
+						.context("Failed creating the ICC transformer")?;
+					xfm.apply(&mut data);
+				}


Please pull these out into a new function called apply_profile

leiserfg · 2025-11-24T06:12:39Z

would you like to pull it out to a separate PR so it won't block this one?

The thing is that without this change I can't build in the nix shell, cargo fails to build jemalloc. The change is only to tell jemalloc-sys where to find the pre built one.

sxyazi · 2025-11-24T06:31:51Z

That sounds like a problem specific to your environment since our CI passed the build and other people seem not to have reported similar issues.

CC @XYenon who is the maintainer of Yazi's Nix package - do you think that change makes sense?

leiserfg · 2025-11-24T06:46:01Z

That sounds like a problem specific to your environment since our CI

In CI it uses nix build which does work fine cause it's replacing rust-jemalloc-sys with the one ni nixpkgs, same thing I did here for nix-shell. Still if you don't like it I can remove it after you approve the other changes, or move it to another pr.

XYenon · 2025-11-24T09:06:37Z

By the way, it was a headache to deal with jemalloc in nix-shell, no idea why it does not compile. I had to disable it while making the PR and then enable it again before pushing.

What error message occurred on which platform?

leiserfg · 2025-11-24T09:16:27Z

By the way, it was a headache to deal with jemalloc in nix-shell, no idea why it does not compile. I had to disable it while making the PR and then enable it again before pushing.

What error message occurred on which platform?

In nix develop / nix-shell / direnv, cargo build triggers a compilation of jemalloc and that breaks (the configure step cant figure out the type of stderr_r). I don't think solving that in this project is worthy given that nixpkgs ships a jemalloc pre-built.

I'm in nixos btw.

XYenon · 2025-11-24T10:30:39Z

I found the issue because shell.nix references yazi.nix, and yazi.nix is a wrapper created by directly referencing the path of the yazi-unwrapped package. Nix cannot infer the dependencies that should be introduced by the yazi-unwrapped package through the wrapper. I will submit a fix shortly.

leiserfg · 2025-11-24T11:29:30Z

Ok, then I will rebase the PR and remove the commit for the jemalloc thingy as soon as the one from @XYenon gets merged.

sxyazi · 2025-11-25T01:37:23Z

yazi-adapter/src/image.rs

 				let rgba = img.into_rgba8();
 				let mut encoder = PngEncoder::new(&mut buf);
 				icc.map(|b| encoder.set_icc_profile(b));
 				encoder.write_image(&rgba, rgba.width(), rgba.height(), ExtendedColorType::Rgba8)?;


Should the same process of sRGB to RGB(A) conversion be applied to the PNG encoder as well for transparent images?

I didn't do it there as I don't know of a case where that affects images with alpha, therefore I don't have how to test it.

Ok, I think I found some, I'll give it a try.

sxyazi · 2025-11-25T01:47:03Z

yazi-adapter/src/image.rs

+				let width = img.width();
+				let height = img.height();
+				let mut data = img.into_rgb8().into_raw();
+				Self::apply_profile(icc, &mut data)?;


I noticed that apply_profile only applied to the precache function, which means it will only work if the user doesn't disable the image preloader and the image has generated a cache at the time, which is problematic as image preview should behave the same way regardless of whether it has cached, i.e. it should also be implemented for downscale which is directlly called by image_show (instead of image_precache), maybe this process should placed under decode_from which precache and downscale both call

leiserfg · 2025-11-25T11:12:55Z

I don't like the _decode name either, so let me know if you have any alternative.

leiserfg force-pushed the push-utowrzwrmqtq branch from c506e93 to db3a79e Compare November 23, 2025 20:08

sxyazi requested changes Nov 24, 2025

View reviewed changes

leiserfg force-pushed the push-utowrzwrmqtq branch from db3a79e to f65ce08 Compare November 24, 2025 07:10

sxyazi force-pushed the main branch 2 times, most recently from 41034d0 to 449450d Compare November 24, 2025 07:23

leiserfg force-pushed the push-utowrzwrmqtq branch from f65ce08 to 66464e6 Compare November 24, 2025 08:03

XYenon mentioned this pull request Nov 24, 2025

fix(nix): shell env #3363

Merged

sxyazi closed this in #3363 Nov 24, 2025

sxyazi reopened this Nov 24, 2025

leiserfg force-pushed the push-utowrzwrmqtq branch from 66464e6 to 0b88073 Compare November 24, 2025 14:05

sxyazi reviewed Nov 25, 2025

View reviewed changes

sxyazi requested changes Nov 25, 2025

View reviewed changes

leiserfg force-pushed the push-utowrzwrmqtq branch from 0b88073 to 81e218d Compare November 25, 2025 11:11

Fix preview of images with non srgb icc profile

ae5e3fc

leiserfg force-pushed the push-utowrzwrmqtq branch from 81e218d to ae5e3fc Compare November 25, 2025 20:29

	let (mut img, orientation, icc) = Self::decode_from(src.clone()).await?;
	let (mut img, orientation, icc) = Self::decode_from(src).await?;

	let mut data = img.to_rgb8().into_raw();
	let mut data = img.into_rgb8().into_raw();

Fix preview of images with non srgb icc profile #3358

Are you sure you want to change the base?

Fix preview of images with non srgb icc profile #3358

Conversation

leiserfg commented Nov 23, 2025

Which issue does this PR resolve?

Rationale of this PR

Uh oh!

leiserfg commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sxyazi commented Nov 24, 2025

Uh oh!

sxyazi left a comment

Choose a reason for hiding this comment

Uh oh!

sxyazi Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

sxyazi Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

sxyazi Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

leiserfg commented Nov 24, 2025

Uh oh!

sxyazi commented Nov 24, 2025

Uh oh!

leiserfg commented Nov 24, 2025

Uh oh!

XYenon commented Nov 24, 2025

Uh oh!

leiserfg commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XYenon commented Nov 24, 2025

Uh oh!

leiserfg commented Nov 24, 2025

Uh oh!

sxyazi Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

leiserfg Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

leiserfg Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

sxyazi Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leiserfg commented Nov 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leiserfg commented Nov 23, 2025 •

edited

Loading

leiserfg commented Nov 24, 2025 •

edited

Loading

sxyazi Nov 25, 2025 •

edited

Loading