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

[2058] Implement Directory Argument Loading for iv #4010

Conversation

CheeksTheGeek
Copy link
Collaborator

@CheeksTheGeek CheeksTheGeek commented Oct 10, 2023

Solves the issue "Load a directory: iv dirname ought to be the same as specifying all the image files in that directory" from the Great Big iv Wish List #2058

Description

This PR addresses the request from the Master list of iv feature requests to allow loading a directory as an argument, which in turn loads all images within the specified directory. The code modifications include checks to validate the file or directory existence, and if a directory is provided, iterates through its files to load all valid images into iv. This enhancement simplifies the process of viewing multiple images collectively located in a single directory.

Tests

N/A

Checklist:

  • I have read the contribution guidelines.
  • 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).
  • If I added or modified a C++ API call, I have also amended the
    corresponding Python bindings (and if altering ImageBufAlgo functions, also
    exposed the new functionality as oiiotool options).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 10, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

src/iv/ivmain.cpp Outdated Show resolved Hide resolved
@CheeksTheGeek
Copy link
Collaborator Author

CheeksTheGeek commented Oct 12, 2023

I've added a processing area for the extensions since they're formatted like openexr:exr,sxr,mxr;tiff:tif,tiff,tx,env,sm,vsm;jpeg:jpg,jpe,jpeg......., and then inside the looping for the directory members, it matches it via a vector.
The first loop across the files handles filling in the validImages vector, and the second one which only runs if validImages is not empty.

Also, just saw the message on slack to sign my commits, so this next commit is just a signing commit

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

@CheeksTheGeek ALL the commits need to be signed. (Silly, yes, but that's what the bot is looking for.)

The easiest way to do this is

git rebase -i HEAD~5     # (replace 5 with whatever number of commits you have)

This will bring up an editor window with the list of commits. Change "pick" to "r" on every line and save & exit. It will then bring up an editor on each commit in sequence, allowing you to edit the commit messages individually. For each one, just add this line:

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>

and save & exit. When this is all done, git push origin feature/iv-directory-as-arg --force

…d to the viewer

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
@CheeksTheGeek
Copy link
Collaborator Author

CheeksTheGeek commented Oct 12, 2023

Done!
Turns out VSCode has a great UI for Rebasing, and I just set vscode as my editor for git stuff.
git config --global core.editor "code-insiders --wait"

src/iv/ivmain.cpp Outdated Show resolved Hide resolved
@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

I build and tested this on my end -- works as advertised, very nice!

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
@CheeksTheGeek
Copy link
Collaborator Author

Done!

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

Works very well!

There is one small behavior that is surprising. I guess when you enumerate directory entries, it comes out in any random order (I suppose in the order the files within the directory appears to the OS), and that's a little surprising because if you'd said iv dir/*.exr you would have gotten them in alphabetical order. This will be especially crucial if, for example, your directory has all the frames of a shot (dir/foo.0001.exr, dir/foo.0002.exr, etc.).

I wonder if you would mind putting in an extra step so that the files within each directory are sorted lexicographically? (But only within each directory, not across all files and directories, if you know what I mean.)

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
@CheeksTheGeek
Copy link
Collaborator Author

Done!

src/iv/ivmain.cpp Outdated Show resolved Hide resolved
@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

Thank you, the sorting definitely is better behavior.

The only other request I had is about those blank line, but otherwise I think this is basically complete!

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM!

@lgritz lgritz merged commit 8d04dae into AcademySoftwareFoundation:master Oct 13, 2023
24 of 25 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Oct 14, 2023
…eFoundation#4010)

Solves the issue "Load a directory: iv dirname ought to be the same as
specifying all the image files in that directory" from the Great Big iv
Wish List AcademySoftwareFoundation#2058

This PR addresses the request from the Master list of iv feature
requests to allow loading a directory as an argument, which in turn
loads all images within the specified directory. The code modifications
include checks to validate the file or directory existence, and if a
directory is provided, iterates through its files to load all valid
images into iv. This enhancement simplifies the process of viewing
multiple images collectively located in a single directory.

---------

Signed-off-by: Chaitanya Sharma <39400946+CheeksTheGeek@users.noreply.github.com>
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.

None yet

2 participants