ilia3101 / MLV-App

All in one MLV processing app.
https://mlv.app/
GNU General Public License v3.0
288 stars 31 forks source link

Replace "Gamma" and "Tonemapping" with "transfer function" #178

Open ilia3101 opened 5 years ago

ilia3101 commented 5 years ago

I am going to implement a method for passing a string math expression to create custom tonemapping functions, something I have wanted so long. It will be amazing for experimentation. Also quite easy to do, especially if I use some premade expression parser library.

This seems like a great option: https://github.com/jamesgregson/expression_parser

ilia3101 commented 4 years ago

@masc4ii Can you help me add https://github.com/codeplea/tinyexpr to MLV App's source code, in the processing folder (just the two files tinyexpr.c and tinyexpr.h), I am not sure how to make Qt compile them.

masc4ii commented 4 years ago

Sure. https://github.com/ilia3101/MLV-App/commit/f56f84100f190c5d6ea46bec7cb94722b1f18c53 Here you are.

ilia3101 commented 4 years ago

This will require a change to Qt interface and MASXML format. Gamma and tonemapping function will be replaced by "Transfer function" expression string, because "Transfer function" is a more correct term and gamma is a meaningless word. This way it will be much more customisable as well. Maybe we should also add a box for transfer function presets, as it's not easy to memorise them.

For updating old MASXML, I will write a function that converts a combination of "Gamma" and "Tonemapping function" in to a transfer function expression string.

Examples: "x" = linear, "pow(1/(x+1), 1/3.15)" = old reinhard preset

masc4ii commented 4 years ago

Yes, such a function would be good. I think 99% of the people just use those presets. So it will be good to still have this combobox, as preset selection. No problem, if we could change the formula then. Or is it maybe better if we could select between what we have now and this new function style? So we would just have to add a flag and the formula as string (to the MASXML).

ilia3101 commented 4 years ago

I think 99% of the people just use those presets.

Yep, and those who want to, will be able to add their own functions or log curves.

Or is it maybe better if we could select between what we have now and this new function style?

Maybee, but it's going to be more difficult to implement, also more difficult on the interface side.

masc4ii commented 4 years ago

Maybee, but it's going to be more difficult to implement, also more difficult on the interface side.

That is true. But maybe it is worth it... Would it be possible to "automate the written function" by a gamma slider and the curve combo? I think yes. So we could setup the function like in the past (but now you see what the corresponding function is), or set to custom, which disables the gamma slider and enables the edit on the function.

ilia3101 commented 4 years ago

Ah yes it could be done from the interface, using the function that converts gamma and tonemap function to a string.

I'll be honest. the thing that motivated me to do this was zeek using gamma and log 😂

ilia3101 commented 4 years ago

https://github.com/ilia3101/MLV-App/commit/6277c35be51b522034680528e698f86870b3a87d

Now I need to add a conversion from "Gamma" and "Tonemaping function" to transfer function expression string (so we can have MASXML compatibility, and maybe have old mode in GUI if you want to add that).

masc4ii commented 4 years ago

https://github.com/ilia3101/MLV-App/commit/87370c98fd01a9aef3c84c0e0e22b2fe416eb796 Now you can test it. I disabled visible flag for the other GUI elements, it seems the elements have no function right now.

ilia3101 commented 4 years ago

Thank you

ilia3101 commented 3 years ago

Happy new year everyone!

Sorry about the massive disappearance recently.

This needs to be finished :) I have left MLV App in a somewhat unusable state. But now I think it's time to fix it and release soon, as it's a new year and there is so much exciting Magic Lantern news. Will be committing plenty of improvements soon.

Or is it maybe better if we could select between what we have now and this new function style? So we would just have to add a flag and the formula as string (to the MASXML).

Yes! We should definitely do it like this. Are you ready to implement this? You can use the void processingSetGammaAndTonemapping(processingObject_t * processing, double gamma, int tonemapping); to support the old way. Currently there isn't a getter for those values, as they aren't stored in processingObject now, as it internally always uses the string. Would you like there to be one?

@masc4ii

masc4ii commented 3 years ago

Happy new year to you too!

Nice to have you back!

I am ready, yes. I was already working with this function, but it makes life hard, because I need some logic to ask for tonemapping when changing gamma and the other way. And I think I had a problem with this function, because sometimes gamma is 1/x and sometimes it is x. So the try to use it failed. Having the old way would be nice, and additionally this new feature.

masc4ii commented 3 years ago

Do these two changes to the code and try yourself: Bildschirmfoto 2021-01-08 um 17 06 41 Bildschirmfoto 2021-01-08 um 17 07 28

ilia3101 commented 3 years ago

Trying that.