Skip to content

fix!: int pixel to int64_t in deepdata #5252

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.

Sign up for GitHub

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

Open
luna-y-kim wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −2
Open

fix!: int pixel to int64_t in deepdata #5252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/include/OpenImageIO/deepdata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ class OIIO_API DeepData {
/// Merge the samples of `src`'s pixel into this `DeepData`'s pixel.
/// Return `true` if ok, `false` if the operation could not be
/// performed.
void merge_deep_pixels(int64_t pixel, const DeepData& src, int srcpixel);
void merge_deep_pixels(int64_t pixel, const DeepData& src,
int64_t srcpixel);
Comment on lines +191 to +192

@lgritz lgritz Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you avoid the ABI break by just not removing the old one at all? Just add the new one, and change the implementation of the old one to call the new one. Leave a comment on the old one indicating that we intend to deprecate it. So the idiom is like this:

    /// full comments
    void func(int64_t);

    // DEPRECATED(3.2): use the version with int64_t
    void func(int);

does that work? Or do you just get warnings everywhere about ambiguous arguments?

@lgritz lgritz Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wait, I think this is even better:

    /// full comments
    void func(int64_t);

#ifdef OIIO_INTERNAL
    // DEPRECATED(3.2): use the version with int64_t
    void func(int);
#endif

This hides the declaration from external/downstream clients of OIIO when they recompile. They will only see the int64_t version, which if they pass an int32, will just transparently accept it. So starting from their next recompile, they will start using the new function, without having to do anything.

Meanwhile, internally we still declare and implement the old function. Which means that ABI is preserved, the library exports all the same symbols it used to, and an existing program that calls the old function will continue to link properly against a new copy of libOpenImageIO.

When we eventually have OIIO 4.0, we'll chase down all those "DEPRECATED" comments and finally remove the functions entirely (because we don't guarantee any compatibility at first-digit version changes).

@lgritz lgritz Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(I should point out that it's only because the change is specifically int -> int64_t, which is a widening that is implicitly accepted by the compiler without warning, that we are able to do this trick and something like func(my_int) will just work and know to call func(int64_t). If we were changing int to uint64_t, or doing some other change that wouldn't just implicitly cast, we'd be in a more complex situation.)


/// Return the z depth at which the pixel reaches full opacity.
float opaque_z(int64_t pixel) const;
Expand Down
3 changes: 2 additions & 1 deletion src/libOpenImageIO/deepdata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,7 +1176,8 @@ DeepData::merge_overlaps(int64_t pixel)


void
DeepData::merge_deep_pixels(int64_t pixel, const DeepData& src, int srcpixel)
DeepData::merge_deep_pixels(int64_t pixel, const DeepData& src,
int64_t srcpixel)
{
int srcsamples = src.samples(srcpixel);
if (srcsamples == 0)
Expand Down
Loading