bug: deep paste wraps into the next scanline instead of clipping, and ignores chbegin

Start in src/libOpenImageIO/imagebufalgo_copy.cpp: the dispatch in paste() near line 120 and deep_paste_(), where the OIIO_ASSERT sits before the !dst.initialized() reset. Then read set_deep_samples and copy_deep_pixel in imagebuf.cpp, which pass the default check_range=false. Run the deep paste cases in testsuite/oiiotool-deep. Done when a deep paste clips at the destination edge, chbegin is honoured or refused, and no assertion prints.

Describe the bug

ImageBufAlgo::paste has two paths. The flat one calls IBAprep; the deep one
does not, and the substitute it uses loses the destination region. Three
separate things follow, and all three are reported as success.

src/libOpenImageIO/imagebufalgo_copy.cpp at v3.2.2.0 (identical on main):

    // Special case for deep                                          // :119
    if ((dst.deep() || !dst.initialized()) && src.deep())             // :120
        return deep_paste_(dst, src, dstroi, srcroi, nthreads);

    if (!IBAprep(dstroi, &dst))                                       // :123
        return false;

IBAprep sizes an uninitialized destination to the destination region
(imagebufalgo.cpp:252, set_roi(spec, roi)). deep_paste_ substitutes this,
at :72:

    if (!dst.initialized()) {
        dst.reset(src.spec());
    }

That takes the source's window. dstroi is then used only to compute
relative_x/y/z at the top of the function, and its channel range is never
read at all.

Symptom one: the paste wraps into the next scanline instead of clipping

After the reset, deep_paste_ writes:

    dst.set_deep_samples(x + relative_x, y + relative_y, z + relative_z,
                         src.deep_samples(x, y, z));
    ...
    dst.copy_deep_pixel(x + relative_x, y + relative_y, z + relative_z,
                        src, x, y, z);

ImageBuf::set_deep_samples (imagebuf.cpp:2942) and
ImageBuf::copy_deep_pixel (:2999) both resolve the coordinate with
pixelindex(x, y, z), leaving check_range at its declared default of false
(imagebuf.h:1419). With the range check off, a coordinate past the right edge
is not clipped. It gets a linear index that lands on the next scanline.

Of the ten deep accessors in imagebuf.cpp, only two pass true:
deep_samples at :2896 and deep_pixel_ptr at :2909. The other eight,
readers and writers alike, leave it at the default.

#2369 is the change that extended paste to deep images, and it defines the
contract this breaks, in its own words: "Normally, the foreground image will
'clip' against the extent of the background image's data window", and "pixel
(0,0,0) of F will replace (x,y,z) in B". It also says "This works for deep
files, too". The clipping does not happen for deep; the wrap happens instead.

Symptom two: chbegin is ignored

dstroi carries srcroi.chbegin + chbegin, and deep_paste_ never looks at
the channel range. copy_deep_pixel is whole-pixel. So a flat paste with a
first channel of 2 widens the destination to seven channels and puts the source
at channel 2, and a deep one returns five channels with the source at channel
0, both returning true.

Symptom three: the assertion rejects a case the function supports

deep_paste_ opens at :59 with

    OIIO_ASSERT(dst.deep() && src.deep());

twelve lines above the !dst.initialized() handling, and paste dispatches to
it on (dst.deep() || !dst.initialized()) && src.deep(). dst.deep() is false
for an uninitialized ImageBuf, so the assertion fires on the one shape the
function goes on to accommodate.

OIIO_ASSERT is not compiled out in a release build. It always evaluates the
condition and always writes the message to stderr; only the abort is
conditional, through OIIO_ABORT_IF_DEBUG (dassert.h:176-189). Its own
documentation says so: "When in release mode, it prints the error message if
the condition fails, but does not abort." So every deep paste into an
uninitialized ImageBuf writes an assertion line to stderr, on every build,
while returning true. One test suite run of mine produced 25 of them while
reporting success throughout, which is what hid it: the output looks like a
failure and the exit status does not.

The assertion at :157, in copy_deep, is a different case and looks correct.
That one dereferences dst.deepdata() immediately, so a destination that is
not deep would be a null dereference rather than a supported shape.

This is not a memory safety bug. Three independent bounds tests absorb the
out-of-range writes: DeepData::set_samples (deepdata.cpp:547),
DeepData::copy_deep_pixel (:911) and DeepData::data_ptr (:636 and
:647). What is left is a wrong-pixel bug and silent sample loss.

OpenImageIO version and dependencies

OIIO 3.2.2.0 | Windows/x86_64
    Build compiler: MSVS 1951 | C++17/199711
    HW features enabled at build: sse2,sse3,ssse3,sse41,sse42
    No CUDA support (disabled / unavailable at build time)
Dependencies: BZip2 1.0.8, DCMTK NONE, FFmpeg NONE, fmt 12.1.0, Freetype 2.13.3, GIF 5.2.2, Imath 3.2.2, JXL NONE, libdeflate 1.25, Libheif NONE, libjpeg-turbo 3.1.3, LibRaw NONE, libuhdr NONE, OpenColorIO 2.5.1, OpenCV NONE, OpenEXR 3.4.7, OpenJPEG 2.5.4, openjph 0.26.3, PNG 1.6.55, Ptex NONE, Ptex NONE, Robinmap 1.4.1, TBB NONE, TIFF 4.7.1, WebP 1.6.0, ZLIB 1.3.1, zstd 1.5.7

Nothing unusual about the environment and no nonstandard build options. The
cited imagebufalgo_copy.cpp lines are identical on v3.2.2.0, v3.1.9.0 and
main. The imagebuf.cpp, deepdata.cpp and imagebufalgo.cpp line numbers
above are v3.2.2.0 and main; on v3.1.9.0 the same statements are at
imagebuf.cpp:2868, :2925, :2822, deepdata.cpp:552 and
imagebufalgo.cpp:229.

To Reproduce

oiiotool only, on a published sample file:
openexr-images/v2/LowResLeftView/Balls.exr, a deep render element whose data
window is 764 by 406 at 131,170.

B=Balls.exr

# Paste the file onto itself, ten pixels to the right.
oiiotool $B $B --paste +10+0 -o p10.exr

# Flatten both so the samples can be compared as pixels.
oiiotool p10.exr --flatten -o f10.exr
oiiotool $B      --flatten -o fsrc.exr

# The destination's leftmost 10 columns, from row 171 down.
oiiotool f10.exr  --cut 10x405+131+171 -o A.exr
# The source's RIGHTMOST 10 columns, from row 170, one row earlier.
oiiotool fsrc.exr --cut 10x405+885+170 -o B2.exr
# What a correct paste leaves in that strip: the untouched background.
oiiotool fsrc.exr --cut 10x405+131+171 -o C.exr

oiiotool --diff A.exr B2.exr    # PASS: bit identical. This is the wrap.
oiiotool --diff A.exr C.exr     # FAILURE: 644 pixels (15.9%) over 1e-06

The destination's left strip is bit-identical to the source's right strip from
the preceding scanline. That is the wrap, measured: columns 885..894 of row
y landed at columns 131..140 of row y + 1.

For the sample loss, use an offset larger than the data window is wide:

oiiotool --stats $B | grep -E "Total deep samples|Pixels with deep"
#     Total deep samples in all pixels: 162877
#     Pixels with deep samples   : 117256

oiiotool $B $B --paste +800+0 -o p800.exr
oiiotool --stats p800.exr | grep -E "Total deep samples|Pixels with deep"
#     Total deep samples in all pixels: 162464
#     Pixels with deep samples   : 117051

413 samples and 205 pixels gone, exit status 0, nothing reported. The control
is --paste +0+0, which returns the source's figures exactly.

oiiotool --paste takes no channel offset, so symptom two needs the API. This
uses only public headers:

#include <OpenImageIO/imagebuf.h>
#include <OpenImageIO/imagebufalgo.h>
#include <cstdio>

int main()
{
    OIIO::ImageSpec spec(8, 8, 5, OIIO::TypeDesc::FLOAT);
    spec.channelnames = { "R", "G", "B", "A", "Z" };
    spec.deep         = true;
    OIIO::ImageBuf deep_src(spec);
    for (int y = 0; y < 8; ++y)
        for (int x = 0; x < 8; ++x) {
            deep_src.set_deep_samples(x, y, 0, 2);
            deep_src.set_deep_value(x, y, 0, 4, 0, 1.0f);
            deep_src.set_deep_value(x, y, 0, 4, 1, 2.0f);
        }

    OIIO::ImageBuf flat_src(OIIO::ImageSpec(8, 8, 5, OIIO::TypeDesc::FLOAT));

    OIIO::ImageBuf a, b;
    OIIO::ImageBufAlgo::paste(a, 0, 0, 0, 2, flat_src);
    OIIO::ImageBufAlgo::paste(b, 0, 0, 0, 2, deep_src);
    std::printf("flat chbegin 2: %d channels\ndeep chbegin 2: %d channels\n",
                a.nchannels(), b.nchannels());
    return 0;
}

Expected 7 and 7. Observed, on the 3.2.2.0 build above:

flat chbegin 2: 7 channels
deep chbegin 2: 5 channels

on stdout, one assertion line from the deep call on stderr, and exit status 0.
Linking needs OpenImageIO and OpenImageIO_Util.

Expected behavior

A deep paste should clip against the destination's data window, as #2369 says
it does and as the flat path does, and should put the source where a flat paste
of the same arguments puts it. It should honour chbegin or refuse it. And it
should not print an assertion failure for a call it supports.

The fix

All three come from the missing IBAprep, so the shape of the fix is to stop
substituting for it.

Size an uninitialized destination from dstroi rather than from the source,
and move the assertion below the reset so it checks the state the rest of the
function relies on:

    if (!dst.initialized()) {
        ImageSpec newspec = src.spec();
        set_roi(newspec, dstroi);
        set_roi_full(newspec, dstroi);
        newspec.nchannels      = src.nchannels();
        newspec.channelnames   = src.spec().channelnames;
        newspec.channelformats = src.spec().channelformats;
        dst.reset(newspec);
    }
    OIIO_ASSERT(dst.deep() && src.deep());

Then clip the source region to what actually lands, rather than relying on
DeepData's bounds tests to absorb out-of-range indices:

    srcroi = roi_intersection(srcroi,
                              ROI(dst.xbegin() - relative_x,
                                  dst.xend()   - relative_x,
                                  dst.ybegin() - relative_y,
                                  dst.yend()   - relative_y,
                                  dst.zbegin() - relative_z,
                                  dst.zend()   - relative_z,
                                  srcroi.chbegin, srcroi.chend));

An alternative for the second half is to pass true for check_range in
ImageBuf::set_deep_samples and ImageBuf::copy_deep_pixel, which would make
every out-of-window write a no-op rather than a wrong-pixel write. That is a
broader change, since both are public and the other six unchecked deep
accessors have the same shape, but it would close the same hole for any other
caller that has it.

chbegin needs either an implementation, which means a per-channel deep copy
rather than copy_deep_pixel, or an explicit refusal.

Test coverage

There is no deep --paste case at all. testsuite/oiiotool-deep and
testsuite/python-deep contain no paste, and the only non-trivial --paste
in the testsuite is flat: oiiotool-copy/run.py:168 pastes a checker at
+150+75, which is why the flat path's clipping is known to work and the deep
path's absence of it is not.

testsuite/oiiotool-deep/run.py:43 already reads Balls.exr from the
openexr-images directory, so the file the reproduction above uses is one the
testsuite has to hand. A regression test could paste it at a non-zero offset
and assert that the deep sample total is conserved when the whole source lands
inside the destination, and that it drops monotonically as the offset carries
the source out.