-
Notifications
You must be signed in to change notification settings - Fork 693
BMP: Faster bitfield reading #2900
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
Open
RunDevelopment
wants to merge
3
commits into
image-rs:main
Choose a base branch
from
RunDevelopment:bmp-faster-bitfield
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+51
−43
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I do believe many of these are much more apparent in hex. Like
85 = 0x55is intuitively right and not weird. Obviously there are weirder cases but even for 6bit seeing0x40c + 0x84is 'simpler' to correlate with the arithmetic than the decimal variant. I think it also gets rid of the need to write only some of these with a bitshift, i.e.85 << 8should be written as0x5500and9344as0x2480, the 4-bit case as0x1100etc.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.
Ah, so you were one that made the previous constants hex. I was wondering who did that, because I don't find them to be intuitive at all in hex :)
Hex constants are a bad fit here IMO. The multiply-add method (MAM) is based on integer approximations for linear functions with rational parameters. MAM only uses bitshifts for fast division to approximate rationals. In fact, there's nothing special about division by powers of two. Any integer power will work. So representing MA constants as hex does not make their function more apparent. You can reinterpret their function in a different context, but that has nothing to do with MAM itself.
Take 85, for example. That's just 255 / 3. Very natural in decimal, no? Yes, there is the alternate interpretation that 85 = 0b01010101 duplicates bits, but that has nothing to do with MAM.
Uh oh!
There was an error while loading. Please reload this page.
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.
The connection to hex is that
255 = 1<<8 - 1; the2**n ± 1connection makes it natural for me to use a notation where bits stand out. It's certainly not completely arbitrary in a mathematical sense; plus the whole conversion sequence ends with a fixed-point number0p1in base2^8. The reason to prefer base 2^4 over base 2^3 or 2^1 is, apart from slightly simpler fixed-point interpretation, convenience—in that there are probably more IT people fluent in that base than another. Octal would honestly be fine with me, too, binary is too verbose (for the same reason some folk used base-12 but not fewer civilizations base smaller than 10).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.
I'm not saying there isn't a connection. I'm saying that connection doesn't matter for the multiply-add method.
The multiply-add method works by using rational linear functions. For any MAM problem (e.g.
round(x * 255 / 3) for x in 0..=3) there exists an infinite set of rational linear functions that solve the problem. We just typically pick functions with coefficients of the formf/2^s * x + a/2^sbecause hardware is good at dividing by powers of two. If hardware was good at dividing by prime numbers, we'd pick differently.Choosing to interpret these numbers as fixed-point numbers base two has no advantage, but misleadingly suggests a relevant connection that does not exist.
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.
I would be taking the notation on the 256-base fixed-point argument alone which you seem to think is easier for at least some, having written 1,2,4 as
_ << 8. I think that applies to all and hex obviates the need of switching notation.And while you could use arbitrary functions very clearly this specific one is a linear one and so I am not buying the argument of arbitrariness. The coefficient matching a slope of roughly
0xff.80/(2^n - 1)is required for this form to work; this heritage is definitely not misleading, it's the first simplification of the necessary & sufficient inequality criteria. At least for me that quotient is simple to grok in hex and awful to do in decimal.Uh oh!
There was an error while loading. Please reload this page.
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.
Okay, then I'll explain the MAM a bit more. One way to formulate MAM is this:
Given an expression of the form$\lfloor (x\cdot t+r_d)/d\rfloor$ and an input range $u$ where $u\in\N_1, t\in\N, d\in\N_1, r_d\in\N, r_d<d, x\in\N, x\le u$ , find a tuple $(f,a,s)\in\N^3$ such that $\lfloor (x\cdot t+r_d)/d\rfloor = \lfloor (x\cdot f+a)/2^s\rfloor$ .
This should be familiar. There are a few ways to find these solution tuples. If you go a more traditional number-theory route with fixed-point arithmetic, you'll get to a fairly well-known result:$f=\lceil t/d\cdot 2^s \rceil$ , $a=\lceil r_d/d\cdot 2^s \rceil$ , and $s=\lceil \log_2 d + \log_2(u+1) \rceil$ . This works but it's an incomplete picture of the solution space.
A slightly modified version of MAM makes it easy to see the whole solution space. Let$(m,n)\in\R^2$ be a solution iff $\lfloor (x\cdot t+r_d)/d\rfloor = \lfloor xm+n\rfloor$ . The connection to $(f,a,s)$ should be clear: $(f,a,s)$ is a solution if $(m,n)=(f/2^s,a/2^s)$ is a solution.
For example, for the problem round(x • 255 / 31) for x in 0..=31, the set of all solutions$(m,n)$ looks like this (white pixels):
(The magenta vertical line marks$m=255/31\approx 8.2258$ . The horizontal magenta line marks $n=15/31\approx 0.48387$ , which comes from $round(x \cdot 255 / 31) = \lfloor (x \cdot 255 + 15) / 31\rfloor$ .)
This (half-open) polygon is the true nature of MAM. Any point$(m,n)$ within it is a solution.
This is why I said that MAM doesn't have much of a connection with fixed-point or anything base-two really. Fundamentally, MAM is about finding a point in a polygon. The points we pick with solutions$(f,a,s)$ only look like fixed-point numbers, because they represent rational points $(m,n)=(f/2^s,a/2^s)$ . But the important property isn't that the denominator is a power of two, but that the rational numbers represent a point inside the polygon. We could have picked points of the form $(m,n) = (p/1234,q/1234)$ for $p,q\in\N$ if we wanted to (e.g.
((x as u32 * 10154 + 560) / 1234) as u8also works as a 5- to 8-bit unorm conversion).This is why I dislike representing the constants as fixed-point or hex so much. IMO they suggest an important connection to something related to powers of two or base two, but there's nothing there.