Skip to content

Commit

Permalink
cleanup: Make span::size() return size_t
Browse files Browse the repository at this point in the history 
With 3.0 coming soon, it's time to break compatibility by switching
OIIO::span::size() to return an unsized value, matching C++20
std::span.

The history is that when we first added OIIO::span, the C++ draft
proposal for std::span included a signed size() result, but by the
time it got finalized into C++20, it was changed to unsigned to match
all the other std containers, but it was too late for us to switch
without breaking API compatibility rather seriously.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
@lgritz
lgritz committed Jul 9, 2024
1 parent 7c04af9 commit a9f60a0
Show file tree
Hide file tree
Showing 18 changed files with 103 additions and 105 deletions.
4 changes: 2 additions & 2 deletions src/dpx.imageio/libdpx/EndianSwap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ SwapBytes(T& value)

template<typename T>
void
SwapBuffer(T* buf, unsigned int len)
SwapBuffer(T* buf, size_t len)
{
OIIO::byteswap_span(OIIO::span<T>(buf, len));
OIIO::byteswap_span(OIIO::span<T>(buf, OIIO::span_size_t(len)));
}

#else
Expand Down
48 changes: 25 additions & 23 deletions src/include/OpenImageIO/imagebufalgo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1942,7 +1942,7 @@ bool OIIO_API colorconvert (span<float> color,
// DEPRECATED(2.1): Less safe version with raw pointer and length.
inline bool colorconvert (float *color, int nchannels,
const ColorProcessor *processor, bool unpremult) {
return colorconvert ({color,nchannels}, processor, unpremult);
return colorconvert ({color,span_size_t(nchannels)}, processor, unpremult);
}

/// @}
Expand Down Expand Up @@ -2546,21 +2546,21 @@ bool OIIO_API deep_holdout (ImageBuf &dst, const ImageBuf &src,
inline bool fill (ImageBuf &dst, const float *values,
ROI roi={}, int nthreads=0) {
int nc (roi.defined() ? roi.nchannels() : dst.nchannels());
return fill (dst, {values, nc}, roi, nthreads);
return fill (dst, {values, span_size_t(nc)}, roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool fill (ImageBuf &dst, const float *top, const float *bottom,
ROI roi={}, int nthreads=0) {
int nc (roi.defined() ? roi.nchannels() : dst.nchannels());
return fill (dst, {top, nc}, {bottom, nc}, roi, nthreads);
return fill (dst, {top, span_size_t(nc)}, {bottom, span_size_t(nc)}, roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool fill (ImageBuf &dst, const float *topleft, const float *topright,
const float *bottomleft, const float *bottomright,
ROI roi={}, int nthreads=0) {
int nc (roi.defined() ? roi.nchannels() : dst.nchannels());
return fill (dst, {topleft, nc}, {topright, nc}, {bottomleft, nc},
{bottomright, nc}, roi, nthreads);
return fill (dst, {topleft, span_size_t(nc)}, {topright, span_size_t(nc)}, {bottomleft, span_size_t(nc)},
{bottomright, span_size_t(nc)}, roi, nthreads);
}

//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
Expand All @@ -2569,7 +2569,8 @@ inline bool checker (ImageBuf &dst, int width, int height, int depth,
int xoffset=0, int yoffset=0, int zoffset=0,
ROI roi={}, int nthreads=0) {
int nc (roi.defined() ? roi.nchannels() : dst.nchannels());
return checker (dst, width, height, depth, {color1,nc}, {color2,nc},
return checker (dst, width, height, depth,
{color1,span_size_t(nc)}, {color2,span_size_t(nc)},
xoffset, yoffset, zoffset, roi, nthreads);
}

Expand All @@ -2586,22 +2587,22 @@ inline bool sub (ImageBuf &dst, const ImageBuf &A, const float *B,
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool absdiff (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return absdiff (dst, A, cspan<float>(B,A.nchannels()), roi, nthreads);
return absdiff (dst, A, cspan<float>(B,span_size_t(A.nchannels())), roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool mul (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return mul (dst, A, {B, A.nchannels()}, roi, nthreads);
return mul (dst, A, {B, int(A.nchannels())}, roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool div (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return div (dst, A, {B, A.nchannels()}, roi, nthreads);
return div (dst, A, {B, int(A.nchannels())}, roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool mad (ImageBuf &dst, const ImageBuf &A, const float *B,
const ImageBuf &C, ROI roi={}, int nthreads=0) {
return mad (dst, A, {B, A.nchannels()}, C, roi, nthreads);
return mad (dst, A, {B, int(A.nchannels())}, C, roi, nthreads);
}
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool mad (ImageBuf &dst, const ImageBuf &A, const ImageBuf &B,
Expand All @@ -2611,20 +2612,20 @@ inline bool mad (ImageBuf &dst, const ImageBuf &A, const ImageBuf &B,
//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool mad (ImageBuf &dst, const ImageBuf &A, const float *B,
const float *C, ROI roi={}, int nthreads=0) {
return mad (dst, A, {B, A.nchannels()}, {C, A.nchannels()}, roi, nthreads);
return mad (dst, A, {B, int(A.nchannels())}, {C, int(A.nchannels())}, roi, nthreads);
}

//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool pow (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return pow (dst, A, {B, A.nchannels()}, roi, nthreads);
return pow (dst, A, {B, span_size_t(A.nchannels())}, roi, nthreads);
}

//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool channel_sum (ImageBuf &dst, const ImageBuf &src,
const float *weights=nullptr, ROI roi={},
int nthreads=0) {
return channel_sum (dst, src, {weights, src.nchannels()},
return channel_sum (dst, src, {weights, span_size_t(src.nchannels())},
roi, nthreads);
}

Expand All @@ -2635,9 +2636,9 @@ inline bool channels (ImageBuf &dst, const ImageBuf &src,
const std::string *newchannelnames=nullptr,
bool shuffle_channel_names=false, int nthreads=0) {
return channels (dst, src, nchannels,
{ channelorder, channelorder?nchannels:0 },
{ channelvalues, channelvalues?nchannels:0 },
{ newchannelnames, newchannelnames?nchannels:0},
{ channelorder, span_size_t(channelorder?nchannels:0) },
{ channelvalues, span_size_t(channelvalues?nchannels:0) },
{ newchannelnames, span_size_t(newchannelnames?nchannels:0) },
shuffle_channel_names, nthreads);
}

Expand All @@ -2646,16 +2647,17 @@ inline bool clamp (ImageBuf &dst, const ImageBuf &src,
const float *min=nullptr, const float *max=nullptr,
bool clampalpha01 = false,
ROI roi={}, int nthreads=0) {
return clamp (dst, src, { min, min ? src.nchannels() : 0 },
{ max, max ? src.nchannels() : 0 }, clampalpha01,
return clamp (dst, src, { min, span_size_t(min ? src.nchannels() : 0) },
{ max, span_size_t(max ? src.nchannels() : 0) }, clampalpha01,
roi, nthreads);
}

//OIIO_DEPRECATED("use version that takes span<> instead of raw pointer (2.0)")
inline bool isConstantColor (const ImageBuf &src, float *color,
ROI roi={}, int nthreads=0) {
int nc = roi.defined() ? std::min(roi.chend,src.nchannels()) : src.nchannels();
return isConstantColor (src, {color, color ? nc : 0}, roi, nthreads);
return isConstantColor (src, {color, span_size_t(color ? nc : 0) },
roi, nthreads);
}

//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
Expand All @@ -2664,8 +2666,8 @@ inline bool color_count (const ImageBuf &src, imagesize_t *count,
const float *eps=nullptr,
ROI roi={}, int nthreads=0) {
return color_count (src, count, ncolors,
{ color, ncolors*src.nchannels() },
eps ? cspan<float>(eps,src.nchannels()) : cspan<float>(),
{ color, span_size_t(ncolors*src.nchannels()) },
eps ? cspan<float>(eps,span_size_t(src.nchannels())) : cspan<float>(),
roi, nthreads);
}

Expand All @@ -2675,7 +2677,7 @@ inline bool color_range_check (const ImageBuf &src, imagesize_t *lowcount,
const float *low, const float *high,
ROI roi={}, int nthreads=0) {
return color_range_check (src, lowcount, highcount, inrangecount,
{low,src.nchannels()}, {high,src.nchannels()},
{low,span_size_t(src.nchannels())}, {high,span_size_t(src.nchannels())},
roi, nthreads);
}

Expand All @@ -2684,7 +2686,7 @@ inline bool render_text (ImageBuf &dst, int x, int y, string_view text,
int fontsize, string_view fontname,
const float *textcolor) {
return render_text (dst, x, y, text, fontsize, fontname,
{textcolor, textcolor?dst.nchannels():0});
{textcolor, textcolor?span_size_t(dst.nchannels()):0});
}


Expand Down
6 changes: 3 additions & 3 deletions src/include/OpenImageIO/imagebufalgo_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,12 +588,12 @@ inline TypeDesc type_merge (TypeDesc a, TypeDesc b, TypeDesc c)
// were no entries at all. This is used in many IBA functions that take
// constant per-channel values.
#define IBA_FIX_PERCHAN_LEN(av,len,missing,zdef) \
if (av.size() < len) { \
if (std::ssize(av) < len) { \
int nc = len; \
float *vals = OIIO_ALLOCA(float, nc); \
for (int i = 0; i < nc; ++i) \
vals[i] = i < av.size() ? av[i] : (i ? vals[i-1] : zdef); \
av = cspan<float>(vals, nc); \
vals[i] = i < std::ssize(av) ? av[i] : (i ? vals[i-1] : zdef); \
av = cspan<float>(vals, span_size_t(nc)); \
}

// Default IBA_FIX_PERCHAN_LEN, with zdef=0.0 and missing = the last value
Expand Down
70 changes: 30 additions & 40 deletions src/include/OpenImageIO/span.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,26 @@

OIIO_NAMESPACE_BEGIN

// By default, our span::size() is a signed value, because we wrote this at
// a time that the draft of std::span said it should be signed. The final
// C++20 std::span ended up with an unsigned size, like all the other STL
// classes. We will eventually conform by switching, but not until we are at
// OIIO 3.0, allowing source code API-breaking incompatibilities. In the
// mean time, we allow a back door to experiment with standard conformance
// by pre-defining OIIO_SPAN_SIZE_IS_UNSIGNED=1.
#if OIIO_VERSION_GREATER_EQUAL(3,0,0)
// Our pre-3.0 implementation had span::size() as a signed value, because we
// wrote it at a time that the draft of std::span said it should be signed.
// The final C++20 std::span ended up with an unsigned size, like all the
// other STL classes. It took us until OIIO 3.0 (or the in-progress 2.6.3)
// before we were able to break compatibility by switching it to match
// std::span::size() returning a size_t.
#ifndef OIIO_SPAN_SIZE_IS_UNSIGNED
# define OIIO_SPAN_SIZE_IS_UNSIGNED
#endif

#ifdef OIIO_SPAN_SIZE_IS_UNSIGNED
using oiio_span_size_type = size_t;
#else
using oiio_span_size_type = ptrdiff_t;
#endif
using span_size_t = size_t;
using oiio_span_size_type = OIIO::span_size_t; // back-compat alias

OIIO_INLINE_CONSTEXPR oiio_span_size_type dynamic_extent = -1;
OIIO_INLINE_CONSTEXPR span_size_t dynamic_extent = -1;



/// `span<T>` is a non-owning, non-copying, non-allocating reference to a
/// contiguous array of T objects known length. A 'span` encapsulates both a
/// pointer and a length, and thus is a safer way of passing pointers around
/// contiguous array of T objects of known length. A 'span` encapsulates both
/// a pointer and a length, and thus is a safer way of passing pointers around
/// (because the function called knows how long the array is). A function
/// that might ordinarily take a `T*` and a length could instead just take a
/// `span<T>`.
Expand All @@ -70,17 +66,14 @@ OIIO_INLINE_CONSTEXPR oiio_span_size_type dynamic_extent = -1;
/// structure (unless you are really sure you know what you're doing).
///

template <typename T, oiio_span_size_type Extent = dynamic_extent>
template <typename T, span_size_t Extent = dynamic_extent>
class span {
static_assert (std::is_array<T>::value == false, "can't have span of an array");
public:
using element_type = T;
using value_type = typename std::remove_cv<T>::type;
using size_type = oiio_span_size_type;
using size_type = span_size_t;
using difference_type = ptrdiff_t;
#if OIIO_VERSION < OIIO_MAKE_VERSION(3,0,0)
using index_type = size_type; // DEPRECATED(3.0)
#endif
using pointer = element_type*;
using reference = element_type&;
using iterator = element_type*;
Expand All @@ -93,7 +86,7 @@ class span {
constexpr span () noexcept = default;

/// Copy constructor (copies the span pointer and length, NOT the data).
template<class U, oiio_span_size_type N>
template<class U, span_size_t N>
constexpr span (const span<U,N> &copy) noexcept
: m_data(copy.data()), m_size(copy.size()) { }
/// Copy constructor (copies the span pointer and length, NOT the data).
Expand Down Expand Up @@ -230,27 +223,27 @@ class span {


/// cspan<T> is a synonym for a non-mutable span<const T>.
template <typename T, oiio_span_size_type Extent = dynamic_extent>
template <typename T, span_size_t Extent = dynamic_extent>
using cspan = span<const T, Extent>;



/// Compare all elements of two spans for equality
template <class T, oiio_span_size_type X, class U, oiio_span_size_type Y>
template <class T, span_size_t X, class U, span_size_t Y>
constexpr bool operator== (span<T,X> l, span<U,Y> r) {
#if OIIO_CPLUSPLUS_VERSION >= 20
return std::equal (l.begin(), l.end(), r.begin(), r.end());
#else
auto lsize = l.size();
bool same = (lsize == r.size());
for (ptrdiff_t i = 0; same && i < lsize; ++i)
for (span_size_t i = 0; same && i < lsize; ++i)
same &= (l[i] == r[i]);
return same;
#endif
}

/// Compare all elements of two spans for inequality
template <class T, oiio_span_size_type X, class U, oiio_span_size_type Y>
template <class T, span_size_t X, class U, span_size_t Y>
constexpr bool operator!= (span<T,X> l, span<U,Y> r) {
return !(l == r);
}
Expand All @@ -261,18 +254,15 @@ constexpr bool operator!= (span<T,X> l, span<U,Y> r) {
/// array with known length and optionally non-default strides through the
/// data. A span_strided<T> is mutable (the values in the array may
/// be modified), whereas a span_strided<const T> is not mutable.
template <typename T, oiio_span_size_type Extent = dynamic_extent>
template <typename T, span_size_t Extent = dynamic_extent>
class span_strided {
static_assert (std::is_array<T>::value == false,
"can't have span_strided of an array");
public:
using element_type = T;
using value_type = typename std::remove_cv<T>::type;
using size_type = oiio_span_size_type;
using size_type = span_size_t;
using difference_type = ptrdiff_t;
#if OIIO_VERSION < OIIO_MAKE_VERSION(3,0,0)
using index_type = size_type; // DEPRECATED(3.0)
#endif
using stride_type = ptrdiff_t;
using pointer = element_type*;
using reference = element_type&;
Expand Down Expand Up @@ -356,25 +346,25 @@ class span_strided {


/// cspan_strided<T> is a synonym for a non-mutable span_strided<const T>.
template <typename T, oiio_span_size_type Extent = dynamic_extent>
template <typename T, span_size_t Extent = dynamic_extent>
using cspan_strided = span_strided<const T, Extent>;



/// Compare all elements of two spans for equality
template <class T, oiio_span_size_type X, class U, oiio_span_size_type Y>
template <class T, span_size_t X, class U, span_size_t Y>
constexpr bool operator== (span_strided<T,X> l, span_strided<U,Y> r) {
auto lsize = l.size();
if (lsize != r.size())
return false;
for (ptrdiff_t i = 0; i < lsize; ++i)
for (span_size_t i = 0; i < lsize; ++i)
if (l[i] != r[i])
return false;
return true;
}

/// Compare all elements of two spans for inequality
template <class T, oiio_span_size_type X, class U, oiio_span_size_type Y>
template <class T, span_size_t X, class U, span_size_t Y>
constexpr bool operator!= (span_strided<T,X> l, span_strided<U,Y> r) {
return !(l == r);
}
Expand All @@ -387,25 +377,25 @@ OIIO_NAMESPACE_END
// Declare std::size and std::ssize for our span.
namespace std {

template<class T, OIIO::oiio_span_size_type E = OIIO::dynamic_extent>
template<class T, OIIO::span_size_t E = OIIO::dynamic_extent>
constexpr size_t size(const OIIO::span<T, E>& c) {
return static_cast<size_t>(c.size());
}

template<class T, OIIO::oiio_span_size_type E = OIIO::dynamic_extent>
template<class T, OIIO::span_size_t E = OIIO::dynamic_extent>
constexpr size_t size(const OIIO::span_strided<T, E>& c) {
return static_cast<size_t>(c.size());
}


#if OIIO_CPLUSPLUS_VERSION < 20
// C++20 and beyond already have these declared.
template<class T, OIIO::oiio_span_size_type E = OIIO::dynamic_extent>
template<class T, OIIO::span_size_t E = OIIO::dynamic_extent>
constexpr ptrdiff_t ssize(const OIIO::span<T, E>& c) {
return static_cast<ptrdiff_t>(c.size());
}

template<class T, OIIO::oiio_span_size_type E = OIIO::dynamic_extent>
template<class T, OIIO::span_size_t E = OIIO::dynamic_extent>
constexpr ptrdiff_t ssize(const OIIO::span_strided<T, E>& c) {
return static_cast<ptrdiff_t>(c.size());
}
Expand All @@ -423,7 +413,7 @@ constexpr ptrdiff_t ssize(const OIIO::span_strided<T, E>& c) {

/// Custom fmtlib formatters for span/cspan types.
namespace fmt {
template<typename T, OIIO::oiio_span_size_type Extent>
template<typename T, OIIO::span_size_t Extent>
struct formatter<OIIO::span<T, Extent>>
: OIIO::pvt::index_formatter<OIIO::span<T, Extent>> {};
} // namespace fmt
4 changes: 2 additions & 2 deletions src/include/OpenImageIO/span_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ make_span(T (&arg)[N]) // span from C array of known length

template<typename T>
inline constexpr span<T>
make_span(T* data, oiio_span_size_type size) // span from ptr + size
make_span(T* data, span_size_t size) // span from ptr + size
{
return { data, size };
}
Expand All @@ -92,7 +92,7 @@ make_cspan(const T& arg) // cspan from a single value

template<typename T>
inline constexpr cspan<T>
make_cspan(const T* data, oiio_span_size_type size) // cspan from ptr + size
make_cspan(const T* data, span_size_t size) // cspan from ptr + size
{
return { data, size };
}
Expand Down
Loading

0 comments on commit a9f60a0

Please sign in to comment.