Skip to content

Commit b356152

Browse files
lgritzlgritz
authored
api!: add image_span versions of ImageOutput methods (AcademySoftwareFoundation#4727)
* ImageOutput methods that write scanlines, tiles, and images, which are the main customization points for format output implementations, are given additional methods that take `image_span` in place of raw pointers and strides. * Generally, for each, there is a templated version that takes `image_span<T>` that communicates both the memory area and the data type conversion requested, a "base case" that takes an explicit TypeDesc for the conversion type and a `image_view<std::byte>` giving the raw memory layout, and a `span<T>` convenience version for when there are contiguous strides. Note that when reading mixed channel data types in "native" mode (no type conversion, just leave the data in its original types), you have to use the std::byte image_span version, since the idea is not to do any format conversion, and there may not be a single type involved. * For now, the default implementations of these new ImageOutput methods are just wrappers that call the old pointer-based ones. One by one, over time, we can swap them, changing the format implementations to have a full implementation of the new bounded versions, and make their raw pointer versions call the wrappers. The raw pointer ones will be understood to be "unsafe", merely assuming that the pointers always refer to appropriately-sized memory areas. Meanwhile, the ones using spans and image_spans will, due to assertions in their implementations, make it easier to verify (at least in debug mode), that we never touch memory outside these bounds. --------- Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 946a9d3 commit b356152

File tree

11 files changed

+798
-88
lines changed

11 files changed

+798
-88
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
cmake_minimum_required (VERSION 3.18.2...4.0)
66

7-
set (OpenImageIO_VERSION "3.1.1.0")
7+
set (OpenImageIO_VERSION "3.1.2.0")
88
set (OpenImageIO_VERSION_OVERRIDE "" CACHE STRING
99
"Version override (use with caution)!")
1010
mark_as_advanced (OpenImageIO_VERSION_OVERRIDE)

‎src/include/OpenImageIO/image_span.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ template<typename T> using image1d_span = image_span<T, 2>;
365365
/// covering the same range of memory.
366366
template<typename T, size_t Rank>
367367
image_span<const std::byte>
368-
as_image_span_bytes(image_span<T, Rank> src) noexcept
368+
as_image_span_bytes(const image_span<T, Rank>& src) noexcept
369369
{
370370
return image_span<const std::byte>(
371371
reinterpret_cast<const std::byte*>(src.data()), src.nchannels(),
@@ -378,7 +378,7 @@ as_image_span_bytes(image_span<T, Rank> src) noexcept
378378
/// the same range of memory.
379379
template<typename T, size_t Rank>
380380
image_span<std::byte>
381-
as_image_span_writable_bytes(image_span<T, Rank> src) noexcept
381+
as_image_span_writable_bytes(const image_span<T, Rank>& src) noexcept
382382
{
383383
return image_span<std::byte>(reinterpret_cast<std::byte*>(src.data()),
384384
src.nchannels(), src.width(), src.height(),

‎src/include/OpenImageIO/imageio.h‎

Lines changed: 474 additions & 72 deletions
Large diffs are not rendered by default.

‎src/include/OpenImageIO/span.h‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ OIIO_NAMESPACE_BEGIN
4141
using span_size_t = size_t;
4242
using oiio_span_size_type = OIIO::span_size_t; // back-compat alias
4343

44-
inline constexpr span_size_t dynamic_extent = -1;
44+
inline constexpr span_size_t dynamic_extent
45+
= std::numeric_limits<span_size_t>::max();
4546

4647

4748

@@ -588,6 +589,18 @@ as_bytes_ref(const T& ref) noexcept
588589

589590

590591

592+
/// Copy the memory contents of `src` to `dst`. They must have the same
593+
/// total size.
594+
template<typename T, typename S>
595+
inline void
596+
spancpy(span<T> dst, span<S> src)
597+
{
598+
OIIO_DASSERT(dst.size_bytes() == src.size_bytes());
599+
memcpy(dst.data(), src.data(), src.size_bytes());
600+
}
601+
602+
603+
591604
/// Try to copy `n` items of type `T` from `src[srcoffset...]` to
592605
/// `dst[dstoffset...]`. Don't read or write outside the respective span
593606
/// boundaries. Return the number of items actually copied, which should be

‎src/include/OpenImageIO/typedesc.h‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,44 +394,60 @@ inline constexpr TypeDesc TypeUstringhash(TypeDesc::USTRINGHASH);
394394
///
395395
template<typename T> struct BaseTypeFromC {};
396396
template<> struct BaseTypeFromC<unsigned char> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT8; };
397+
template<> struct BaseTypeFromC<const unsigned char> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT8; };
397398
template<> struct BaseTypeFromC<char> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT8; };
399+
template<> struct BaseTypeFromC<const char> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT8; };
398400
template<> struct BaseTypeFromC<uint16_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT16; };
401+
template<> struct BaseTypeFromC<const uint16_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT16; };
399402
template<> struct BaseTypeFromC<int16_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT16; };
403+
template<> struct BaseTypeFromC<const int16_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT16; };
400404
template<> struct BaseTypeFromC<uint32_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT; };
405+
template<> struct BaseTypeFromC<const uint32_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT; };
401406
template<> struct BaseTypeFromC<int32_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT; };
407+
template<> struct BaseTypeFromC<const int32_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT; };
402408
template<> struct BaseTypeFromC<uint64_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT64; };
409+
template<> struct BaseTypeFromC<const uint64_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT64; };
403410
template<> struct BaseTypeFromC<int64_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT64; };
411+
template<> struct BaseTypeFromC<const int64_t> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT64; };
404412
#if defined(__GNUC__) && __WORDSIZE == 64 && !(defined(__APPLE__) && defined(__MACH__))
405413
// Some platforms consider int64_t and long long to be different types, even
406414
// though they are actually the same size.
407415
static_assert(!std::is_same_v<unsigned long long, uint64_t>);
408416
static_assert(!std::is_same_v<long long, int64_t>);
409417
template<> struct BaseTypeFromC<unsigned long long> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT64; };
418+
template<> struct BaseTypeFromC<const unsigned long long> { static constexpr TypeDesc::BASETYPE value = TypeDesc::UINT64; };
410419
template<> struct BaseTypeFromC<long long> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT64; };
420+
template<> struct BaseTypeFromC<const long long> { static constexpr TypeDesc::BASETYPE value = TypeDesc::INT64; };
411421
#endif
412422
#if defined(_HALF_H_) || defined(IMATH_HALF_H_)
413423
template<> struct BaseTypeFromC<half> { static constexpr TypeDesc::BASETYPE value = TypeDesc::HALF; };
424+
template<> struct BaseTypeFromC<const half> { static constexpr TypeDesc::BASETYPE value = TypeDesc::HALF; };
414425
#endif
415426
template<> struct BaseTypeFromC<float> { static constexpr TypeDesc::BASETYPE value = TypeDesc::FLOAT; };
427+
template<> struct BaseTypeFromC<const float> { static constexpr TypeDesc::BASETYPE value = TypeDesc::FLOAT; };
416428
template<> struct BaseTypeFromC<double> { static constexpr TypeDesc::BASETYPE value = TypeDesc::DOUBLE; };
417-
template<> struct BaseTypeFromC<const char*> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
429+
template<> struct BaseTypeFromC<const double> { static constexpr TypeDesc::BASETYPE value = TypeDesc::DOUBLE; };
418430
template<> struct BaseTypeFromC<char*> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
431+
template<> struct BaseTypeFromC<const char*> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
419432
template<> struct BaseTypeFromC<std::string> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
433+
template<> struct BaseTypeFromC<const std::string> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
420434
template<> struct BaseTypeFromC<string_view> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
435+
template<> struct BaseTypeFromC<const string_view> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
421436
class ustring;
422437
template<> struct BaseTypeFromC<ustring> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
438+
template<> struct BaseTypeFromC<const ustring> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
423439
template<size_t S> struct BaseTypeFromC<char[S]> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
424440
template<size_t S> struct BaseTypeFromC<const char[S]> { static constexpr TypeDesc::BASETYPE value = TypeDesc::STRING; };
425441
template<typename P> struct BaseTypeFromC<P*> { static constexpr TypeDesc::BASETYPE value = TypeDesc::PTR; };
426442

427443
/// `BaseTypeFromC_v<T>` is shorthand for `BaseTypeFromC<T>::value()`.
428444
template<typename T>
429-
constexpr TypeDesc::BASETYPE BaseTypeFromC_v = BaseTypeFromC<std::remove_cv_t<T>>::value();
445+
constexpr TypeDesc::BASETYPE BaseTypeFromC_v = BaseTypeFromC<std::remove_cv_t<T>>::value;
430446

431447
/// A template mechanism for getting the TypeDesc from a C type.
432448
/// The default for simple types is just the TypeDesc based on BaseTypeFromC.
433449
/// But we can specialize more complex types.
434-
template<typename T> struct TypeDescFromC { static const constexpr TypeDesc value() { return TypeDesc(BaseTypeFromC<T>::value); } };
450+
template<typename T> struct TypeDescFromC { static const constexpr TypeDesc value() { return TypeDesc(BaseTypeFromC_v<T>); } };
435451
template<> struct TypeDescFromC<int32_t> { static const constexpr TypeDesc value() { return TypeDesc::INT32; } };
436452
template<> struct TypeDescFromC<uint32_t> { static const constexpr TypeDesc value() { return TypeDesc::UINT32; } };
437453
template<> struct TypeDescFromC<int16_t> { static const constexpr TypeDesc value() { return TypeDesc::INT16; } };

‎src/include/imageio_pvt.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ get_default_quantize(TypeDesc format, long long& quant_min,
8989
/// which is either dst or src (if the strides indicated that data were
9090
/// already contiguous).
9191
OIIO_API span<const std::byte>
92-
contiguize(image_span<const std::byte> src, span<std::byte> dst);
92+
contiguize(const image_span<const std::byte>& src, span<std::byte> dst);
9393

9494
/// Turn potentially non-contiguous-stride data (e.g. "RGBxRGBx") into
9595
/// contiguous-stride ("RGBRGB"), for any format or stride values

‎src/libOpenImageIO/image_span_test.cpp‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,124 @@ test_image_span_convert_image()
349349

350350

351351

352+
// Sum all values in an image using a pass-by-value image_span
353+
float
354+
sum_image_span_val(image_span<const float> img)
355+
{
356+
float sum = 0;
357+
for (uint32_t z = 0; z < img.depth(); ++z) {
358+
for (uint32_t y = 0; y < img.height(); ++y) {
359+
for (uint32_t x = 0; x < img.width(); ++x) {
360+
for (uint32_t c = 0; c < img.nchannels(); ++c) {
361+
sum += img.get(c, x, y, z);
362+
}
363+
}
364+
}
365+
}
366+
return sum;
367+
}
368+
369+
370+
// Sum all values in an image using a pass-by-reference image_span
371+
float
372+
sum_image_span_ref(const image_span<const float>& img)
373+
{
374+
float sum = 0;
375+
for (uint32_t z = 0; z < img.depth(); ++z) {
376+
for (uint32_t y = 0; y < img.height(); ++y) {
377+
for (uint32_t x = 0; x < img.width(); ++x) {
378+
for (uint32_t c = 0; c < img.nchannels(); ++c) {
379+
sum += img.get(c, x, y, z);
380+
}
381+
}
382+
}
383+
}
384+
return sum;
385+
}
386+
387+
388+
// Sum all values in an image using raw pointers, sizes, strides
389+
float
390+
sum_image_span_ptr(const float* ptr, uint32_t chans, uint32_t width,
391+
uint32_t height, uint32_t depth, int64_t chstride,
392+
int64_t xstride, int64_t ystride, int64_t zstride)
393+
{
394+
float sum = 0;
395+
for (uint32_t z = 0; z < depth; ++z) {
396+
for (uint32_t y = 0; y < height; ++y) {
397+
for (uint32_t x = 0; x < width; ++x) {
398+
for (uint32_t c = 0; c < chans; ++c) {
399+
const float* p = reinterpret_cast<const float*>(
400+
(const char*)ptr + c * chstride + x * xstride
401+
+ y * ystride + z * zstride);
402+
sum += *p;
403+
}
404+
}
405+
}
406+
}
407+
return sum;
408+
}
409+
410+
411+
412+
void
413+
benchmark_image_span_passing()
414+
{
415+
print("\nbenchmark_image_span_passing\n");
416+
const int xres = 2048, yres = 1536, nchans = 4;
417+
std::vector<float> sbuf(xres * yres * nchans, 1.0f);
418+
image_span<const float> ispan(sbuf.data(), nchans, xres, yres, 1);
419+
420+
Benchmarker bench;
421+
bench.units(Benchmarker::Unit::us);
422+
float sum = 0.0f;
423+
424+
bench(" pass by value (big)",
425+
[=, &sum]() { sum += sum_image_span_val(ispan); });
426+
bench(" pass by value imm (big)", [=, &sum]() {
427+
sum += sum_image_span_val(
428+
image_span<const float>(sbuf.data(), nchans, xres, yres, 1));
429+
});
430+
bench(" pass by ref (big)",
431+
[=, &sum]() { sum += sum_image_span_ref(ispan); });
432+
bench(" pass by ref imm (big)", [=, &sum]() {
433+
sum += sum_image_span_ref(
434+
image_span<const float>(sbuf.data(), nchans, xres, yres, 1));
435+
});
436+
bench(" pass by ptr (big)", [=, &sum]() {
437+
sum += sum_image_span_ptr(sbuf.data(), nchans, xres, yres, 1,
438+
sizeof(float), nchans * sizeof(float),
439+
nchans * sizeof(float) * xres,
440+
nchans * sizeof(float) * xres * yres);
441+
});
442+
443+
// Do it all again for a SMALL image
444+
bench.units(Benchmarker::Unit::ns);
445+
int small = 16;
446+
image_span<const float> smispan(sbuf.data(), nchans, small, small, 1);
447+
bench(" pass by value (small)",
448+
[=, &sum]() { sum += sum_image_span_val(smispan); });
449+
bench(" pass by value imm (small)", [=, &sum]() {
450+
sum += sum_image_span_val(
451+
image_span<const float>(sbuf.data(), nchans, small, small, 1));
452+
});
453+
bench(" pass by ref (small)",
454+
[=, &sum]() { sum += sum_image_span_ref(smispan); });
455+
bench(" pass by ref imm (small)", [=, &sum]() {
456+
sum += sum_image_span_ref(
457+
image_span<const float>(sbuf.data(), nchans, small, small, 1));
458+
});
459+
bench(" pass by ptr (small)", [=, &sum]() {
460+
sum += sum_image_span_ptr(sbuf.data(), nchans, small, small, 1,
461+
sizeof(float), nchans * sizeof(float),
462+
nchans * sizeof(float) * small,
463+
nchans * sizeof(float) * small * small);
464+
});
465+
print(" [sum={}]\n", sum); // seems necessary to not optimize away
466+
}
467+
468+
469+
352470
int
353471
main(int /*argc*/, char* /*argv*/[])
354472
{
@@ -378,5 +496,7 @@ main(int /*argc*/, char* /*argv*/[])
378496
test_image_span_convert_image<uint16_t, half>();
379497
test_image_span_convert_image<half, uint16_t>();
380498

499+
benchmark_image_span_passing();
500+
381501
return unit_test_failures;
382502
}

‎src/libOpenImageIO/imageio.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ _contiguize(const T* src, int nchannels, stride_t xstride, stride_t ystride,
734734

735735

736736
span<const std::byte>
737-
pvt::contiguize(image_span<const std::byte> src, span<std::byte> dst)
737+
pvt::contiguize(const image_span<const std::byte>& src, span<std::byte> dst)
738738
{
739739
// Contiguized result must fit in dst
740740
OIIO_DASSERT(src.size_bytes() <= dst.size_bytes());
@@ -1053,7 +1053,8 @@ copy_image(int nchannels, int width, int height, int depth, const void* src,
10531053

10541054
template<typename T>
10551055
void
1056-
aligned_copy_image(image_span<std::byte> dst, image_span<const std::byte> src)
1056+
aligned_copy_image(const image_span<std::byte>& dst,
1057+
const image_span<const std::byte>& src)
10571058
{
10581059
size_t systride = src.ystride();
10591060
size_t dystride = dst.ystride();
@@ -1081,7 +1082,8 @@ aligned_copy_image(image_span<std::byte> dst, image_span<const std::byte> src)
10811082

10821083

10831084
bool
1084-
copy_image(image_span<std::byte> dst, image_span<const std::byte> src)
1085+
copy_image(const image_span<std::byte>& dst,
1086+
const image_span<const std::byte>& src)
10851087
{
10861088
OIIO_DASSERT(src.width() == dst.width() && src.height() == dst.height()
10871089
&& src.depth() == dst.depth()

0 commit comments

Comments
 (0)