-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add ProPhoto RGB standard #413
Conversation
CodSpeed Performance ReportMerging #413 will not alter performanceComparing Summary
|
Thanks, this looks good, but I think it's a bit excessive to have a 16bit lookup table enabled by default. I would like to explore a code generation option for cases like these later, so let's skip it for now. |
That's fair. I went ahead and removed the lookup table. I'm planning on opening a new pull request that will add macros that will implement both lookup tables and fast f32 to uint conversion similar to that of |
Thank you! As one last thing, do you think you could rebase this to a single commit? The old P3 commit(s) are still in there, and it would be nice to not have them in the main branch.
My hunch is that it would fit best to be generated by the build script, behind a feature flag. Similar to the SVG/CSS color names. That way, only those who want the big tables get them. Alternatively, let people generate them during runtime. That's another research project... |
I'm sorry. I'm still getting used to using git. How would I rebase this into a single commit? |
No worries! The merge in there makes it a bit harder than it usually is, so it may be worth following the answer here: https://stackoverflow.com/questions/30136558/how-to-squash-commits-which-have-merge-commit-in-between Make a note of the commit hash first, so you can easily go back to it (restore it with |
An alternative approach, if the merge conflict fix isn't too bad to lose, is to reset the branch to the commit before you merged and squash from there. Then, you can follow a simpler guide and finally use |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #413 +/- ##
==========================================
+ Coverage 82.80% 82.90% +0.09%
==========================================
Files 130 132 +2
Lines 20039 20260 +221
Branches 20039 20260 +221
==========================================
+ Hits 16593 16796 +203
- Misses 3319 3337 +18
Partials 127 127
|
I think that was successful. Thank you! I think I understand git much better now. |
Yep, looks great! Git is sort of conceptually simple (a directed graph of versions, where branches are "bookmarks"), but the interface and terminology isn't always easy to learn at first. I'll merge it after the last test is done, but this is all for this one. Thank you! 🙏 |
This seems to be the best option, yeah. I'm hard at work on it =) |
It could be nice to start with the |
This pull request adds the ProPhoto RGB standard along with a
u16
->f64
lookup table forIntoLinear
. Ideally, I would like to implement a fast lookup forFromLinear
similar to that offast-srgb8
, but I need to figure out how to adapt that to u16 first.Completes
prophoto-rgb
task of #408, by implementing the ProPhoto RGB standard.