Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Buffer based on canvas #1718

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Buffer based on canvas #1718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1903b76
Add work-in-progress bindings to image-canvas
HeroicKatora May 24, 2022
d55a451
Fill in flat layouts for canvas bindings
HeroicKatora May 24, 2022
18bac68
Expand on canvas concepts, document
HeroicKatora May 25, 2022
dc6b818
Implement layout to color type
HeroicKatora May 26, 2022
11b9b54
Update canvas to decode L1
HeroicKatora Jun 9, 2022
bb0b60e
Implement extended color types
HeroicKatora Jun 11, 2022
d764076
Merge remote-tracking branch 'upstream/master' into buffer-based-on-c…
HeroicKatora Nov 2, 2022
7def82b
A minimal formatting change
HeroicKatora Nov 2, 2022
11aabfc
Setup buffer as private dependency
HeroicKatora Nov 2, 2022
1bef917
Add in-place reading of image data
HeroicKatora Nov 2, 2022
c69c1d0
Ensure roundtrips for simple color type
HeroicKatora Nov 2, 2022
811898c
Add documentation to discriminate buffers
HeroicKatora Nov 7, 2022
2379f7e
Make Canvas::to_buffer conversion fallible
HeroicKatora Nov 7, 2022
bdc6626
Fix canvas tests to use fallible conversion
HeroicKatora Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ path = "./src/lib.rs"
[dependencies]
bytemuck = { version = "1.7.0", features = ["extern_crate_alloc"] } # includes cast_vec
byteorder = "1.3.2"
image-canvas = "0.4.1"
num-rational = { version = "0.4", default-features = false }
num-traits = "0.2.0"

gif = { version = "0.11.1", optional = true }
jpeg = { package = "jpeg-decoder", version = "0.2.1", default-features = false, optional = true }
png = { version = "0.17.0", optional = true }
Expand Down
3 changes: 2 additions & 1 deletion Cargo.toml.public-private-dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ path = "./src/lib.rs"
[dependencies]
bytemuck = { version = "1.7.0", features = ["extern_crate_alloc"] } # includes cast_vec
byteorder = "1.3.2"
num-iter = "0.1.32"
image-canvas = "0.4.1"
num-rational = { version = "0.4", default-features = false }
num-traits = { version = "0.2.0", public = true }

gif = { version = "0.11.1", optional = true }
jpeg = { package = "jpeg-decoder", version = "0.2.1", default-features = false, optional = true }
png = { version = "0.17.0", optional = true }
Expand Down
19 changes: 18 additions & 1 deletion src/buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
//! Contains the generic `ImageBuffer` struct.
//! Defines standard containers for image data.
//!
//! The best container depends on the use:
//! * The generic [`ImageBuffer`] struct, a simplified pixel container with a standard matrix
//! layout of uniform channels. The strength is convenient access for modifications with a direct
//! mapping and access to the underlying data.
//! * The [`DynamicImage`] enumeration builds on `ImageBuffer` to provide a standard selection of
//! basic channel representations, and conventionally in sRGB color space. This is usually enough
//! to represent any source data without loosing much precision. This makes is suitable for
//! basic, but generic, image operations.
//! * The [`Canvas`] struct as general texel container of general layouts as a buffer. The strength
//! is the generality (and extensibility) that allows its use for interchange of image data and
//! reduced processing cost if the convenience above is not required.
Comment on lines +3 to +13
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this somewhat clear motivation?

Copy link
Contributor

@fintelia fintelia Nov 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay responding. That explains what sorts of images the different types can store, but doesn't really think through what the user can do with them. My rough thinking:

  • ImageBuffer provides both a collection of image processing methods (resize, crop, etc.) as well as ergonomic pixel getters and setters to implement your own image processing. The serious drawback is that the color type is determined at compile time. One consequence is that saving works fine but loading files as ImageBuffers isn't directly supported.
  • DynamicImage supports even more built in image processing as well as loading/saving a whole range of file formats. Conversion between color types is supported. The enum is non-exhaustive but users can implement their own algorithms by matching on the current variants with special case logic for each one.
  • Canvas holds and converts between a broader range of color types. No other image processing algorithms are provided and users who want to write their own must... rely on raw access to aligned byte slices? Cast them to FlatSamples first? That's perhaps a bit unfair of a characterization since the type could still be useful as a common "interchange" representation for image data in Rust. However, I suspect it would have a better chance of catching on if users didn't have to immediately convert the Canvas into something else to operate on it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other image processing algorithms are provided and users who want to write their own must... rely on raw access to aligned byte slices?

Precisely. The best reason to use the buffer is uploading it to a GPU texture as directly as possible; for which only the layout and raw bytes are necessary and sufficient, and all further CPU processing is less precise and more costly than necessary. (And I stopped expanding the interface, before anyone could get the idea of making this PR contingent on a corresponding wgpu GPU framework).

The lack of direct operations is by design. Precisely because we may not have interpreted the correct color information (color space, or ICC, or otherwise) yet, just the need to store some numeric texel matrix anyways. There's just nothing sensible to do with the matrix entries, at that point. It should get support for Yuv layouts and CMYK rather soon to fix the tragedy of wrongly applying incorrect ICC tables while loading jpeg, WebP, png and tiff, as we are doing currently... But that's for another PR in my opinion.

#[path = "buffer/canvas.rs"]
mod canvas;

use num_traits::Zero;
use std::fmt;
use std::marker::PhantomData;
Expand All @@ -15,6 +30,8 @@ use crate::math::Rect;
use crate::traits::{EncodableLayout, Pixel, PixelWithColorType};
use crate::utils::expand_packed;

pub use self::canvas::{Canvas, CanvasLayout, UnknownCanvasTexelError};

/// Iterate over pixel refs.
pub struct Pixels<'a, P: Pixel + 'a>
where
Expand Down
Loading