Skip to content

Conversation

@ripytide
Copy link
Contributor

@ripytide ripytide commented Dec 1, 2024

I'm not sure #138 should have been merged given the number of warnings and bugs in it unless I am misunderstanding the code. Regardless, I've attempted to fix the issues with this PR.

@ripytide
Copy link
Contributor Author

ripytide commented Dec 1, 2024

@burbokop you might want to review these changes to see if they agree with your usecase.

@kornelski kornelski merged commit ca5b03f into kornelski:main Dec 2, 2024
1 check passed
@burbokop
Copy link
Contributor

burbokop commented Dec 3, 2024

I used different formula. Instead of passing v and a to new as values in range 0..16 and doing (v << 4) | a my idea was to pass values in range 0..256 and compress its precision like this: ((v / 16) << 4) | (a / 16) which can be simplified as ((v >> 4) << 4) | (a >> 4) because v / 2^n = v >> n and then ((v >> 4) << 4) reduces to (v & 0xf0) and we have (v & 0xf0) | (a >> 4). But your version also works for me. I just need to change my code from new(v, a) to new(v / 16, a / 16) And the same logic with fn v() and fn a().

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.

3 participants