From 55138fff254b218aa099eda6f03dada49b78f15c Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 20 Aug 2024 11:40:21 -0700 Subject: [PATCH] api(IB): Add span-based constuctors to ImageBuf The flavor of ImageBuf that wraps caller-owned memory is constructed or reset by merely passing a raw pointer and it assumes that the range of memory implied by the strides is entirely safe to read from or write to. And even if correct, there is not much within ImageBuf internals to prevent bugs from buffer overruns. This PR introduces a new constructor and reset() method to ImageBuf where instead of taking a raw pointer, it takes a span describing the extent of the safely addressible memory (and also an optional "pointer to first pixel" within the buffer, which is necessary if you're using negative strides or for some other reason are describing safe memory that starts before the location of pixel 0). That span is stored in the ImageBuf, and also it remembers if it was a cspan (which should indicate a read-only IB) or a non-const span (which should wrap writable memory. At this moment, I'm just adding this API and will start using and documenting it as the perferred method, with the raw pointer versions being regarded as unsafe for now (and maybe some day will be deprecated?). But we're not yet using the span in other IB methods. In the future, my intent is to gradually add safeguards to do bounds checking against the span for IB operations that touch the wrapped memory. Because we'll most likely bounds check only in debug mode, it shouldn't ever impact performance, but it will make debugging, sanitizer, and fuzz tests more robust to catch and zero in on buffer overruns. I also like how it forces the caller, at the call site, to think hard about exactly what range of memory the IB is wrapping, and not merely pass a single pointer and hope it works out. I also dhanged all ImageBuf ctrs and resets within the OIIO codebase to use the new span-based versions This is part of an overall trend to migrate from raw pointer based API calls to span-based API design. Signed-off-by: Larry Gritz --- src/doc/Doxyfile | 1 + src/include/OpenImageIO/fmath.h | 8 +- src/include/OpenImageIO/imagebuf.h | 97 ++++++++++- src/include/OpenImageIO/imagebufalgo_opencv.h | 4 +- src/libOpenImageIO/imagebuf.cpp | 158 +++++++++++++++--- src/libOpenImageIO/imagebuf_test.cpp | 13 +- src/libOpenImageIO/imagebufalgo_draw.cpp | 4 +- src/libOpenImageIO/imagespeed_test.cpp | 2 +- src/libOpenImageIO/maketexture.cpp | 4 +- src/libtexture/imagecache.cpp | 7 +- src/webp.imageio/webpinput.cpp | 8 +- src/webp.imageio/webpoutput.cpp | 6 +- 12 files changed, 266 insertions(+), 46 deletions(-) diff --git a/src/doc/Doxyfile b/src/doc/Doxyfile index caee03d212..504ac3d1a7 100644 --- a/src/doc/Doxyfile +++ b/src/doc/Doxyfile @@ -2187,6 +2187,7 @@ PREDEFINED = DOXYGEN_SHOULD_SKIP_THIS \ OIIO_NAMESPACE_END="}" \ OIIO_CONSTEXPR17=constexpr \ OIIO_CONSTEXPR20=constexpr \ + OIIO_IB_DEPRECATE_RAW_PTR:= \ OIIO_INLINE_CONSTEXPR="inline constexpr" \ OIIO_PURE_FUNC= \ OIIO_CONST_FUNC= \ diff --git a/src/include/OpenImageIO/fmath.h b/src/include/OpenImageIO/fmath.h index c7242dab99..4f89c2aca4 100644 --- a/src/include/OpenImageIO/fmath.h +++ b/src/include/OpenImageIO/fmath.h @@ -693,9 +693,13 @@ sincos (double x, double* sine, double* cosine) } -inline OIIO_HOSTDEVICE float sign (float x) + +/// Return -1 if x<0, 0 if x==0, 1 if x>0. For floating point types, this is +/// not friendly to NaN inputs! +template +inline OIIO_HOSTDEVICE T sign(T x) { - return x < 0.0f ? -1.0f : (x==0.0f ? 0.0f : 1.0f); + return x < T(0) ? T(-1) : (x == T(0) ? T(0) : T(1)); } diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index c3f4402bc3..e6ce497581 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -20,6 +20,23 @@ #include +// Signal that this version of ImageBuf has constructors from spans +#define OIIO_IMAGEBUF_SPAN_CTR 1 + +// If before this header is included, OIIO_IMAGEBUF_DEPRECATE_RAW_PTR is +// defined, then we will deprecate the methods taking raw pointers. This is +// helpful for downstream apps aiming to transition to the new span-based API +// and want to ensure they are not using the old raw pointer API. +// #define OIIO_IMAGEBUF_DEPRECATE_RAW_PTR +#ifdef OIIO_IMAGEBUF_DEPRECATE_RAW_PTR +# define OIIO_IB_DEPRECATE_RAW_PTR \ + [[deprecated("Prefer the version that takes a span")]] +#else +# define OIIO_IB_DEPRECATE_RAW_PTR +#endif + + + OIIO_NAMESPACE_BEGIN class ImageBuf; @@ -224,7 +241,7 @@ class OIIO_API ImageBuf { /// Construct a writable ImageBuf that "wraps" existing pixel memory /// owned by the calling application. The ImageBuf does not own the - /// pixel storage and will will not free/delete that memory, even when + /// pixel storage and will not free/delete that memory, even when /// the ImageBuf is destroyed. Upon successful initialization, the /// storage will be reported as `APPBUFFER`. /// @@ -235,16 +252,38 @@ class OIIO_API ImageBuf { /// format), the ImageBuf will remain in an UNINITIALIZED /// state. /// @param buffer - /// A pointer to the caller-owned memory containing the - /// storage for the pixels. It must be already allocated - /// with enough space to hold a full image as described by - /// `spec`. + /// A span delineating the extent of the safely accessible + /// memory comprising the pixel data. + /// @param buforigin + /// A pointer to the first pixel of the buffer. If null, it + /// will be assumed to be the beginning of the buffer. (This + /// parameter is useful if any negative strides are used to + /// give an unusual layout of pixels within the buffer.) /// @param xstride/ystride/zstride /// The distance in bytes between successive pixels, /// scanlines, and image planes in the buffer (or /// `AutoStride` to indicate "contiguous" data in any of /// those dimensions). /// + template + ImageBuf(const ImageSpec& spec, span buffer, void* buforigin = nullptr, + stride_t xstride = AutoStride, stride_t ystride = AutoStride, + stride_t zstride = AutoStride) + : ImageBuf(spec, as_bytes(buffer), buforigin, xstride, ystride, zstride) + { + } + // Special base case for read-only byte spans, this one does the hard work. + ImageBuf(const ImageSpec& spec, cspan buffer, + void* buforigin = nullptr, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride); + // Special base case for mutable byte spans, this one does the hard work. + ImageBuf(const ImageSpec& spec, span buffer, + void* buforigin = nullptr, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride); + + // Unsafe constructor of an ImageBuf that wraps an existing buffer, where + // only the origin pointer and the strides are given. Use with caution! + OIIO_IB_DEPRECATE_RAW_PTR ImageBuf(const ImageSpec& spec, void* buffer, stride_t xstride = AutoStride, stride_t ystride = AutoStride, stride_t zstride = AutoStride); @@ -293,9 +332,51 @@ class OIIO_API ImageBuf { set_name(name); } - /// Destroy any previous contents of the ImageBuf and re-initialize it - /// as if newly constructed with the same arguments, to "wrap" existing - /// pixel memory owned by the calling application. + /// Destroy any previous contents of the ImageBuf and re-initialize it as + /// if newly constructed with the same arguments, to "wrap" existing pixel + /// memory owned by the calling application. + /// + /// @param spec + /// An ImageSpec describing the image and its metadata. + /// @param buffer + /// A span delineating the extent of the safely accessible + /// memory comprising the pixel data. + /// @param buforigin + /// A pointer to the first pixel of the buffer. If null, it + /// will be assumed to be the beginning of the buffer. (This + /// parameter is useful if any negative strides are used to + /// give an unusual layout of pixels within the buffer.) + /// @param xstride/ystride/zstride + /// The distance in bytes between successive pixels, + /// scanlines, and image planes in the buffer (or + /// `AutoStride` to indicate "contiguous" data in any of + /// those dimensions). + template + void reset(const ImageSpec& spec, span buffer, + const void* buforigin = nullptr, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride) + { + // The general case for non-byte data types just converts to bytes and + // calls the byte version. + reset(spec, as_writable_bytes(buffer), buforigin, xstride, ystride, + zstride); + } + // Special base case for read-only byte spans, this one does the hard work. + void reset(const ImageSpec& spec, cspan buffer, + const void* buforigin = nullptr, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride); + // Special base case for mutable byte spans, this one does the hard work. + void reset(const ImageSpec& spec, span buffer, + const void* buforigin = nullptr, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride); + + /// Unsafe reset of a "wrapped" buffer, mostly for backward compatibility. + /// This version does not pass a span that explicitly delineates the + /// memory bounds, but only passes a raw pointer and assumes that the + /// caller has ensured that the buffer pointed to is big enough to + /// accommodate accessing any valid pixel as describes by the spec and the + /// strides. Use with caution! + OIIO_IB_DEPRECATE_RAW_PTR void reset(const ImageSpec& spec, void* buffer, stride_t xstride = AutoStride, stride_t ystride = AutoStride, stride_t zstride = AutoStride); diff --git a/src/include/OpenImageIO/imagebufalgo_opencv.h b/src/include/OpenImageIO/imagebufalgo_opencv.h index 334bbd91b8..56b0e349a4 100644 --- a/src/include/OpenImageIO/imagebufalgo_opencv.h +++ b/src/include/OpenImageIO/imagebufalgo_opencv.h @@ -218,9 +218,11 @@ ImageBufAlgo::to_OpenCV(cv::Mat& dst, const ImageBuf& src, ROI roi, size_t pixelsize = dstSpecFormat.size() * chans; size_t linestep = pixelsize * roi.width(); + size_t bufsize = linestep * roi.height(); // Make an IB that wraps the OpenCV buffer, then IBA:: copy to it ImageBuf cvib(ImageSpec(roi.width(), roi.height(), chans, dstSpecFormat), - dst.ptr(), pixelsize, linestep, AutoStride); + make_span((std::byte*)dst.ptr(), bufsize), nullptr, pixelsize, + linestep); bool converted = ImageBufAlgo::copy(cvib, src); if (!converted) { OIIO::errorfmt( diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 25a4c5b94e..45f9ee105c 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -90,7 +90,8 @@ class ImageBufImpl { public: ImageBufImpl(string_view filename, int subimage, int miplevel, std::shared_ptr imagecache = {}, - const ImageSpec* spec = nullptr, void* buffer = nullptr, + const ImageSpec* spec = nullptr, span bufspan = {}, + const void* buforigin = nullptr, bool readonly = false, const ImageSpec* config = nullptr, Filesystem::IOProxy* ioproxy = nullptr, stride_t xstride = AutoStride, stride_t ystride = AutoStride, @@ -106,9 +107,10 @@ class ImageBufImpl { // supplied, use it for the "native" spec, otherwise make the nativespec // just copy the regular spec. void reset(string_view name, const ImageSpec& spec, - const ImageSpec* nativespec = nullptr, void* buffer = nullptr, - stride_t xstride = AutoStride, stride_t ystride = AutoStride, - stride_t zstride = AutoStride); + const ImageSpec* nativespec = nullptr, + span bufspan = {}, const void* buforigin = {}, + bool readonly = false, stride_t xstride = AutoStride, + stride_t ystride = AutoStride, stride_t zstride = AutoStride); void alloc(const ImageSpec& spec, const ImageSpec* nativespec = nullptr); void realloc(); bool init_spec(string_view filename, int subimage, int miplevel, @@ -120,6 +122,11 @@ class ImageBufImpl { DoLock do_lock = DoLock(true)); void copy_metadata(const ImageBufImpl& src); + // At least one of bufspan or buforigin is supplied. Set this->m_bufspan + // and this->m_localpixels appropriately. + void set_bufspan_localpixels(span bufspan, + const void* buforigin); + // Note: Uses std::format syntax template void error(const char* fmt, const Args&... args) const @@ -286,11 +293,13 @@ class ImageBufImpl { ImageSpec m_nativespec; ///< Describes the true native image std::unique_ptr m_pixels; ///< Pixel data, if local and we own it char* m_localpixels; ///< Pointer to local pixels + span m_bufspan; ///< Bounded buffer for local pixels typedef std::recursive_mutex mutex_t; typedef std::unique_lock lock_t; mutable mutex_t m_mutex; ///< Thread safety for this ImageBuf mutable bool m_spec_valid; ///< Is the spec valid mutable bool m_pixels_valid; ///< Image is valid + bool m_readonly; ///< The bufspan is read-only bool m_badfile; ///< File not found float m_pixelaspect; ///< Pixel aspect ratio of the image stride_t m_xstride; @@ -349,7 +358,8 @@ ImageBuf::impl_deleter(ImageBufImpl* todel) ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, std::shared_ptr imagecache, - const ImageSpec* spec, void* buffer, + const ImageSpec* spec, span bufspan, + const void* buforigin, bool readonly, const ImageSpec* config, Filesystem::IOProxy* ioproxy, stride_t xstride, stride_t ystride, stride_t zstride) @@ -363,6 +373,7 @@ ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, , m_localpixels(NULL) , m_spec_valid(false) , m_pixels_valid(false) + , m_readonly(readonly) , m_badfile(false) , m_pixelaspect(1) , m_xstride(0) @@ -391,8 +402,8 @@ ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, OIIO_SIMD_MAX_SIZE_BYTES), 0); // NB make it big enough for SSE - if (buffer) { - m_localpixels = (char*)buffer; + if (buforigin || bufspan.size()) { + set_bufspan_localpixels(bufspan, buforigin); m_storage = ImageBuf::APPBUFFER; m_pixels_valid = true; } else { @@ -401,11 +412,13 @@ ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, m_spec_valid = true; } else if (filename.length() > 0) { // filename being nonempty means this ImageBuf refers to a file. - OIIO_DASSERT(buffer == nullptr); + OIIO_DASSERT(buforigin == nullptr); + OIIO_DASSERT(bufspan.empty()); reset(filename, subimage, miplevel, std::move(imagecache), config, ioproxy); } else { - OIIO_DASSERT(buffer == nullptr); + OIIO_DASSERT(buforigin == nullptr); + OIIO_DASSERT(bufspan.empty()); } eval_contiguous(); } @@ -423,6 +436,7 @@ ImageBufImpl::ImageBufImpl(const ImageBufImpl& src) , m_threads(src.m_threads) , m_spec(src.m_spec) , m_nativespec(src.m_nativespec) + , m_readonly(src.m_readonly) , m_badfile(src.m_badfile) , m_pixelaspect(src.m_pixelaspect) , m_xstride(src.m_xstride) @@ -449,14 +463,17 @@ ImageBufImpl::ImageBufImpl(const ImageBufImpl& src) if (m_storage == ImageBuf::APPBUFFER) { // Source just wrapped the client app's pixels, we do the same m_localpixels = src.m_localpixels; + m_bufspan = src.m_bufspan; } else { // We own our pixels -- copy from source new_pixels(src.m_spec.image_bytes(), src.m_pixels.get()); + // N.B. new_pixels will set m_bufspan } } else { // Source was cache-based or deep // nothing else to do m_localpixels = nullptr; + m_bufspan = span(); } if (m_localpixels || m_spec.deep) { // A copied ImageBuf is no longer a direct file reference, so clear @@ -509,7 +526,8 @@ ImageBuf::ImageBuf(string_view filename, int subimage, int miplevel, const ImageSpec* config, Filesystem::IOProxy* ioproxy) : m_impl(new ImageBufImpl(filename, subimage, miplevel, std::move(imagecache), nullptr /*spec*/, - nullptr /*buffer*/, config, ioproxy), + {} /*bufspan*/, nullptr /*buforigin*/, true, + config, ioproxy), &impl_deleter) { } @@ -520,6 +538,7 @@ ImageBuf::ImageBuf(const ImageSpec& spec, InitializePixels zero) : m_impl(new ImageBufImpl("", 0, 0, NULL, &spec), &impl_deleter) { m_impl->alloc(spec); + // N.B. alloc will set m_bufspan if (zero == InitializePixels::Yes && !deep()) ImageBufAlgo::zero(*this); } @@ -528,8 +547,36 @@ ImageBuf::ImageBuf(const ImageSpec& spec, InitializePixels zero) ImageBuf::ImageBuf(const ImageSpec& spec, void* buffer, stride_t xstride, stride_t ystride, stride_t zstride) - : m_impl(new ImageBufImpl("", 0, 0, NULL, &spec, buffer, nullptr, nullptr, - xstride, ystride, zstride), + : m_impl(new ImageBufImpl("", 0, 0, nullptr /*imagecache*/, &spec, + {} /*bufspan*/, buffer /*buforigin*/, + false /*readonly*/, nullptr /*config*/, + nullptr /*ioproxy*/, xstride, ystride, zstride), + &impl_deleter) +{ +} + + + +ImageBuf::ImageBuf(const ImageSpec& spec, cspan buffer, + void* buforigin, stride_t xstride, stride_t ystride, + stride_t zstride) + : m_impl(new ImageBufImpl("", 0, 0, nullptr /*imagecache*/, &spec, + make_span((std::byte*)buffer.data(), + buffer.size()), + buforigin, true /*readonly*/, nullptr /*config*/, + nullptr /*ioproxy*/, xstride, ystride, zstride), + &impl_deleter) +{ +} + + + +ImageBuf::ImageBuf(const ImageSpec& spec, span buffer, + void* buforigin, stride_t xstride, stride_t ystride, + stride_t zstride) + : m_impl(new ImageBufImpl("", 0, 0, NULL, &spec, buffer, buforigin, + false /*readonly*/, nullptr /*config*/, + nullptr /*ioproxy*/, xstride, ystride, zstride), &impl_deleter) { } @@ -579,6 +626,8 @@ ImageBufImpl::new_pixels(size_t size, const void* data) free_pixels(); try { m_pixels.reset(size ? new char[size] : nullptr); + // Set bufspan to the allocated memory + m_bufspan = { reinterpret_cast(m_pixels.get()), size }; } catch (const std::exception& e) { // Could not allocate enough memory. So don't allocate anything, // consider this an uninitialized ImageBuf, issue an error, and hope @@ -587,7 +636,8 @@ ImageBufImpl::new_pixels(size_t size, const void* data) OIIO::debugfmt("ImageBuf unable to allocate {} bytes ({})\n", size, e.what()); error("ImageBuf unable to allocate {} bytes ({})\n", size, e.what()); - size = 0; + size = 0; + m_bufspan = {}; } m_allocated_size = size; pvt::IB_local_mem_current += m_allocated_size; @@ -757,7 +807,7 @@ ImageBufImpl::reset(string_view filename, int subimage, int miplevel, add_configspec(); m_configspec->attribute("oiio:ioproxy", TypeDesc::PTR, &m_rioproxy); } - + m_bufspan = {}; if (m_name.length() > 0) { // filename non-empty means this ImageBuf refers to a file. read(subimage, miplevel); @@ -779,9 +829,49 @@ ImageBuf::reset(string_view filename, int subimage, int miplevel, +void +ImageBufImpl::set_bufspan_localpixels(span bufspan, + const void* buforigin) +{ + if (bufspan.size() && !buforigin) { + buforigin = bufspan.data(); + } else if (buforigin && (!bufspan.data() || bufspan.empty())) { + // Need to figure out the span based on the origin and strides. + // Start with the span range of one pixel. + std::byte* bufstart = (std::byte*)buforigin; + std::byte* bufend = bufstart + m_spec.format.size(); + // Expand to the span range for one row. Remember negative strides! + if (m_xstride >= 0) { + bufend += m_xstride * (m_spec.width - 1); + } else { + bufstart -= m_xstride * (m_spec.width - 1); + } + // Expand to the span range for a whole image plane. + if (m_ystride >= 0) { + bufend += m_ystride * (m_spec.height - 1); + } else { + bufstart -= m_ystride * (m_spec.height - 1); + } + // Expand to the span range for a whole volume. + if (m_spec.depth > 1 && m_zstride != 0) { + if (m_zstride >= 0) { + bufend += m_zstride * (m_spec.depth - 1); + } else { + bufstart -= m_zstride * (m_spec.depth - 1); + } + } + bufspan = span { bufstart, size_t(bufend - bufstart) }; + } + m_bufspan = bufspan; + m_localpixels = (char*)buforigin; +} + + + void ImageBufImpl::reset(string_view filename, const ImageSpec& spec, - const ImageSpec* nativespec, void* buffer, stride_t xstride, + const ImageSpec* nativespec, span bufspan, + const void* buforigin, bool readonly, stride_t xstride, stride_t ystride, stride_t zstride) { clear(); @@ -794,24 +884,27 @@ ImageBufImpl::reset(string_view filename, const ImageSpec& spec, m_name = ustring(filename); m_current_subimage = 0; m_current_miplevel = 0; - if (buffer) { + if (buforigin || bufspan.size()) { m_spec = spec; m_nativespec = nativespec ? *nativespec : spec; m_channel_stride = stride_t(spec.format.size()); m_xstride = xstride; m_ystride = ystride; m_zstride = zstride; + m_readonly = readonly; ImageSpec::auto_stride(m_xstride, m_ystride, m_zstride, m_spec.format, m_spec.nchannels, m_spec.width, m_spec.height); m_blackpixel.resize(round_to_multiple(spec.pixel_bytes(), OIIO_SIMD_MAX_SIZE_BYTES), 0); - m_localpixels = (char*)buffer; + set_bufspan_localpixels(bufspan, buforigin); m_storage = ImageBuf::APPBUFFER; m_pixels_valid = true; } else { - m_storage = ImageBuf::LOCALBUFFER; + m_storage = ImageBuf::LOCALBUFFER; + m_readonly = false; alloc(spec); + // N.B. alloc sets m_bufspan } if (nativespec) m_nativespec = *nativespec; @@ -833,7 +926,30 @@ void ImageBuf::reset(const ImageSpec& spec, void* buffer, stride_t xstride, stride_t ystride, stride_t zstride) { - m_impl->reset("", spec, nullptr, buffer, xstride, ystride, zstride); + m_impl->reset("", spec, nullptr, {}, buffer, xstride, ystride, zstride); +} + + + +void +ImageBuf::reset(const ImageSpec& spec, cspan buffer, + const void* buforigin, stride_t xstride, stride_t ystride, + stride_t zstride) +{ + m_impl->reset("", spec, nullptr, + { const_cast(buffer.data()), buffer.size() }, + buforigin, true /*readonly*/, xstride, ystride, zstride); +} + + + +void +ImageBuf::reset(const ImageSpec& spec, span buffer, + const void* buforigin, stride_t xstride, stride_t ystride, + stride_t zstride) +{ + m_impl->reset("", spec, nullptr, buffer, buforigin, false /*readonly*/, + xstride, ystride, zstride); } @@ -842,6 +958,7 @@ void ImageBufImpl::realloc() { new_pixels(m_spec.deep ? size_t(0) : m_spec.image_bytes()); + // N.B. new_pixels will set m_bufspan m_channel_stride = m_spec.format.size(); m_xstride = AutoStride; m_ystride = AutoStride; @@ -859,6 +976,7 @@ ImageBufImpl::realloc() m_deepdata.init(m_spec); m_storage = ImageBuf::LOCALBUFFER; } + m_readonly = false; eval_contiguous(); #if 0 std::cerr << "ImageBuf " << m_name << " local allocation: " << m_allocated_size << "\n"; @@ -880,6 +998,7 @@ ImageBufImpl::alloc(const ImageSpec& spec, const ImageSpec* nativespec) m_nativespec = nativespec ? *nativespec : spec; realloc(); + // N.B. realloc sets m_bufspan m_spec_valid = true; } @@ -1152,6 +1271,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, else m_spec.format = m_nativespec.format; realloc(); + // N.B. realloc sets m_bufspan // If forcing a full read, make sure the spec reflects the nativespec's // tile sizes, rather than that imposed by the ImageCache. diff --git a/src/libOpenImageIO/imagebuf_test.cpp b/src/libOpenImageIO/imagebuf_test.cpp index 912a7ef6d6..d2673ea6dc 100644 --- a/src/libOpenImageIO/imagebuf_test.cpp +++ b/src/libOpenImageIO/imagebuf_test.cpp @@ -81,7 +81,7 @@ iterator_read_test() { { 0, 2, 8 }, { 1, 2, 9 }, { 2, 2, 10 }, { 3, 2, 11 } }, { { 0, 3, 12 }, { 1, 3, 13 }, { 2, 3, 14 }, { 3, 3, 15 } } }; ImageSpec spec(WIDTH, HEIGHT, CHANNELS, TypeDesc::FLOAT); - ImageBuf A(spec, buf); + ImageBuf A(spec, cspan(&buf[0][0][0], HEIGHT * WIDTH * CHANNELS)); ITERATOR p(A); OIIO_CHECK_EQUAL(p[0], 0.0f); @@ -134,7 +134,7 @@ iterator_wrap_test(ImageBuf::WrapMode wrap, std::string wrapname) { { 0, 2, 8 }, { 1, 2, 9 }, { 2, 2, 10 }, { 3, 2, 11 } }, { { 0, 3, 12 }, { 1, 3, 13 }, { 2, 3, 14 }, { 3, 3, 15 } } }; ImageSpec spec(WIDTH, HEIGHT, CHANNELS, TypeDesc::FLOAT); - ImageBuf A(spec, buf); + ImageBuf A(spec, cspan(&buf[0][0][0], HEIGHT * WIDTH * CHANNELS)); std::cout << "iterator_wrap_test " << wrapname << ":"; int i = 0; @@ -201,7 +201,7 @@ ImageBuf_test_appbuffer() }; // clang-format on ImageSpec spec(WIDTH, HEIGHT, CHANNELS, TypeDesc::FLOAT); - ImageBuf A(spec, buf); + ImageBuf A(spec, span(&buf[0][0][0], HEIGHT * WIDTH * CHANNELS)); // Make sure A now points to the buffer OIIO_CHECK_EQUAL((void*)A.pixeladdr(0, 0, 0), (void*)buf); @@ -246,7 +246,8 @@ ImageBuf_test_appbuffer_strided() memset(mem, 0, res * res * nchans * sizeof(float)); // Wrap the whole buffer, fill with green - ImageBuf wrapped(ImageSpec(res, res, nchans, TypeFloat), mem); + ImageBuf wrapped(ImageSpec(res, res, nchans, TypeFloat), + span(&mem[0][0][0], res * res * nchans)); const float green[nchans] = { 0.0f, 1.0f, 0.0f }; ImageBufAlgo::fill(wrapped, cspan(green)); float color[nchans] = { -1, -1, -1 }; @@ -256,7 +257,9 @@ ImageBuf_test_appbuffer_strided() // Do a strided wrap in the interior: a 3x3 image with extra spacing // between pixels and rows, and fill it with red. - ImageBuf strided(ImageSpec(3, 3, nchans, TypeFloat), &mem[4][4][0], + ImageBuf strided(ImageSpec(3, 3, nchans, TypeFloat), + span(&mem[0][0][0], res * res * nchans), + &mem[4][4][0], 2 * nchans * sizeof(float) /* every other pixel */, 2 * res * nchans * sizeof(float) /* ever other line */); const float red[nchans] = { 1.0f, 0.0f, 0.0f }; diff --git a/src/libOpenImageIO/imagebufalgo_draw.cpp b/src/libOpenImageIO/imagebufalgo_draw.cpp index f5f17d10c9..4508841504 100644 --- a/src/libOpenImageIO/imagebufalgo_draw.cpp +++ b/src/libOpenImageIO/imagebufalgo_draw.cpp @@ -719,7 +719,9 @@ const ImageBuf& ImageBufAlgo::bluenoise_image() { // This ImageBuf "wraps" the table. - static const ImageBuf img(bnspec(), pvt::bluenoise_table); + using namespace pvt; + static ImageBuf img(bnspec(), make_cspan(&bluenoise_table[0][0][0], + bntable_res * bntable_res * 4)); return img; } diff --git a/src/libOpenImageIO/imagespeed_test.cpp b/src/libOpenImageIO/imagespeed_test.cpp index d929023317..24b7bcd661 100644 --- a/src/libOpenImageIO/imagespeed_test.cpp +++ b/src/libOpenImageIO/imagespeed_test.cpp @@ -280,7 +280,7 @@ time_write_tiles_row_at_a_time() static void time_write_imagebuf() { - ImageBuf ib(bufspec, &buffer[0]); // wrap the buffer + ImageBuf ib(bufspec, span(buffer)); // wrap the buffer auto out = ImageOutput::create(output_filename); OIIO_ASSERT(out); bool ok = out->open(output_filename, outspec); diff --git a/src/libOpenImageIO/maketexture.cpp b/src/libOpenImageIO/maketexture.cpp index 5bf2da43c6..75b11c8aa5 100644 --- a/src/libOpenImageIO/maketexture.cpp +++ b/src/libOpenImageIO/maketexture.cpp @@ -1038,7 +1038,9 @@ make_texture_impl(ImageBufAlgo::MakeTextureMode mode, const ImageBuf* input, src.reset(new ImageBuf(*input)); } else { // Image buffer supplied that has pixels -- wrap it - src.reset(new ImageBuf(input->spec(), (void*)input->localpixels())); + src.reset(new ImageBuf(input->spec(), + make_cspan((std::byte*)input->localpixels(), + input->spec().image_bytes()))); } OIIO_DASSERT(src.get()); diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index e520aebf90..bc03cbece6 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -880,9 +880,10 @@ ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info, if (id.colortransformid() > 0) { // print("CONVERT id {} {},{} to cs {}\n", filename(), id.x(), id.y(), // id.colortransformid()); - ImageBuf wrapper(ImageSpec(spec.tile_width, spec.tile_height, - spec.nchannels, format), - data); + ImageSpec tilespec(spec.tile_width, spec.tile_height, + spec.nchannels, format); + ImageBuf wrapper(tilespec, make_cspan((const std::byte*)data, + tilespec.image_bytes())); ImageBufAlgo::colorconvert( wrapper, wrapper, ColorConfig::default_colorconfig().getColorSpaceNameByIndex( diff --git a/src/webp.imageio/webpinput.cpp b/src/webp.imageio/webpinput.cpp index 4948e101ef..f5c215eaac 100644 --- a/src/webp.imageio/webpinput.cpp +++ b/src/webp.imageio/webpinput.cpp @@ -308,14 +308,18 @@ WebpInput::read_current_subimage() m_spec.scanline_bytes()); // WebP requires unassociated alpha, and it's sRGB. // Handle this all by wrapping an IB around it. - ImageBuf fullbuf(m_spec, m_decoded_image.get()); + ImageBuf fullbuf(m_spec, + span((std::byte*)m_decoded_image.get(), + m_spec.image_bytes())); ImageBufAlgo::premult(fullbuf, fullbuf); } } else { // This subimage writes *atop* the prior image, we must composite ImageSpec fullspec(m_spec.width, m_spec.height, m_spec.nchannels, m_spec.format); - ImageBuf fullbuf(fullspec, m_decoded_image.get()); + ImageBuf fullbuf(fullspec, + span((std::byte*)m_decoded_image.get(), + fullspec.image_bytes())); ImageSpec fragspec(m_iter.width, m_iter.height, 4, TypeUInt8); fragspec.x = m_iter.x_offset; fragspec.y = m_iter.y_offset; diff --git a/src/webp.imageio/webpoutput.cpp b/src/webp.imageio/webpoutput.cpp index a5b936bec2..44ee385aab 100644 --- a/src/webp.imageio/webpoutput.cpp +++ b/src/webp.imageio/webpoutput.cpp @@ -139,15 +139,15 @@ WebpOutput::write_scanline(int y, int z, TypeDesc format, const void* data, // WebP requires unassociated alpha, and it's sRGB. // Handle this all by wrapping an IB around it. ImageSpec specwrap(m_spec.width, m_spec.height, 4, TypeUInt8); - ImageBuf bufwrap(specwrap, &m_uncompressed_image[0]); + ImageBuf bufwrap(specwrap, cspan(m_uncompressed_image)); ROI rgbroi(0, m_spec.width, 0, m_spec.height, 0, 1, 0, 3); ImageBufAlgo::pow(bufwrap, bufwrap, 2.2f, rgbroi); ImageBufAlgo::unpremult(bufwrap, bufwrap); ImageBufAlgo::pow(bufwrap, bufwrap, 1.0f / 2.2f, rgbroi); - WebPPictureImportRGBA(&m_webp_picture, &m_uncompressed_image[0], + WebPPictureImportRGBA(&m_webp_picture, m_uncompressed_image.data(), m_scanline_size); } else { - WebPPictureImportRGB(&m_webp_picture, &m_uncompressed_image[0], + WebPPictureImportRGB(&m_webp_picture, m_uncompressed_image.data(), m_scanline_size); } if (!WebPEncode(&m_webp_config, &m_webp_picture)) {