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

Update ImageInput docs to not use deprecated APIs #3907

Merged
merged 3 commits into from
Jul 15, 2023

Conversation

jessey-git
Copy link
Contributor

Description

Update the ImageInput documentation to not use the deprecated read_image APIs.

Based on the discussion in #3902

Tests

Checklist:

  • I have read the contribution guidelines.
  • If this is more extensive than a small change to existing code, I
    have previously submitted a Contributor License Agreement
    (individual, and if there is any way my
    employers might think my programming belongs to them, then also
    corporate).
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

@jessey-git
Copy link
Contributor Author

I tried to keep each sample relatively similar to each other and changed a few uses of non-standard dynamic arrays to just use vector.

I've never used DCO before. Do I just edit the above message with the special text or do I need to do something else before I use the special text?

@lgritz
Copy link
Collaborator

lgritz commented Jul 11, 2023

The easiest way to correct this is:

git commit --amend -s
git push --force origin deprecateddocs

The -s (which you could have used on any commit) appends the "Signed-off-by" line to your commit message.

@lgritz
Copy link
Collaborator

lgritz commented Jul 11, 2023

https://github.com/OpenImageIO/oiio/blob/master/CONTRIBUTING.md

Read the section about DCO signoff. There's also a tool you can use (per repo) that will automatically sign all your commits, if you are like me and will forget otherwise.

@lgritz
Copy link
Collaborator

lgritz commented Jul 11, 2023

This in particular: https://github.com/coderanger/dco

Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
@@ -360,10 +362,11 @@ flexible functionality. A few representative examples follow:

* Flip an image vertically upon reading, by using *negative* ``y`` stride::

unsigned char pixels[spec.width * spec.height * spec.nchannels];
std::vector<unsigned char> pixels(spec.width * spec.height * spec.nchannels);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as an FYI, if I were writing this for real, I think the best idiom would be:

auto pixels = std::unique_ptr<unsigned char[]>(new unsigned char [spec.width * spec.height * spec.nchannels]);

That would be freed upon scope exit (like std::vector), but it's much more efficient -- new[] doesn't initialize the memory, but std::vector will fill it with zeroes unnecessarily!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about I change them to the following. Seem ok?
auto pixels = std::make_unique<unsigned char[]>(spec.width * spec.height * spec.nchannels);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will that really work without the new?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, make_unique does work like that, however it looks like the semantics of make_unique include value initialization. After trying it out on compiler explorer the make_unique calls will include some form of memset basically (rep stosq)... c++ why are you the way you are... https://godbolt.org/z/9PEYKbY6f

I'll replace all of them with your suggestion then.

Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
@lgritz lgritz merged commit b1f5265 into AcademySoftwareFoundation:master Jul 15, 2023
1 check passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Jul 15, 2023
…areFoundation#3907)

Update the ImageInput documentation to not use the deprecated
`read_image` APIs.

Based on the discussion in AcademySoftwareFoundation#3902

---------

Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
@jessey-git jessey-git deleted the deprecateddocs branch August 25, 2023 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants