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

TIFF: subtle bug -- re-opening forgot about rawcolor hint. #2285

Merged
merged 6 commits into from
Jul 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions src/doc/pythonbindings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,21 @@ awaiting a call to `reset()` or `copy()` before it is useful.



.. py:method:: ImageBuf (filename, subimage, miplevel, config)

Construct a read-only ImageBuf that will read from the named file,
with an ImageSpec `config` giving configuration hints.

Example::

import OpenImageIO as oiio
...
config = ImageSpec()
config.attribute("oiio:RawColor", 1)
buf = ImageBuf ("grid.tif", 0, 0, config)



.. py:method:: ImageBuf (imagespec, zero = True)

Construct a writeable ImageBuf of the dimensions and data format
Expand Down
9 changes: 7 additions & 2 deletions src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -765,8 +765,13 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel)
m_nmiplevels = 0;
static ustring s_subimages("subimages"), s_miplevels("miplevels");
static ustring s_fileformat("fileformat");
if (m_configspec) // Pass configuration options to cache
m_imagecache->add_file(m_name, nullptr, m_configspec.get());
if (m_configspec) { // Pass configuration options to cache
// Invalidate the file in the cache, and add with replacement
// because it might have a different config than last time.
m_imagecache->invalidate(m_name, true);
m_imagecache->add_file(m_name, nullptr, m_configspec.get(),
/*replace=*/true);
}
m_imagecache->get_image_info(m_name, subimage, miplevel, s_subimages,
TypeInt, &m_nsubimages);
m_imagecache->get_image_info(m_name, subimage, miplevel, s_miplevels,
Expand Down
5 changes: 5 additions & 0 deletions src/python/py_imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ declare_imagebuf(py::module& m)
auto z = zero ? InitializePixels::Yes : InitializePixels::No;
return ImageBuf(spec, z);
}))
.def(py::init([](const std::string& name, int subimage, int miplevel,
const ImageSpec& config) {
return ImageBuf(name, subimage, miplevel, nullptr, &config);
}),
"name"_a, "subimage"_a, "miplevel"_a, "config"_a)
.def("clear", &ImageBuf::clear)
.def(
"reset",
Expand Down
11 changes: 10 additions & 1 deletion src/tiff.imageio/tiffinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ class TIFFInput final : public ImageInput {
m_subimage_specs.clear();
}

// Just close the TIFF file handle, but don't forget anything we
// learned about the contents of the file or any configuration hints.
void close_tif()
{
if (m_tif) {
Expand Down Expand Up @@ -1428,7 +1430,14 @@ TIFFInput::read_native_scanline(int subimage, int miplevel, int y, int z,
ImageSpec dummyspec;
int old_subimage = current_subimage();
int old_miplevel = current_miplevel();
if (!close() || !open(m_filename, dummyspec)
// We need to close the TIFF file s that we can re-open and
// seek back to the beginning of this subimage. The close_tif()
// accomplishes that. It's important not to do a full close()
// here, because that would also call init() to fully reset
// to a fresh ImageInput, thus forgetting any configuration
// settings such as raw_color or keep_unassociated_alpha.
close_tif();
Copy link
Contributor

Choose a reason for hiding this comment

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

Given how tricky this was to track down -- at least leave a comment about why you call close_tif vs. close() here?

And surely this deserves a unit test? 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, will do...

if (!open(m_filename, dummyspec)
|| !seek_subimage(old_subimage, old_miplevel)) {
return false; // Somehow, the re-open failed
}
Expand Down
26 changes: 26 additions & 0 deletions testsuite/python-imageinput/ref/out-alt.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,30 @@ Test read_image into FLOAT:
Opened "testu16.tif" as a tiff
Read array typecode float32 [ 12288 ]

Testing write and read of unassociated:
writing: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

default reading as IB: [[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]

[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]]

reading as IB with unassoc hint: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

reading as II with hint, read scanlines backward:
[1] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]
[0] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]


Done.
26 changes: 26 additions & 0 deletions testsuite/python-imageinput/ref/out-alt2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,30 @@ Test read_image into FLOAT:
Opened "testu16.tif" as a tiff
Read array typecode float32 [ 12288 ]

Testing write and read of unassociated:
writing: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

default reading as IB: [[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]

[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]]

reading as IB with unassoc hint: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

reading as II with hint, read scanlines backward:
[1] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]
[0] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]


Done.
26 changes: 26 additions & 0 deletions testsuite/python-imageinput/ref/out-python3.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,30 @@ Test read_image into FLOAT:
Opened "testu16.tif" as a tiff
Read array typecode float32 [ 12288 ]

Testing write and read of unassociated:
writing: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

default reading as IB: [[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]

[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]]

reading as IB with unassoc hint: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

reading as II with hint, read scanlines backward:
[1] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]
[0] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]


Done.
26 changes: 26 additions & 0 deletions testsuite/python-imageinput/ref/out-travis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,30 @@ Test read_image into FLOAT:
Opened "testu16.tif" as a tiff
Read array typecode float32 [ 12288 ]

Testing write and read of unassociated:
writing: [[[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]

[[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]]

default reading as IB: [[[ 0.25 0.25 0.25 0.5 ]
[ 0.25 0.25 0.25 0.5 ]]

[[ 0.25 0.25 0.25 0.5 ]
[ 0.25 0.25 0.25 0.5 ]]]

reading as IB with unassoc hint: [[[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]

[[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]]

reading as II with hint, read scanlines backward:
[1] = [[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]
[0] = [[ 0.5 0.5 0.5 0.5]
[ 0.5 0.5 0.5 0.5]]


Done.
26 changes: 26 additions & 0 deletions testsuite/python-imageinput/ref/out.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,30 @@ Test read_image into FLOAT:
Opened "testu16.tif" as a tiff
Read array typecode float32 [ 12288 ]

Testing write and read of unassociated:
writing: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

default reading as IB: [[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]

[[0.25 0.25 0.25 0.5 ]
[0.25 0.25 0.25 0.5 ]]]

reading as IB with unassoc hint: [[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]

[[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]]

reading as II with hint, read scanlines backward:
[1] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]
[0] = [[0.5 0.5 0.5 0.5]
[0.5 0.5 0.5 0.5]]


Done.
28 changes: 28 additions & 0 deletions testsuite/python-imageinput/src/test_imageinput.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,32 @@ def write (image, filename, format=oiio.UNKNOWN) :
print ("Error writing", filename, ":", image.geterror())


# Regression test for #2285: configuration settings were "forgotten" if the
# scanline read order necessitated closing and reopening the file.
def test_tiff_remembering_config() :
# Create a file that has unassociated alpha
print ("Testing write and read of unassociated:")
spec = oiio.ImageSpec(2,2,4,"float")
spec.attribute("oiio:UnassociatedAlpha", 1)
wbuf = oiio.ImageBuf(spec)
oiio.ImageBufAlgo.fill(wbuf, (0.5, 0.5, 0.5, 0.5))
print (" writing: ", wbuf.get_pixels())
wbuf.write("test_unassoc.tif")
rbuf = oiio.ImageBuf("test_unassoc.tif")
print ("\n default reading as IB: ", rbuf.get_pixels())
config = oiio.ImageSpec()
config.attribute("oiio:UnassociatedAlpha", 1)
rbuf = oiio.ImageBuf("test_unassoc.tif", 0, 0, config)
print ("\n reading as IB with unassoc hint: ", rbuf.get_pixels())
print ("\n reading as II with hint, read scanlines backward: ")
ii = oiio.ImageInput.open("test_unassoc.tif", config)
print (" [1] = ", ii.read_scanline(1))
print (" [0] = ", ii.read_scanline(0))
print ("\n")




######################################################################
# main test starts here

Expand Down Expand Up @@ -258,6 +284,8 @@ def write (image, filename, format=oiio.UNKNOWN) :
test_readimage ("testu16.tif", method="image", type="float",
keep_unknown=True, print_pixels=False)

test_tiff_remembering_config()

print ("Done.")
except Exception as detail:
print ("Unknown exception:", detail)
Expand Down