IBA::normalize() - Normalize 3D vectors (i.e., divide by their length) textures.

[ssh4net]

Description

Normalize 3D vectors (i.e., divide by their length) textures.

inCenter and outCenter define 0.0 coordinate in input and output
3D vectors encodings (e.g. 0.0f for [-1,1] or 0.5f for [0,1] encoding).

scale defines the scale factor to apply to the normalized vectors
(i.e. 1.0f for a full range normalization, 0.5f for 0.0-1.0 range encoding).

Default values are for [-1,1] encoding and full range normalization.

Functions required 3 or 4 channels. If destination have no alpha channel,
but source have, alpha channel will be dropped.

Tests

int main(int argc, const char* argv[])
{
    ImageBuf input(argv[1]);
    ImageBuf output;

    bool ok = ImageBufAlgo::normalize(output, input, 0.0f, 0.0f, 1.0f);
    if (ok)
    {
		ok = output.write(argv[2]);
    }
    else {
        std::cerr << "Could not normalize image: " << argv[1] << std::endl;
        return -1;
    }
    return 0;
}

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.

[lgritz]

The code looks great and is fine as-is. I made two minor suggestions for a more clear description comment and a very minor formatting fix. You should be able to merge those into what you have via the web interface here.

What I would like to request, though, is related to what is missing:

• The Python bindings for normalize(). This should go in py_imagebufalgo.cpp, and there should be plenty of examples to copy from in that file, so this should be very straightforward and only be a few lines of code. I definitely want that as part of this PR, we should never add a C++ function without also adding the Python wrapper.

• Tests. You could add a test in imagebufalgo_test.cpp, but if you add the python binding, you can just test python (in testsuite/python-imagebufalgo) and know that the python test will also exercise the C++ underneath.

• Expose a --normalize command in oiiotool. This is more work, and somewhat trickier if you aren't familiar with oiiotool internals. I wouldn't blame you if you wanted to skip this part and leave it to me or somebody else, or for you to do it separately as follow-up PR. (It should also include a test, probably in testsuite/oiiotool.) But if you want to try it as part of this PR, it would be much appreciated!

[ssh4net]

Actually, have zero understanding of how these python tests are working. Any examples?
I'm on windows and need explicitly define all variables otherwise python run.py don't work

[lgritz]

The run.py for each test is not executed directly, it is called by testsuite/runtest.py.

I took the liberty of helping out by making a few minor fixes:

• Fix the clang-format differences.
• Change your tests from saving as FLOAT to HALF (still a floating point type, but MUCH smaller files).
• Check the normalize output files into the ref/ subdirectory.

You can find this in the lgritz/normalize branch.

[ssh4net]

Sorry, can't find lgritz/normalize 😳

[lgritz]

I think this should work for you:

git checkout -b lg-normalize master
git pull https://github.com/lgritz/oiio normalize --rebase

[lgritz]

This looks good. The only think I can think of is that it should really have a test for the oiiotool part. But I'm also happy to tackle that (few lines) after the merge, if you just want to get what you've got in.

[ssh4net]

@lgritz if you can add this test lines, that will be great. I feel uncomfortable with code that I can't run and test myself to be sure that I got idea correctly.
For main normalize() function implementation I probably did all I can 😀

[lgritz]

Yeah, sounds good, @ssh4net. I will merge and then add the tests as a small follow-on PR.

Next time you submit a PR, try to create a new branch for it rather than using your master. I think use of master is what prevented me from being able to push amendments directly to your branch to include it in this PR directly.

[lgritz]

LGTM! Thanks for all the hard work on this.

I'll add the oiiotool test separately as a small follow-on PR.