Skip to content

Commit

Permalink
cleanup: Make span use unsized-agnostic
Browse files Browse the repository at this point in the history 
In preparation for switching `OIIO::span<>::size_type` from signed to
unsigned (to conform to C++20 std::span) that we intend to do before
releasing 3.0, this set of changes makes our internal code "unsigned
safe" by using appropriate casting in all the spots where warnings
will crop up once that switch is made.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
@lgritz
lgritz committed May 25, 2024
1 parent 83da4bb commit 5591696
Show file tree
Hide file tree
Showing 18 changed files with 104 additions and 87 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
52 changes: 27 additions & 25 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,39 +2569,40 @@ 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);
}

//OIIO_DEPRECATED("use version that takes cspan<> instead of raw pointer (2.0)")
inline bool add (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return add (dst, A, {B,A.nchannels()}, roi, nthreads);
return add (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 sub (ImageBuf &dst, const ImageBuf &A, const float *B,
ROI roi={}, int nthreads=0) {
return sub (dst, A, {B,A.nchannels()}, roi, nthreads);
return sub (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 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, span_size_t(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, span_size_t(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, span_size_t(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, span_size_t(A.nchannels())}, {C, span_size_t(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 @@ -492,12 +492,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
51 changes: 30 additions & 21 deletions src/include/OpenImageIO/span.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ OIIO_NAMESPACE_BEGIN

#define OIIO_SPAN_SIZE_IS_UNSIGNED
#ifdef OIIO_SPAN_SIZE_IS_UNSIGNED
using oiio_span_size_type = size_t;
using span_size_t = size_t;
#else
using oiio_span_size_type = ptrdiff_t;
using span_size_t = ptrdiff_t;
#endif
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;



Expand Down Expand Up @@ -71,13 +72,13 @@ 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)
Expand All @@ -94,7 +95,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 All @@ -104,6 +105,14 @@ class span {
constexpr span (pointer data, size_type size) noexcept
: m_data(data), m_size(size) { }

#ifdef OIIO_SPAN_SIZE_IS_UNSIGNED
/// Construct from T* and integer length, to avoid compatibility issues
/// with calls that assume the old int-based size_type after we have
/// switched to size_t-based size_type.
// constexpr span (pointer data, int size) noexcept
// : m_data(data), m_size(size_type(size)) { }
#endif

/// Construct from begin and end pointers.
constexpr span (pointer b, pointer e) noexcept
: m_data(b), m_size(e-b) { }
Expand Down Expand Up @@ -231,27 +240,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 @@ -262,14 +271,14 @@ 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)
Expand Down Expand Up @@ -357,25 +366,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 @@ -388,25 +397,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 @@ -424,7 +433,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 5591696

Please sign in to comment.