pnm-plugin #4253
pnm-plugin #4253
Conversation
• import/export support for 8/16 uint 32-float • bigendian • pfmflip for float pfm files Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
src/pnm.imageio/pnminput.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| m_spec["pnm:pfmflip"] = config.get_int_attribute("pnm:pfmflip", 1); |
There was a problem hiding this comment.
m_spec is for communicating what's in the file to the caller.
config is for the caller to give us instructions about how to read.
So it's unusual to copy something from the config into m_spec. I suspect you are merely trying to communicate that config hint to the later reading functions, but usually what we do in a case like this is just have a bool as a private member of the PNMInput, set it here, and use it as needed in the read_scanline function.
There was a problem hiding this comment.
I think at the very least, we should only set this in m_spec if it really is a pfm file, not for the non-float varieties.
Orthography fix. Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
|
I'm a little confused, so let me say what I think is going on, and you can confirm or let me know where I'm getting it wrong: The pbm, pgm, and ppm varieties are stored with scanlines ordered in the file as top-to-bottom (the same as the usual OIIO convention), but the float-based pfm files are conventionally ordered in the file as bottom-to-top. Therefore, by default, reading and writing of the pfm variety will automatically flip the image so that an application calling the OpenImageIO API can, as usual, assume that scanline 0 is the visual "top" (even though it is actually the last scanline stored in the file). Both the reader and writer accept configuration hints "pnm:pfmflip" (default: 1), which if set to 0 will disable this flipping and ensure that scanline 0 is written as the first in the file (therefore representing what PFM assumes is the visual "bottom" of the image). This hint only affects PFM files and has no effect on the pbm, pgm, or ppm varieties. |
|
Correct. If you think it worth to disable flipping to be on the same page with USC ITC (https://pauldebevec.com/Research/HDR/PFM/ ) and Adobe I'm in! |
|
Is GIMP the only major faction that assumes y=0 is the bottom? Adobe thinks it's the top? Is there an official "PFM spec" and does it address the issue? When you encounter pfm files in the wild, do they tend to mostly follow the GIMP convention? |
|
Gimp, tools from libjxl, NetPBM package... looks like all that use bottom line as y=0. And there is no official spec for this extension of NetPBM format 🤷♂️ Float NetPBM is rarely used, as most of image sources are uint. |
|
OK, NetPBM seems like the closest to determining what should be the de facto standard, so let's go with flipping by default (for pfm only), with the ability of providing the hint to turn it off if needed. Could you please add those two paragraphs I wrote above in the introduction to pnm part of the documentation? |
src/pnm.imageio/pnminput.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| m_spec["pnm:pfmflip"] = config.get_int_attribute("pnm:pfmflip", 1); |
There was a problem hiding this comment.
I think at the very least, we should only set this in m_spec if it really is a pfm file, not for the non-float varieties.
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
some text formatting Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
m_pfm_flip private variable. Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
revert back Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
pfm always binary Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
|
@lgritz hope fixed now |
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Fixes added: - import/export support for 8/16 uint 32-float - "pnm:bigendian" attribute reports/controls andianness - "pnm:pfmflip" attribute (when set to 0) can disable the automatic vertical flip of pfm files (GIMP flips float PFM files by default, Photoshop does not) - update in documentation --------- Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
Fixes added: - import/export support for 8/16 uint 32-float - "pnm:bigendian" attribute reports/controls andianness - "pnm:pfmflip" attribute (when set to 0) can disable the automatic vertical flip of pfm files (GIMP flips float PFM files by default, Photoshop does not) - update in documentation --------- Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com> Signed-off-by: Vlad (Kuzmin) Erium <shaamaan@gmail.com>
Description
Fixes added:
Checklist:
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.