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

Increased API coverage of QRgba64 and Qrgb #214

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

caoeri
Copy link
Contributor

@caoeri caoeri commented Nov 21, 2021

QRgba64 is now derived from the C++ class rather than implemented in Rust.
Additionally, some conversions were added.
Conversion from u64 to QRgba64 is not currently implemented due to endian-ness concerns.

QRgba64 is now derived from the C++ class rather than implemented in Rust.
Additionally, some conversions were added.
Conversion from u64 to QRgba64 is not currently implemented due to
endian-ness concerns.
@ogoffart
Copy link
Member

QRgba64 is now derived from the C++ class rather than implemented in Rust.

What's the rationale? Having it implemented in rust if probably much more efficient for such low level API. When using QRgba64, one want to use it as light class, and it ideally should be Copy and use bit shuffling to extract the color without going through a function call in C++

@caoeri caoeri changed the title Updated Q Increased API coverage of QRgba64 and Qrgb Nov 21, 2021
@caoeri
Copy link
Contributor Author

caoeri commented Nov 21, 2021

If we do that you might have to change the license to LGPL.

We could create a Rust version but it would need to be compatible and have the same internal representation since it is used for SIMD applications, which makes many of the operations exactly identical in code. I could do it if you really want but it seems a bit strange to do it this way when the implementation is already there.

QRgba64 is now fully implemented in Rust.
Additionally, some conversions were added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants