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.

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

Support reading image print resolution #1067

Open
cormacrelf opened this issue Oct 21, 2019 · 10 comments
Open

Support reading image print resolution #1067

cormacrelf opened this issue Oct 21, 2019 · 10 comments
Labels
kind: enhancement topic: meta data Towards dealing with meta data

Comments

@cormacrelf
Copy link

I would like to be able to access image print resolution in image formats which support it.

My specific use case for this functionality is to replace the image decoders in the c2rust-converted XeTeX source code in this Tectonic working group. See crlf0710/tectonic#68. Tectonic is obviously print-oriented, and needs to be able to lay out images on real paper dimensions. It therefore tries to establish how big an image would like to be printed, which it currently does by reading, for example, pixels per metre from a Windows BMP's BITMAPINFOHEADER and converting to pixels per inch as f64. So without this feature essentially all of the image decoding routines have to stay just to get these values, and image doesn't help.

This is more generally applicable to ... any code that intends to display images on screens or on paper. I would expect a GUI image viewer component to need this.

Draft

Different image formats expose this kind of thing differently.

  • The png crate exposes PixelDimensions
  • The jpeg_decoder crate currently just ignores JFIF APP0 blocks which include x/y/units (with units = {cm | in | pixel aspect ratio}
  • The bmp module consumes but ignores those parts of headers that support density information (

    image/src/bmp/decoder.rs

    Lines 710 to 716 in e48d283

    // The next 12 bytes represent data array size in bytes,
    // followed the horizontal and vertical printing resolutions
    // We will calculate the pixel array size using width & height of image
    // We're not interesting the horz or vert printing resolutions
    self.reader.read_u32::<LittleEndian>()?;
    self.reader.read_u32::<LittleEndian>()?;
    self.reader.read_u32::<LittleEndian>()?;
    )
  • ...

I haven't looked at the rest but I couldn't quite say at this stage if there should be a common few enums that can support all formats' density information, or different types for each format. I would say that for Tectonic, I don't think we care about preserving all this information, because however it was represented, we are just going to convert it to PPI. Maybe a simple fn pixels_per_inch(&self) -> Option<(f64, f64)> on the *Decoders which support it would suffice.

@fintelia
Copy link
Contributor

I'd be in favor of adding a method like this to the ImageDecoder trait. Do you have a sense of what the best units to return would be? I'd lean towards SI meters or mm per pixel, but would be open to others as well

@HeroicKatora
Copy link
Member

Do you have a sense of what the best units to return would be?

What about a very simple newtype wrapper? It doesn't have to be fancy but simply a distinct f64 that we can properly document and may offer some convenience conversions to mm/per pixel and pixel/inch etc.. Also to give it a name and to avoid mistakes.

@cormacrelf
Copy link
Author

cormacrelf commented Oct 21, 2019

Just spitballing how a big old enum would look. It's not that bad. I think it might be worth it to represent pixel aspect ratios, which appear to be common; people might want to handle them differently ('customisable default image resolution' would obviously be useful in a graphics program). JPEG, BMP, PNG and GIF support it, which is far as I have looked.

/// Not exhaustive
pub enum PixelUnit {
    Metre,
    Millimetre,
    Inch,
    ??
}
pub enum BikeShedThis {
    /// Represents a pixel aspect ratio of  X : Y (should fail decoding if zero?)
    AspectRatio(u32, u32),
    /// The image specified how many pixels fit in one unit
    Density(u32, u32, PixelUnit),

    /// The image specified how long each of a single pixel's sides is
    /// Probably specified as floating point because units are big.
    /// Do any formats actually use this? I'm not sure.
    PixelLength(??, ??, PixelUnit),
}

pub struct PixelsPerInch(f64, f64);

// And then dissolve all that complexity with
impl BikeShedThis {
    /// If self is an aspect ratio -- what should it do? Default to scaling by 72.0?
    fn to_ppi(&self) -> PixelsPerInch { ... }
}

Also you don't have to make BikeShedThis public yet.

GIF 89a spec

viii) Pixel Aspect Ratio - Factor used to compute an approximation
            of the aspect ratio of the pixel in the original image.  If the
            value of the field is not 0, this approximation of the aspect ratio
            is computed based on the formula:

            Aspect Ratio = (Pixel Aspect Ratio + 15) / 64

            The Pixel Aspect Ratio is defined to be the quotient of the pixel's
            width over its height.  The value range in this field allows
            specification of the widest pixel of 4:1 to the tallest pixel of
            1:4 in increments of 1/64th.

            Values :        0 -   No aspect ratio information is given.
                       1..255 -   Value used in the computation.

EXIF

EXIF, which WebP can use for metadata, stores each of Exif.Image.XResolution and Exif.Image.YResolution as rationalu64, which means two u32s each, with a unit in Exif.Image.ResolutionUnit.

Units: https://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/EXIF.html

(the value 1 is not standard EXIF)   <<----- not sure what None means then: is it aspect ratio?
1 = None
2 = inches
3 = cm

PNG

png's documentation is a little inaccurate as "unspecified" is actually specified as being pixel aspect ratio: https://www.w3.org/TR/PNG/#11pHYs. PNG's have a u32 for each x and y.

JPEG

See the linked PR.

BMP

https://en.wikipedia.org/wiki/BMP_file_format#Bitmap_file_header

@fintelia
Copy link
Contributor

None of the conversions between units seem all that hard, so I don't think it is worth having conversions functions that just compute x / 1000.0, x / 25.4, 25.4 / x or whatever. Further an f64 has vastly more precision than anyone could possibly need or even measure, so converting wouldn't lose any precision.

The one concern I see is that some formats might report the pixel aspect ratio but not the actual pixel size. This can be solved by exposing two methods:

fn pixel_size_millimeters(&self) -> Option<(f64, f64)>;
fn pixel_aspect_ratio(&self) -> Option<f64> {
    self.pixel_size_millimeters().map(|(w,h)| w/h)
}

@cormacrelf
Copy link
Author

I'm more worried about converting twice than losing precision when converting once.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=82173379edbd8a361893a2d54c119d8b

@cormacrelf
Copy link
Author

cormacrelf commented Oct 21, 2019

(That little bit of floating point error is up to about 1/3 of a mm off when printing a photo from a Canon 5D Mark IV.) That was wrong.

However I still haven't found any image formats that actually specify a pixel size, only resolution, so I think it would better to do pixels-per rather than per-pixel.

@fintelia
Copy link
Contributor

fintelia commented Oct 21, 2019

Those errors are on the scale of 1/1000000 of the diameter of a hydrogen atom...

No objections to doing pixel-per-X though

@cormacrelf
Copy link
Author

Yeah, it was from an erroneous u32 division, my bad. I would probably give both pixels per inch and pixels per cm as they are the two most likely things people would need by a long margin.

@HeroicKatora
Copy link
Member

With regards to BikeShedThis: Resolution.

The public interface should be simpler. For example a method named pixel-per-inch (or dots-per-inch) is already clearly stating the units so PixelPerInch seems excessive. Are there use cases that need to know the underlying enum or can the interface hide this fact? (E.g. Offer constructors and accessors -- similar to Duration -- so we can always add variants or make existing constructors more precise).

@lovasoa
Copy link
Contributor

lovasoa commented Nov 23, 2019

I just made a pull request that is related to this issue (but is about writing the resolution, not reading it). It introduces a PixelDensity and a PixelDensityUnit type. Feel free to comment on #1078 !

@HeroicKatora HeroicKatora added the topic: meta data Towards dealing with meta data label Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement topic: meta data Towards dealing with meta data
Projects
None yet
Development

No branches or pull requests

4 participants