From f879bda2a7ae9556fa00179f145cc556fe431655 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Thu, 18 Jun 2026 15:08:14 -0700 Subject: [PATCH] fix(ImageBuf): Copy ctor of IC-backed ImageBuf zeroed bufspan strides When copy-constructing an ImageCache-backed ImageBuf, the "cache-based or deep" branch set `m_bufspan = {}`, a default-constructed image_span whose strides are all zero. retile() addresses a pixel within a cache tile as `offset = pixelindex * m_bufspan.xstride()`, so a zero xstride made every pixel resolve to tile offset 0 -- the whole copied image read back as a constant color (the value of pixel 0,0). Fixes 5243 Added a regression test (test_copy_of_imagecache_backed) that copies a cache-backed buffer with distinct per-pixel values and verifies the copy does not read back a constant. Assisted-by: Claude Code / Claude Opus 4.8 Signed-off-by: Larry Gritz --- src/libOpenImageIO/imagebuf.cpp | 10 +++++-- src/libOpenImageIO/imagebuf_test.cpp | 43 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index aa8bd9e8b8..46b23b5dce 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -543,9 +543,13 @@ ImageBufImpl::ImageBufImpl(const ImageBufImpl& src) // N.B. new_pixels will set m_bufspan } } else { - // Source was cache-based or deep - // nothing else to do - m_bufspan = {}; + // Source was cache-based or deep. There are no local pixels to copy, + // but for a cache-backed source we must preserve src's bufspan, which + // carries null data but valid (pixel_bytes) strides. retile() relies + // on m_bufspan.xstride() to address pixels within a cache tile; a + // default-constructed (zeroed-stride) bufspan would make every pixel + // resolve to tile offset 0 and read back as a constant color. + m_bufspan = src.m_bufspan; } if (localpixels() || m_spec.deep) { // A copied ImageBuf is no longer a direct file reference, so clear diff --git a/src/libOpenImageIO/imagebuf_test.cpp b/src/libOpenImageIO/imagebuf_test.cpp index f85b78934c..695e524426 100644 --- a/src/libOpenImageIO/imagebuf_test.cpp +++ b/src/libOpenImageIO/imagebuf_test.cpp @@ -611,6 +611,48 @@ test_mutable_iterator_with_imagecache() +// Regression test: copy-constructing an ImageCache-backed ImageBuf must +// preserve the buffer span's strides. A default-constructed bufspan has zero +// strides, which made retile() resolve every pixel to tile offset 0, so the +// whole copied image read back as a constant (the value of pixel 0,0). See +// the ImageBuf copy constructor's "cache-based or deep" branch. +void +test_copy_of_imagecache_backed() +{ + // Make a 16x16 1-channel float image where every pixel has a distinct + // value (so a "constant" misread is obvious), write it. + char srcfilename[] = "tmp_copy_ic.exr"; + const int W = 16, H = 16; + ImageSpec fsize(W, H, 1, TypeFloat); + ImageBuf src(fsize); + for (ImageBuf::Iterator it(src); !it.done(); ++it) + it[0] = float(it.x() + it.y() * W); + src.write(srcfilename); + + // Open it cache-backed (not force-read into local memory). + ImageBuf buf(srcfilename, 0, 0, ImageCache::create()); + OIIO_CHECK_EQUAL(buf.storage(), ImageBuf::IMAGECACHE); + OIIO_CHECK_ASSERT(!buf.localpixels()); + + // Copy-construct -- this is what ImageBufAlgo::make_texture does to an + // ImageCache-backed input buffer. + ImageBuf copy(buf); + OIIO_CHECK_EQUAL(copy.storage(), ImageBuf::IMAGECACHE); + OIIO_CHECK_ASSERT(!copy.localpixels()); + + // Every pixel of the copy must still read its distinct value, not a + // constant. Iterate (exercises retile) and also spot-check getchannel. + for (ImageBuf::ConstIterator it(copy); !it.done(); ++it) + OIIO_CHECK_EQUAL(it[0], float(it.x() + it.y() * W)); + OIIO_CHECK_EQUAL(copy.getchannel(W - 1, H - 1, 0, 0), + float((W - 1) + (H - 1) * W)); + + ImageCache::create()->invalidate(ustring(srcfilename)); + Filesystem::remove(srcfilename); +} + + + void time_iterators() { @@ -788,6 +830,7 @@ main(int argc, char* argv[]) iterator_wrap_test>(ImageBuf::WrapMirror, "mirror"); test_mutable_iterator_with_imagecache(); + test_copy_of_imagecache_backed(); time_iterators(); test_iterator_concurrency();