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
Vectorized linear function #3303
Conversation
Hi, thanks for the PR. How about we switch the (internal) .linear(a, b)
// would be equivalent to
.linear([a], [b])
// but now we would also support
.linear([a1, a2, a3], [b1, b2, b3]) This should make the C++ code path a bit simpler as it will only deal with arrays of doubles. |
@lovell Great suggestion, I have implemented it. Added some fancy logic for dealing with the alpha channel that you may wish to review. Only outstanding thing is test cases, which I can hopefully make some progress on shortly. |
Handling the multi-band expansion for linear() looked tricky in the context of sharp's colourspace handling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. I've left a couple of small comments/questions inline but otherwise this is good to go.
Please can you rebase against the upstream/eagle
branch.
@lovell Done, all yours. |
Please can you add unit test(s) to ensure all the new logic is fully-covered - see https://coveralls.io/builds/51160601 |
Landed via commit 74e3f73 - this will be in v0.31.0, thanks again for working on this. |
@lovell This looked easy enough to implement, and adds some good value. This pull request is not really production quality and my knowledge of the code base is very limited. A few things to consider: