Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Apr 22 15:47
    Jacherr opened #1466
  • Apr 21 18:15
    schanur closed #1465
  • Apr 21 18:15
    schanur commented #1465
  • Apr 21 17:14
    aschampion commented #1465
  • Apr 21 17:11
    schanur opened #1465
  • Apr 14 15:14
    fintelia commented #1464
  • Apr 14 14:16
    alexkirsz opened #1464
  • Apr 14 00:56
    CensoredUsername unlabeled #1463
  • Apr 14 00:56
    CensoredUsername labeled #1463
  • Apr 14 00:13
    CensoredUsername opened #1463
  • Apr 12 23:38
    CensoredUsername commented #1060
  • Apr 12 13:43
    HeroicKatora commented #1462
  • Apr 12 13:40
    HeroicKatora edited #1462
  • Apr 12 13:39
    HeroicKatora reopened #1462
  • Apr 12 13:33
    cbrownsey closed #1462
  • Apr 12 13:21
    cbrownsey edited #1462
  • Apr 12 13:14
    cbrownsey opened #1462
  • Apr 10 19:18
    fintelia closed #1354
  • Apr 05 14:06
    HeroicKatora synchronize #1448
  • Apr 05 13:45
    HeroicKatora synchronize #1448
HeroicKatora
@HeroicKatora
If it's only the actual slice you're interested in, you can use ImageBuffer as ops::Deref though, which will also get you the raw slice of samples.
arthmis
@arthmis
pub fn box_filter_mut(filter: MeanKernel, image: &mut RgbaImage) {
    let (width, height) = image.dimensions();

    // want the truncated value of this division, hence not using float
    let radius: i32 = filter.size() as i32 / 2;

    let mut new_image: RgbaImage =
        ImageBuffer::from_pixel(width, height, Rgba::from_channels(255, 255, 255, 255));

    // at this point both images have the same dimensions and I can do a 
    // horizontal pass over image and write the output onto new_image
    // blur pixels row wise
    horizontal_blur(radius, &image, &mut new_image);

    // Now I don't want to allocate a new buffer to transpose new_image 
    // so I want to reuse image's buffer to reinterpret as (height, width)
    // instead of (width, height)
    let mut image: RgbaImage = ImageBuffer::from_raw(height, width, *image.as_raw()).unwrap();
    transpose_rgba(&new_image, &mut image);
}
I'm actually having an issue where the borrow checker doesn't allow me to use image's buffer because it's behind a mutable reference. So at this point, if you have advice on how I can do this when taking image as a mutable reference. I worked around this issue already by taking ownership of image. I just wanted to know if I could provide two interfaces like, box_filter_mut and box_filter.
arthmis
@arthmis
Would it be possible to directly set the width and height of imagebuffer? Similar in vein to the unsafe function for Vec set_len
HeroicKatora
@HeroicKatora
You can replace the image with a zero-sized allocation temporarily which allows you to take the buffer and reuse it directly.
let temp = core::mem::replace(image, ImageBuffer::new(0, 0));
let temp: RgbaImage = ImageBuffer::from_raw(height, width, tmp.into_raw()).unwrap();
*image = temp;
This should have practically very little overhead (a few instructions at most).
The problem with resizing is of course the unnecessary unsafety and the unclear semantics. Should it move pixels? Obv. you don't want that for your case but that means that the pixel at (1, 1) doesn't stay there which would be arguably unexpected for changing dimensions. So I'd rather add it only with stronger motivation.
arthmis
@arthmis
Yeah I don't even want to have to add that the image crate. I was actually hoping I could implement a trait that does what I want, but it doesn't allow me access to the private fields of ImageBuffer. Oh well, I'll have to live with only box_filter and return the filtered image
HeroicKatora
@HeroicKatora
How do you need to access the private fields here?
Adding a resizing method and the solution I proposed are orthogonal concerns
arthmis
@arthmis
I'm not necessarily resizing, so to speak. I want to have the image Buffer to swap the width and height values. That's the simplest explanation I can give. Because when I transpose an image into another buffer, the other buffer's height would be the old image's width, and the other buffer's width would be the old image's height. I essentially want a quick and dirty way to tell an image that it's height is now its width and its width is now its height. Nothing changes in regards to the buffer. I was hoping to write a trait that simply swaps the height and width of the ImageBuffer.
arthmis
@arthmis
    /// swaps width and height for when transposing image
    pub fn swap_dimensions(&mut self) {
        let (width, height) = self.dimensions();
        self.width = height;
        self.height = width;
    }

I implement this function for ImageBuffer, in the buffer file, which will swap the dimensions, then in my function:

pub fn box_filter_mut(filter: MeanKernel, mut image: &mut RgbaImage) {
    use crate::matrix_ops::transpose_rgba;
    use image::Pixel;

    let (width, height) = image.dimensions();

    // want the truncated value of this division, hence not using float
    let radius: i32 = filter.size() as i32 / 2;

    let mut new_image: RgbaImage =
        ImageBuffer::from_pixel(width, height, Rgba::from_channels(255, 255, 255, 255));

    // blur pixels row wise
    horizontal_blur(radius, &image, &mut new_image);
    // swap image dimensions to allow transposing new_image into image
    image.swap_dimensions();
    transpose_rgba(&new_image, &mut image);

    // swap new_image dimensions to allow blurring image and writing
    // to new_image
    new_image.swap_dimensions();
    // blur pixels column wise
    horizontal_blur(radius, &image, &mut new_image);

    // swap dimensions again to transpose new image into image for final
    // output
    image.swap_dimensions();
    transpose_rgba(&new_image, &mut image);
}

I'm not necessarily asking for this to be implemented, but I want to clarify what I'm trying to do. The reason why i'm trying to reuse buffers is because I'm compiling this to Webassembly and I'm trying to cut down on allocations. I'm using WeeAlloc as the allocator.

arthmis
@arthmis
    /// sets dimensions of image
    pub fn set_dimensions(&mut self, width: u32, height: u32) -> Result<(), String> {
        if (width * height * <P as Pixel>::CHANNEL_COUNT as u32) as usize > self.data.len() {
            return Err("width and height too large for container".to_string());
        }
        self.width = width;
        self.height = height;
        Ok(())
    }
something like this would be nice too. I don't know how useful this would be to the overall project though.
arthmis
@arthmis
image-rs/image#1273
I feel like the ConvertInplace interface proposed in this issue could even be used to take an image and convert into a transposed image, if that makes sense.
HeroicKatora
@HeroicKatora

It seems like an separate isse. ConvertInplace was, to my understanding of the draft, meant to preserve pixel relation and also work pixel-by-pixel. This enables some parallelism where a true transpose of the image matrix requires a different kind of parallelization strategy. Your implemtation of swap_dimensions of course only perserves semantics together with transpose_rgba. I would recommend going to flat::Flat for this purpose, it gives you full control over all aspects.

The reason why i'm trying to reuse buffers is because I'm compiling this to Webassembly and I'm trying to cut down on allocations. I'm using WeeAlloc as the allocator.

Please look closely at the code here, and you will find that it does not allocate any memory. Yet, it gives you an intermediate owned Vec of the pixel data. You seem to be under the impression that &mut RgbaImage makes this impossible. That is not the case. The Rust type system only requires you to give a temporary replacement, which can be constructed without an allocation by substituting a zero-sized image.

With here I mean this piece from earlier:
let temp = core::mem::replace(image, ImageBuffer::new(0, 0));
let temp: RgbaImage = ImageBuffer::from_raw(height, width, tmp.into_raw()).unwrap();
*image = temp;
arthmis
@arthmis
Yessss, thank you very much. This is exactly what I wanted to do. Thank you for taking the time to answer! Why is it that I'm able to replace the image even though its behind a mutable reference? Also there wasn't any meaningful performance differences using this code, just like you said.
arthmis
@arthmis
I actually took the code and put it into a trait and implemented it for RgbaImage. It performed 15% faster. So that was an unexpected bonus.
Niklas Kunz
@mntns
Hi! I'm also experiencing the regression in image-tiff that @ruffson described on oct. 29. expand_strip() seems to read 0 bytes of the first strip. If the LZW decoder changed for 0.6.0, the first place to get deeper into debugging it would probably the lzw crate?
HeroicKatora
@HeroicKatora
@mntns If you have a reproduction that would be great, yes.
I'm really unsure how the described problem would happen due to a failure of the lzw decoder since the compression of each expand_strip call is independent and does not share decompression state as far as I know. I'd first look at the integration code.
Raphael
@ruffson
Ah yes, its quite bad, many simple geotiff example files cannot be read (any more). I did not have time to dig more into it. Would it help if I provided a tiff image that fails to be parsed by LZWReader::new(...)?
HeroicKatora
@HeroicKatora
If you can upload a failing image to an issue that would help immensely, yes.
Even better if it were one that we can re-distribute as part of the test suite but that is by no means necessary :)
Raphael
@ruffson
Okay, sure, I'll do it later today! :)
Oh while I am here, @HeroicKatora are there any obstacles regarding my geotiff PR?
HeroicKatora
@HeroicKatora
It would be neat if you resolve the conflicting file with a rebase but other than that; Only me missing that this review was still in my backlog ^^
Niklas Kunz
@mntns
@ruffson: Great, if you open an issue, I'll also add my reproduction to it :)
Raphael
@ruffson

It would be neat if you resolve the conflicting file with a rebase but other than that; Only me missing that this review was still in my backlog ^^

Alright, will do!

@ruffson: Great, if you open an issue, I'll also add my reproduction to it :)

Cool, I'll ping you when I did it.

HeroicKatora
@HeroicKatora
Great to hear from you both
Raphael
@ruffson
Oh and another newby question: The Limits struct in the TIFF decoder mod.rs has a private field _non_exhaustive which means I cannot simply create a new struct with Limits{...}, what is the idiomatic way of creating a new Limits struct outside of this module if you want to change e.g. all three default values? Creating a struct with Limits::default() and then changing each field one by one?
HeroicKatora
@HeroicKatora
Yes, that's the idea.
Raphael
@ruffson
Ok thanks
Raphael
@ruffson
Ok here it is @HeroicKatora @mntns image-rs/image-tiff#106
(the image data might take a bit to download because it is on my server with rather bad upload speed at the moment)
HeroicKatora
@HeroicKatora

Interesting. So the examples are all too long?
That's weird but maybe not too bad.. What would happen if we were to truncate?
This may have previously worked incidentally.

while bytes_read < compressed_length && uncompressed.len() < max_uncompressed_length {
    let (len, bytes) = decoder.decode_bytes(&compressed[bytes_read..])?;
    uncompressed.extend_from_slice(bytes);

Since the lzw library decodes symbols word-for-word it's not unlikely that we hit exactly the right length.
The new library decodes fixed size chunks

HeroicKatora
@HeroicKatora
(slight clarification: with examples are too long I mean that the apparent size of uncompressed is larger than max_uncompressed_length)
The Geotiff2 example is also curious since 2570*3 = 7710 so that might be 'corrupt' with the wrong buffer.byte_len?
Raphael
@ruffson

That's weird but maybe not too bad.. What would happen if we were to truncate?

Okay I'll try that later!

Raphael
@ruffson
Sorry I am pretty busy right now, not sure how much time I have for this in the next weeks. What about your example @mntns ?
Niklas Kunz
@mntns
Thanks for the reminder, @ruffson. I just added a large example image with which the error also appears to your GitHub issue. I'll also try looking into it during the upcoming week, but my situation time-wise is similar.
Raphael
@ruffson
Okay, thank you!
Raphael
@ruffson
Thank you for the PR @HeroicKatora I promise that I will spent some time on the weekend to look into this again!
HeroicKatora
@HeroicKatora
I'll patch up the review concerns as well. With regards to color treatment we need to have a separate discussion/implementation in any case.
Danilo Bargen
@dbrgn
Hi, is there a way to convert a GdkPixbuf to an image-rs buffer?

According to the docs: https://developer.gnome.org/gdk-pixbuf/unstable/gdk-pixbuf-The-GdkPixbuf-Structure.html#image-data

Image data in a pixbuf is stored in memory in uncompressed, packed format. Rows in the image are stored top to bottom, and in each row pixels are stored from left to right. There may be padding at the end of a row. The "rowstride" value of a pixbuf, as returned by gdk_pixbuf_get_rowstride(), indicates the number of bytes between rows.

HeroicKatora
@HeroicKatora
Have you found https://docs.rs/image/0.23.12/image/flat/struct.FlatSamples.html?
If it is possible to describe a GdkPixbuf with a SampleLayout then it is possible to copy the pixels out from the buffer
Danilo
@dbrgn:matrix.coredump.ch
[m]
Thanks!
Dakota
@codabrink
How do you create a DynamicImage from an RgbImage? I'd like to write it to a vec in a jpeg format in memory.
The documentation isn't clear on how to handle it
Dakota
@codabrink
Figured it out.. still new to Rust. let img = DynamicImage::ImageRgb8(img);
HeroicKatora
@HeroicKatora
Yes, that was quick. Exactly right :+1: