feat(oiiotool): oiiotool --layersplit, new command to split layers #4591
Conversation
mugulmd
force-pushed
the
split-layers
branch
2 times, most recently
from
e3a6b92 to
9bacc12
Compare
…yers on the stack Signed-off-by: Loïc Vital <[email protected]>
Signed-off-by: Loïc Vital <[email protected]>
mugulmd
changed the title
|
Closes #4546 |
There was a problem hiding this comment.
The code LGTM, great job!
On the docs, I made one spelling and phrasing suggestion, as well as a question/suggestion aimed at clarifying what happens to channels that aren't part of a named layer. That's about all I can think of to make this PR better, it's really great.
Breaking out an entirely new test for this as you did is fine, though I wanted to let you know (in case you're inclined to change it) that it would have also been ok to add one more command line to an existing test, whichever of the oiiotool related tests looked like it contained the most related commands (maybe wherever the tests for --siappend and --sisplit are, so that related commands have their tests together).
|
One more FYI: When mentioning an issue in a PR, GitHub expects you to use a particular phrasing in order to truly link them in such a way that accepting the PR will automatically close the issue: or Anything else is just talk, and doesn't really link the issue. |
lgritz
changed the title
lgritz
added
enhancement
Signed-off-by: Loïc Vital <[email protected]>
|
Hey @lgritz |
…cademySoftwareFoundation#4591) Implement FR from AcademySoftwareFoundation#4546 Add a new `--layersplit` command to `oiiotool` to split an image into its channel-name-based layers onto the stack. The extracted layer names are then stored in the `oiio:subimagename` metadata (in case it is needed for later use, e.g merging these "layers" as sub-images), and the extracted channel names replace the old channel names in the new images. Example: an image with channels `R, G, B, A, diffuse.R, diffuse.G, diffuse.B` will be split into two images, the first one with channels `R, G, B, A` and the second one named `diffuse` with channels `R, G, B`. > Note: we did not implement the `--layertosi` command suggested in the FR as it can already be done with a combination of `--layersplit` and `--siappendall` We add a test in the test suite that takes a "multi-layer" image, splits the layers and merges them as sub-images. The final image should have 3 sub-images. --------- Signed-off-by: Loïc Vital <[email protected]>
Description
Implement FR from #4546
Add a new
--layersplitcommand tooiiotoolto split an image into its channel-name-based layers onto the stack.The extracted layer names are then stored in the
oiio:subimagenamemetadata (in case it is needed for later use, e.g merging these "layers" as sub-images), and the extracted channel names replace the old channel names in the new images.Example:
an image with channels
R, G, B, A, diffuse.R, diffuse.G, diffuse.Bwill be split into two images, the first one with channelsR, G, B, Aand the second one nameddiffusewith channelsR, G, B.Tests
We add a test in the test suite that takes a "multi-layer" image, splits the layers and merges them as sub-images. The final image should have 3 sub-images.
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
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.