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

Implement support for Common LUT Format (CLF). #636

Open
KelSolaar opened this issue Oct 28, 2020 · 5 comments · May be fixed by #1310
Open

Implement support for Common LUT Format (CLF). #636

KelSolaar opened this issue Oct 28, 2020 · 5 comments · May be fixed by #1310

Comments

@KelSolaar
Copy link
Member

KelSolaar commented Oct 28, 2020

We have started work on that in multiple locations, especially in the forks from @zachlewis and @nick-shaw. As I'm a bit lost on where we are with that, an issue to centralise the effort is due because information gets lost on Slack.

References

@zachlewis
Copy link
Contributor

zachlewis commented Oct 29, 2020

especially in the forks from @zacklewis and @nick-shaw.

(@zachlewis, not @zacklewis :D)

Since Nick's last update, he's worked on implementing the Exponent ProcessNode as a LUTSequenceOperator; I've implemented the Log LUTSequenceOperator in my features/CLF branch, which is based off of Nick's work.

I've tried to make things as painless as possible to merge into @nick-shaw's features/clf_temp branch -- the commit history should be pretty clean / chronologically linear -- I did my best to implement the Log class that Nick had begun to stub out.

features/CLF was last rebased on the upstream develop branch a year ago. I've cherrypicked a couple of commits implementing @njwardhan's logarithmic_function_camera, logarithmic_function_quasilog, and logarithmic_function_basic methods.

Notes on the Log stuff:

In addition to the Log operator itself, I've included methods for un/structuring to and from CLF based on what Nick's been doing for the other nodes -- but I haven't done any actual testing with that code yet. (The Log operator itself needs more formal, thorough testing; but anecdotally, it's pretty soild.)

There are ten Log node styles (five complimentary encoding-decoding styles) that map to the three logarithimic_function methods: log2/antiLog2, log10 / antiLog10, and logB / antiLogB correspond to logarithmic_function_basic; logToLin / linToLog correspond to logarithimic_function_quasilog; and cameraLogToLin/ cameraLinToLog with logarithmic_function_camera. (In my private implementation, AbstractLUTSequenceOperators have a stateful "direction" attribute, and "_function_fwd" and "_function_inv" abstract methods; which means that LUTSequenceOperators inherit complementary, stateful apply and reverse methods, and pure encode and decode methods. Remind me to share an example!)

It's possible for the state of the style instance variable to be at odds with certain other instance variables. For instance, Log(style="log10", base=2). In my implementation, the style kwarg is mostly only used to establish the direction (encoding or decoding) of the operation when "apply" is invoked. The initial quality of "style" is inferred from how or whether other variables are initialized.

The Log and Exponent CLF nodes may or may not contain per-channel parametrization. Unfortunately, CLF multi-channel parameter representation ({'R': {'param1':..., 'param2':...}, 'G': {'param1:'..., 'param2':...}, 'B': {'param1':..., 'param2'...}) is orthogonal to sane node interfaces ({"param1": (R G, B), "param2": (R, G, B) ...}). Since there are five such LogParam attributes, I tried to come up with a user-friendly general approach that would be easy to adapt to other nodes.

@KelSolaar
Copy link
Member Author

Oups! Sorry for the typo @zachlewis :)

@KelSolaar
Copy link
Member Author

KelSolaar commented Nov 4, 2020

Hi,

I took a quick look tonight and pushed your two respective branches as PRs here:

My initial gut feeling is that it is going to be tricky to merge Zack's branch without a significant amount of doctoring, there has been a lot of merges in-between the commits, some of the critical files have disappeared, e.g. poetry.toml and some others have spawned, e.g. CMakeLists.txt and package.py for Rez.

I tried to rebase on-top of develop but quickly stopped because of the conflicts. I will try to think about a strategy but as of right now, it is really tempting to squash a lot of stuff to only cherry-pick some of the commits.

Nick's branch is quite clean but I haven't compared the code between them yet!

@zachlewis
Copy link
Contributor

zachlewis commented Nov 4, 2020

Oh snap...!

So sorry for the confusion. You'll have to forgive me, I totally forgot about the Rez stuff -- the added package.py and CMakeLists.txt and missing poetry.toml were all part of the same commit -- I'll fix that. (Other than the Rez stuff, I really did do my best to keep stuff easy for Nick to merge into his feature/CLF branch)

Anyway, I thought this was getting a little hairy, so I put together a feature/CLF_Log branch a few days ago based off of the develop branch, and I've extracted just my Log LUTSequenceOperator into a single commit for your rebasing pleasure. (I also have some stuff for un/structuring to and from CLF, but that's better handled in a separate commit).

I've also put together a temporary zzz/CLF_reference branch, where I did my best to rebase the current state of my work and Nick's work onto the develop branch the best I could, with each node or feature squashed into its own commit.

Here's a brief history of time, for posterity:

I had been maintaining my own implementation of the Log operator for colour (in my feature/CLF branch), refactored from another of my own implementations of the Log operator (for a library at work, one of the things I'd alluded to in #500). Shortly after @njwardhan's logarithmic_function_camera etc. work was merged into your develop branch, I figured I'd update my Log operator to use the new colour methods instead of the math I'd implemented directly in the class. Anyway, I noticed @nick-shaw had just implemented an Exponent operator and related CLF read/write stuff in his clf_temp branch, and had stubbed out a Log class. I figured it was a good opportunity to kill two birds with one stone, and maybe save Nick a little time, so I did my best to simplify and refactor my Log operator to closer match what Nick was doing.

@KelSolaar
Copy link
Member Author

No worries at all! It was really just me looking at the branches and trying to understand what was going on.

I'm yet to compare properly the code between your branch and that of Nick but they are very much similar right? Would it make sense for you and Nick to work in a single branch?

@KelSolaar KelSolaar modified the milestones: v0.3.16, v0.4.0 Nov 23, 2020
@KelSolaar KelSolaar modified the milestones: v0.4.0, v0.4.1, v0.4.2 Feb 19, 2022
@KelSolaar KelSolaar modified the milestones: v0.4.2, v0.4.3 Nov 27, 2022
@KelSolaar KelSolaar modified the milestones: v0.4.3, v0.4.4 Aug 25, 2023
@KelSolaar KelSolaar modified the milestones: v0.4.4, v0.4.5 Dec 19, 2023
@KelSolaar KelSolaar added P1 and removed P2 labels Jan 23, 2024
@KelSolaar KelSolaar modified the milestones: v0.4.5, v0.4.6, v0.4.7 Oct 10, 2024
@MichaelMauderer MichaelMauderer linked a pull request Nov 7, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants