Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: imagebufalgo.h mark, warn, or remove various deprecations #4344

Merged
merged 1 commit into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions docs/Deprecations-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ which only occur once every several years, we allow removal of functionality.
OpenImageIO v3.0 is a major release that includes removal of many
long-deprecated API facets. This document lists the deprecations and removals.

NOTE: This is an in-progress document. Some things currently only warning
about being deprecated will be removed in the final 3.0 release.

### Glossary

- "Marked as deprecated" means that we consider an API facet to be obsolete,
Expand All @@ -29,6 +32,7 @@ long-deprecated API facets. This document lists the deprecations and removals.

---

---

## array_view.h

Expand Down Expand Up @@ -82,6 +86,37 @@ long-deprecated API facets. This document lists the deprecations and removals.
deprecated since OIIO 1.5, now has deprecation warnings. Use
`interppixel_NDC()` instead.

## imagebufalgo.h

* The old versions (deprecated since 2.0) of IBA::compare() and
computePixelStats() that took a reference to the CompareResults or
PixelStats structure now have deprecation warnings. Use the versions that
return the structure instead.
* The deprecated (often since as far back as 2.0) versions of functions that
took raw pointers to color values now have deprecations warnings. Use the
versions that take `span<>` or `cspan<>` instead. These include versions of:
isConstantColor, isConstantChannel, isMonochrome, isConstantChannel,
colorconvert, fill, checker, add, sub, absdiff, mul, div, mad, pow,
channel_sum, channels, clamp, color_count, color_range_check, render_text.
* The `histogram()` function that takes a reference to a result vector
(deprecated since 2.0 and previously warned) has been removed. Use the
version that returns the vector instead. The `histogram_draw()` (deprecated
since 2.0 and previously warned) has been removed and has no replacement
since it was always silly.
* The versions of ociodisplay that lacked an `inverse` parameter, which were
marked as deprecated since 2.5, now have deprecation warnings. Use the
version that takes an `inverse` bool.
* The OpenCV-related functions that take old-style IplImage pointers
(deprecated since 2.0) have been removed. Use the modern ones that use
cv::Mat.
* The pre-KWArgs versions of resize, warp, and fit now have deprecation
warnings. Use the versions that take KWArgs instead.

## imagebufalgo_util.h

* IBA::type_merge, deprecated since 2.3, now has a deprecation warning.
Instead, use TypeDesc::basetype_merge().

## missingmath.h

* This header has been removed entirely. It has was originally needed for
Expand Down
338 changes: 166 additions & 172 deletions src/include/OpenImageIO/imagebufalgo.h

Large diffs are not rendered by default.

11 changes: 7 additions & 4 deletions src/include/OpenImageIO/imagebufalgo_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,15 +250,18 @@ enum IBAprep_flags {



// DEPRECATED(2.3): Prefer TypeDesc::basetype_merge().
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you remember why a comment was used for the deprecation long ago vs. just adding the real deprecated attribute back then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you remember why a comment was used for the deprecation long ago vs. just adding the real deprecated attribute back then?

Not specifically, but I can guess that it's one of two things:

  1. Since the usual progression is marked/considered deprecated -> warn as deprecated -> remove, spread over a few releases, sometimes at the first step I'll put the comment while my memory is clear, so when I revisit it a release or two later to enable the warning, I can just uncomment that line rather than have to do archeology to remind myself exactly when it happened or why. But sometimes I forget to come back after the next release and uncomment it.
  2. It may be that I really meant to enable the warnings at some prior time, but found that it was so widely used (within OIIO, or by OSL, or by other users) that it seemed too disruptive to add the warnings at that time and I postponed the task or re-commented it.

But with 3.0 coming soon, my aim really is to make sure that everything that previously warned will be removed, and everything previously considered deprecated gets at least a warning even if it's too inconvenient to remove quite yet. In particular, things that we only decided in 2.5 or 2.6/3.0 itself should be deprecated, I am hesitant to remove entirely with too short a transition. But I am warning and making them inline so that a removal later won't be an ABI break. I will do another pass at this right before 3.0 goes into beta, and "upgrade" a lot of the warnings to removals. But the warnings now will at least let people who do test builds against master to identify the fixes they need before we throw that final switch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need some sort of release checklist to go through once a year before bigger version bumps maybe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have one! (A recent addition.) But I think this is an item I may not have remembered to put on it.

TypeDesc::BASETYPE OIIO_API type_merge (TypeDesc::BASETYPE a, TypeDesc::BASETYPE b);
OIIO_DEPRECATED("Use TypeDesc::basetype_merge [2.3]")
inline TypeDesc::BASETYPE type_merge (TypeDesc::BASETYPE a, TypeDesc::BASETYPE b)
{
return TypeDesc::basetype_merge(a, b);
}

// DEPRECATED(2.3): Prefer TypeDesc::basetype_merge().
OIIO_DEPRECATED("Use TypeDesc::basetype_merge [2.3]")
inline TypeDesc type_merge (TypeDesc a, TypeDesc b) {
return TypeDesc::basetype_merge(a, b);
}

// DEPRECATED(2.3): Prefer TypeDesc::basetype_merge().
OIIO_DEPRECATED("Use TypeDesc::basetype_merge [2.3]")
inline TypeDesc type_merge (TypeDesc a, TypeDesc b, TypeDesc c)
{
return TypeDesc::basetype_merge(TypeDesc::basetype_merge(a,b), c);
Expand Down
2 changes: 2 additions & 0 deletions src/include/OpenImageIO/paramlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,8 @@ class OIIO_UTIL_API ParamValueSpan : public cspan<ParamValue> {
// - operator[int]
// - begin(), end(), cbegin(), cend()

ParamValueSpan() = default;

ParamValueSpan(cspan<ParamValue> p)
: cspan<ParamValue>(p)
{
Expand Down
3 changes: 3 additions & 0 deletions src/include/OpenImageIO/typedesc.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ struct OIIO_UTIL_API TypeDesc {
/// guess for one that can handle both without any loss of range or
/// precision.
static BASETYPE basetype_merge(TypeDesc a, TypeDesc b);
static BASETYPE basetype_merge(TypeDesc a, TypeDesc b, TypeDesc c) {
return basetype_merge(basetype_merge(a, b), c);
}

#if OIIO_DISABLE_DEPRECATED < OIIO_MAKE_VERSION(1,8,0) && OIIO_VERSION_LESS(2,7,0) && !defined(OIIO_DOXYGEN)
// DEPRECATED(1.8): These static const member functions were mildly
Expand Down
27 changes: 0 additions & 27 deletions src/libOpenImageIO/color_ocio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2649,33 +2649,6 @@ ImageBufAlgo::ociodisplay(const ImageBuf& src, string_view display,



// DEPRECATED(2.5)
bool
ImageBufAlgo::ociodisplay(ImageBuf& dst, const ImageBuf& src,
string_view display, string_view view,
string_view from, string_view looks, bool unpremult,
string_view key, string_view value,
const ColorConfig* colorconfig, ROI roi, int nthreads)
{
return ociodisplay(dst, src, display, view, from, looks, unpremult, false,
key, value, colorconfig, roi, nthreads);
}



// DEPRECATED(2.5)
ImageBuf
ImageBufAlgo::ociodisplay(const ImageBuf& src, string_view display,
string_view view, string_view from, string_view looks,
bool unpremult, string_view key, string_view value,
const ColorConfig* colorconfig, ROI roi, int nthreads)
{
return ociodisplay(src, display, view, from, looks, unpremult, false, key,
value, colorconfig, roi, nthreads);
}



bool
ImageBufAlgo::ociofiletransform(ImageBuf& dst, const ImageBuf& src,
string_view name, bool unpremult, bool inverse,
Expand Down
6 changes: 4 additions & 2 deletions src/libOpenImageIO/compute_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,10 @@ main(int argc, char* argv[])
float green[3] = { 0, 1, 0 };
float blue[3] = { 0, 0, 1 };
float black[3] = { 0, 0, 0 };
ImageBufAlgo::fill(imgA, red, green, red, green);
ImageBufAlgo::fill(imgB, blue, blue, black, black);
ImageBufAlgo::fill(imgA, cspan<float>(red), cspan<float>(green),
cspan<float>(red), cspan<float>(green));
ImageBufAlgo::fill(imgB, cspan<float>(blue), cspan<float>(blue),
cspan<float>(black), cspan<float>(black));
// imgA.write ("A.exr");
// imgB.write ("B.exr");

Expand Down
6 changes: 3 additions & 3 deletions src/libOpenImageIO/imagebuf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ ImageBuf_test_appbuffer_strided()
// Wrap the whole buffer, fill with green
ImageBuf wrapped(ImageSpec(res, res, nchans, TypeFloat), mem);
const float green[nchans] = { 0.0f, 1.0f, 0.0f };
ImageBufAlgo::fill(wrapped, green);
ImageBufAlgo::fill(wrapped, cspan<float>(green));
float color[nchans] = { -1, -1, -1 };
OIIO_CHECK_ASSERT(ImageBufAlgo::isConstantColor(wrapped, 0.0f, color)
&& color[0] == 0.0f && color[1] == 1.0f
Expand All @@ -260,7 +260,7 @@ ImageBuf_test_appbuffer_strided()
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 };
ImageBufAlgo::fill(strided, red);
ImageBufAlgo::fill(strided, cspan<float>(red));

// The strided IB ought to look all-red
OIIO_CHECK_ASSERT(ImageBufAlgo::isConstantColor(strided, 0.0f, color)
Expand Down Expand Up @@ -409,7 +409,7 @@ test_read_channel_subset()
// FIrst, write a test image with 6 channels
static float color6[] = { 0.6f, 0.5f, 0.4f, 0.3f, 0.2f, 0.1f };
ImageBuf A(ImageSpec(2, 2, 6, TypeDesc::FLOAT));
ImageBufAlgo::fill(A, color6);
ImageBufAlgo::fill(A, cspan<float>(color6));
A.write("sixchans.tif");
std::cout << " Start with image:\n";
print(A);
Expand Down
14 changes: 3 additions & 11 deletions src/libOpenImageIO/imagebufalgo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -761,15 +761,6 @@ ImageBufAlgo::perpixel_op(const ImageBuf& srcA, const ImageBuf& srcB,



// DEPRECATED(2.3): Replaced by TypeDesc::type_merge(BASETYPE,BASETYPE)
TypeDesc::BASETYPE
ImageBufAlgo::type_merge(TypeDesc::BASETYPE a, TypeDesc::BASETYPE b)
{
return TypeDesc::basetype_merge(a, b);
}



template<typename DSTTYPE, typename SRCTYPE>
static bool
convolve_(ImageBuf& dst, const ImageBuf& src, const ImageBuf& kernel,
Expand Down Expand Up @@ -1618,7 +1609,8 @@ ImageBufAlgo::fillholes_pushpull(ImageBuf& dst, const ImageBuf& src, ROI roi,
ImageSpec smallspec(w, h, src.nchannels(), TypeDesc::FLOAT);
smallspec.alpha_channel = topspec.alpha_channel;
ImageBuf* small = new ImageBuf(smallspec);
ImageBufAlgo::resize(*small, *pyramid.back(), "triangle");
ImageBufAlgo::resize(*small, *pyramid.back(),
{ { "filtername", "triangle" } });
divide_by_alpha(*small, get_roi(smallspec), nthreads);
pyramid.emplace_back(small);
// small->write(Strutil::fmt::format("push{:04d}.exr", small->spec().width));
Expand All @@ -1632,7 +1624,7 @@ ImageBufAlgo::fillholes_pushpull(ImageBuf& dst, const ImageBuf& src, ROI roi,
for (int i = (int)pyramid.size() - 2; i >= 0; --i) {
ImageBuf &big(*pyramid[i]), &small(*pyramid[i + 1]);
ImageBuf blowup(big.spec());
ImageBufAlgo::resize(blowup, small, "triangle");
ImageBufAlgo::resize(blowup, small, { { "filtername", "triangle" } });
ImageBufAlgo::over(big, big, blowup);
// big.write(Strutil::sprintf("pull{:04}.exr", big.spec().width));
}
Expand Down
Loading
Loading