Skip to content

Conversation

@lgritz
Copy link
Collaborator

@lgritz lgritz commented Nov 1, 2024

These commands, which are intended to alter the top image on the stack, were each allocating and copying an image unnecessarily for each operation. By changing the implementation to the right calls that know it doesn't need to make a copy (already a feature I'd implemented in the OiiotoolOp helper class) greatly speeds them up.

@lgritz
Copy link
Collaborator Author

lgritz commented Nov 5, 2024

Comments?

@lgritz
Copy link
Collaborator Author

lgritz commented Nov 6, 2024

Can somebody review this? I'm trying to get out of the habit of merging my own patches without positive proof that somebody else's eyes have been on it.

@mugulmd
Copy link
Contributor

mugulmd commented Nov 6, 2024

Can somebody review this? I'm trying to get out of the habit of merging my own patches without positive proof that somebody else's eyes have been on it.

I can :)
I'm still a bit of a newby here so maybe we'd need another pair of eyes though, but I can definitely do a few tests by the end of the week.

@lgritz
Copy link
Collaborator Author

lgritz commented Nov 6, 2024

I'm less worried about the testing -- the CI ought to vouch for the correctness of the result -- but more just looking over the code to see if it seems reasonable. I just don't like the optics of merging my own PR without so much as a single comment from anybody else. We do it too often, but it's something that any healthy project should only do in dire emergencies.

@lgritz lgritz added image processing Related to ImageBufAlgo or other image processing topic. oiiotool oiiotool labels Nov 6, 2024
These commands, which are intended to alter the top image on the
stack, were each allocating and copying an image unnecessarily for
each operation. By changing the implementation to the right calls that
know it doesn't need to make a copy (already a feature I'd implemented
in the OiiotoolOp helper class) greatly speeds them up.

Signed-off-by: Larry Gritz <[email protected]>
Copy link
Contributor

@mugulmd mugulmd 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 badeba0 into AcademySoftwareFoundation:main Nov 12, 2024
28 checks passed
@lgritz lgritz deleted the lg-inplace branch November 13, 2024 21:10
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Nov 13, 2024
…oundation#4518)

These commands, which are intended to alter the top image on the stack,
were each allocating and copying an image unnecessarily for each
operation. By changing the implementation to the right calls that know
it doesn't need to make a copy (already a feature I'd implemented in the
OiiotoolOp helper class) greatly speeds them up.

---------

Signed-off-by: Larry Gritz <[email protected]>
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Nov 21, 2024
…oundation#4518)

These commands, which are intended to alter the top image on the stack,
were each allocating and copying an image unnecessarily for each
operation. By changing the implementation to the right calls that know
it doesn't need to make a copy (already a feature I'd implemented in the
OiiotoolOp helper class) greatly speeds them up.

---------

Signed-off-by: Larry Gritz <[email protected]>
scott-wilson pushed a commit to scott-wilson/OpenImageIO that referenced this pull request May 18, 2025
…oundation#4518)

These commands, which are intended to alter the top image on the stack,
were each allocating and copying an image unnecessarily for each
operation. By changing the implementation to the right calls that know
it doesn't need to make a copy (already a feature I'd implemented in the
OiiotoolOp helper class) greatly speeds them up.

---------

Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Scott Wilson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

image processing Related to ImageBufAlgo or other image processing topic. oiiotool oiiotool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants