Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Blurred image is grayed out, despite the input not being so #8

Closed
RealKC opened this issue Jul 6, 2022 · 17 comments
Closed

Blurred image is grayed out, despite the input not being so #8

RealKC opened this issue Jul 6, 2022 · 17 comments

Comments

@RealKC
Copy link

RealKC commented Jul 6, 2022

So I'm using your crate in my app and hitting an issue while trying to blur a gdk_pixbuf::Pixbufs, that is for certain images the blurred result is grayed out. I believe that it is related to the size of the image, but I'm not entirely too sure. This is an example that exhibits this issue

use gdk_pixbuf::{Colorspace, Pixbuf};
use stackblur_iter::{blur_argb, imgref::ImgRefMut};

fn main() {
    let pixbuf = Pixbuf::from_file("unknown.png").unwrap();
    assert!(pixbuf.n_channels() == 3); // This assertion exists because the Pixbuf I'm blurring in my program should always lack an alpha channel

    let pixels = pixbuf.pixel_bytes().unwrap().to_vec();
    let width = pixbuf.width() as usize;
    let height = pixbuf.height() as usize;

    let mut pixels = match pixbuf.n_channels() {
        3 => blur_rgb(pixels, width, height, 5),
        _ => unreachable!(),
    };

    Pixbuf::from_mut_slice(
        &mut pixels,
        Colorspace::Rgb,
        false,
        8,
        pixbuf.width(),
        pixbuf.height(),
        pixbuf.rowstride(),
    )
    .savev("blurred.png", "png", &[])
    .unwrap();
}

fn blur_rgb(pixels: Vec<u8>, width: usize, height: usize, radius: usize) -> Vec<u8> {
    assert!(
        pixels.len() % 3 == 0,
        "The pixel buffer's length should be a multiple of 3, but it was '{}'.",
        pixels.len()
    );

    let mut pixels = pixels
        .chunks_exact(3)
        .map(|chunk| {
            let red = chunk[0];
            let green = chunk[1];
            let blue = chunk[2];

            u32::from_be_bytes([0xff, red, green, blue])
        })
        .collect::<Vec<_>>();

    let mut img = ImgRefMut::new(&mut pixels, width, height);

    blur_argb(&mut img, radius);

    pixels
        .into_iter()
        .flat_map(|pixel| {
            let [_, r, g, b] = pixel.to_be_bytes();

            [r, g, b]
        })
        .collect()
}
An example image and the result after the above program is ran

original image

becomes

blurred

ran through the above program

Cargo.toml for the above program
[package]
name = "stackblurtest"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
stackblur-iter = { git = "https://github.com/LoganDark/stackblur-iter/", rev = "53be4ee685f2bc1fbbb79b9ab02ee6679de85694" }
gdk-pixbuf = "0.15.11"

The Pixbuf mentioned above stores its data in an [rgb][rgb][rgb]... format and I'm converting it to ARGB u32s as that's pretty convenient because it allows me to use the blur_argb method provided by your crate and not have to write my own RGB type.

For the record I don't believe this to be an issue with me using 0xff for every alpha byte in the intermediary buffer as I replaced it with a randomly generated byte using the rand crate and the output didn't look like it changed.

@LoganDark
Copy link
Owner

Have you tried using blur_srgb instead? The sRGB EOTF is nonlinear which means blurring the encoded values directly results in some problems.

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

Nevermind. That's not the output of stackblur-iter, there's something going wrong in your pixel conversion code (or something else).

Try using ImgRefMut::new_stride, stride is important

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

also reminds me I'm missing stride checks in imgref-iter, whoops

Edit: fixed

@RealKC
Copy link
Author

RealKC commented Jul 6, 2022

Just tried out your suggestions out, results:

  • using blend_srgb, image is visually distinct but still gray
    blurred

  • passing pixbuf.rowstride() / 3 as the stride parameter to ImgRefMut::new_stride
    blurred
    For the record, the documentation of Pixbuf::rowstride says

Queries the rowstride of a pixbuf, which is the number of bytes between the start of a row and the start of the next row.

so I believe I'm passing the correct value as stride

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

This is weird as hell. Are you sure you're passing the correct parameters to Pixbuf::from_mut_slice? I'm sorry, I just can't imagine this being an issue with stackblur-iter - I'm looking at the latest version right now in my test program:

firefox_puXnUzewHB.mp4

There's certainly nothing that could convert an image to grayscale. Each component is treated totally separately.

@RealKC
Copy link
Author

RealKC commented Jul 6, 2022

Are you sure you're passing the correct parameters to Pixbuf::from_mut_slice?

As far as I can tell yes, gdk_pixbuf only has support for rgb as the colourspace (2nd param), the image doesn't have any alpha (3rd param), and it should have 8 bits per sample (4th param), I added these assertions after assert!(pixbuf.n_channels() == 3):

    assert!(!pixbuf.has_alpha());
    assert!(pixbuf.bits_per_sample() == 8);

and the program continues to run, so I believe those params should be ok, and I believe the width, height, and rowstride should remain unaffected by the blur, so I think those params are fine too.

I replaced the &mut pixels line with &mut dbg!(pixels) and to me the outputted data looks like grays (if I'm interpretting the bytes correctly, groups of 3 tend to have very similar values and I believe that indicates that the pixel will be a shade of gray), this leads me to believe that either the chunks(3) -> u32 conversion is wrong, or that there is an error in the blur algorithm, but I don't believe the conversion is incorrect, since it's just the reverse of

pub const fn from_u32(argb: u32) -> Self {
let [a, r, g, b] = argb.to_be_bytes();
Self([Wrapping(a as u32), Wrapping(r as u32), Wrapping(g as u32), Wrapping(b as u32)])
}

but with a hardcoded to 0xff :S

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

I'll look into it more soon, in the meantime see if removing the blur does anything?

@RealKC
Copy link
Author

RealKC commented Jul 6, 2022

in the meantime see if removing the blur does anything?

As far as I can see the image is visually identical to the original, but I'll also include it as an attachment

blurred

@LoganDark
Copy link
Owner

in the meantime see if removing the blur does anything?

As far as I can see the image is visually identical to the original, but I'll also include it as an attachment

blurred

Okay that's definitely super duper weird, WTF. Going to look into this soon

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

Looks like gdk pixbuf requires some C libraries and pkg-config. Ughhh. This is gonna be a pain

Edit: Sorry, I cannot reproduce this yet. It looks like the only way to get the libraries I need is to install the entirety of GTK, which among other things, requires Python, X11, Perl, and a bunch of different build systems. Yes, those are apparently just the runtime dependencies!! Trying to install it, apt greets me with this:

image

WTF

I'm going to try this in Alpine where I can keep all this shit off my system, sorry for the wait.

@RealKC
Copy link
Author

RealKC commented Jul 6, 2022

sorry for the wait.

No worries

@LoganDark
Copy link
Owner

Yeah okay I dunno wtf is going on but the first line of main just segfaults. I literally can't reproduce your problem because Pixbuf::from_file and Pixbuf::from_read both segfault LOL. Guess I have some issue reporting to do

@LoganDark
Copy link
Owner

LoganDark commented Jul 6, 2022

Nevermind that; random GitHub issue to the rescue. I know what the issue is.

2022-07-06-160951_1453x166_scrot

Because the stride is not a multiple of 3, the chunks_mut is getting thrown off. Because each row has its channels shifted by 1, the vertical part of stackblur essentially averages out the different channels, turning it grayscale.

So it is your conversion code. Or well more accurately it's whatever retarded shit gdk pixbuf is doing.

This version works:

use std::io::Cursor;
use gdk_pixbuf::{Colorspace, Pixbuf};
use stackblur_iter::{blur_argb, imgref::ImgRefMut};

fn main() {
	let pixbuf = Pixbuf::from_read(Cursor::new(include_bytes!("unknown.png"))).unwrap();
	assert_eq!(pixbuf.n_channels(), 3);

	let pixels = pixbuf.pixel_bytes().unwrap().to_vec();
	let width = pixbuf.width() as usize;
	let height = pixbuf.height() as usize;
	let stride = pixbuf.rowstride() as usize;

	let mut pixels = match pixbuf.n_channels() {
		3 => blur_rgb(pixels, width, height, stride, 5),
		_ => unreachable!(),
	};

	Pixbuf::from_mut_slice(
		&mut pixels,
		Colorspace::Rgb,
		false,
		8,
		pixbuf.width(),
		pixbuf.height(),
		pixbuf.width() * 3,
	)
		.savev("blurred.png", "png", &[])
		.unwrap();
}

fn blur_rgb(pixels: Vec<u8>, width: usize, height: usize, stride: usize, radius: usize) -> Vec<u8> {
	assert_eq!(pixels.len() % 3, 0, "The pixel buffer's length should be a multiple of 3");
	//assert_eq!(stride % 3, 0, "The pixel buffer's stride should be a multiple of 3");

	let mut iter = pixels.into_iter();
	let mut pixels = Vec::with_capacity(width * height);

	for _ in 0..height {
		for _ in 0..width {
			pixels.push(u32::from_be_bytes([255, iter.next().unwrap(), iter.next().unwrap(), iter.next().unwrap()]));
		}

		for _ in 0..stride - width * 3 {
			iter.next();
		}
	}

	let mut img = ImgRefMut::new_stride(&mut pixels, width, height, stride / 3);

	blur_argb(&mut img, radius);

	pixels.into_iter().flat_map(|pixel| {
		let [_, r, g, b] = pixel.to_be_bytes();
		[r, g, b]
	}).collect()
}

Keep in mind it was produced in nano (i.e. without a proper IDE) so that's why the formatting is all messed up.

Here's the output:

blurred

With all this in mind, if you inspect the original corrupted blur closely (specifically the one with stride), you can start to see the individual symptoms:

image

@RealKC
Copy link
Author

RealKC commented Jul 7, 2022

Thank you so much!

@LoganDark
Copy link
Owner

LoganDark commented Jul 8, 2022

Thank you so much!

Personally I'd implore you to try things other than pixbuf, it's a huge dynamic library and if picking stride that isn't a multiple of channels isn't a sign of Sus quality, I don't know what is. If you just need something that works with multiple formats I would recommend image

In the future, check your invariants before reporting issues here :) asserting stride is a multiple of 3 would've let you know real fast what the issue is

@RealKC
Copy link
Author

RealKC commented Jul 8, 2022

In the future, check your invariants before reporting issues here :) asserting stride is a multiple of 3 would've let you know real fast what the issue is

honestly it slipped my mind that the stride also had to be a multiple of 3 for that code to work 😅

Personally I'd implore you to try things other than pixbuf, it's a huge dynamic library and if picking stride that isn't a multiple of channels isn't a sign of Sus quality, I don't know what is.

I'm writing a gtk-rs based app so not using pixbuf wouldn't remove the huge dynamic library dependencies I have, though I do think I want to remove pixbuf from this code specifically, but I'll have to understand better how cairo imagesurfaces work.

(for context, I'm writing a screenshot app for Linux because I'm dissatisfied with the existing ones, and decided to use your crate for the blur function of its editor)

@LoganDark
Copy link
Owner

(for context, I'm writing a screenshot app for Linux because I'm dissatisfied with the existing ones, and decided to use your crate for the blur function of its editor)

Will you handle HDR/WCG correctly?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants