-
Notifications
You must be signed in to change notification settings - Fork 11
Reduce allocations during encoding #11
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
Spaceface16518
wants to merge
7
commits into
fpapado:master
Choose a base branch
from
Spaceface16518:master
base: master
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.
Open
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8978c9c
Abstract over quantization function in encode_ac
Spaceface16518 1df6990
Abstract over rounding function in encode_dc
Spaceface16518 230cd86
Remove redundant parenthesis
Spaceface16518 5c0b127
Only allocate once when constructing hash
Spaceface16518 00558a3
Use associated `pow` function
Spaceface16518 5e2586c
Format code
Spaceface16518 90d464a
Don't clone ac vector when calculating max value
Spaceface16518 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,9 +203,9 @@ pub fn encode( | |
bytes_per_pixel, | ||
0, | ||
|a, b| { | ||
(normalisation | ||
normalisation | ||
* f64::cos((PI * x as f64 * a) / width as f64) | ||
* f64::cos((PI * y as f64 * b) / height as f64)) | ||
* f64::cos((PI * y as f64 * b) / height as f64) | ||
}, | ||
); | ||
|
||
|
@@ -219,10 +219,10 @@ pub fn encode( | |
} | ||
} | ||
|
||
let mut hash = String::new(); | ||
let mut hash = String::with_capacity(1 + 1 + 4 + 2 * ac.len()); | ||
|
||
let size_flag = ((cx - 1) + (cy - 1) * 9) as usize; | ||
hash += &encode_base83_string(size_flag, 1); | ||
hash.extend(encode_base83_string(size_flag, 1)); | ||
|
||
let maximum_value: f64; | ||
|
||
|
@@ -239,16 +239,16 @@ pub fn encode( | |
usize::min(82, f64::floor(actual_maximum_value * 166f64 - 0.5) as usize), | ||
); | ||
maximum_value = ((quantised_maximum_value + 1) as f64) / 166f64; | ||
hash += &encode_base83_string(quantised_maximum_value, 1); | ||
hash.extend(encode_base83_string(quantised_maximum_value, 1)); | ||
} else { | ||
maximum_value = 1f64; | ||
hash += &encode_base83_string(0, 1); | ||
hash.extend(encode_base83_string(0, 1)); | ||
} | ||
|
||
hash += &encode_base83_string(encode_dc(dc), 4); | ||
hash.extend(encode_base83_string(encode_dc(dc), 4)); | ||
|
||
for factor in ac { | ||
hash += &encode_base83_string(encode_ac(factor, maximum_value), 2); | ||
hash.extend(encode_base83_string(encode_ac(factor, maximum_value), 2)); | ||
} | ||
|
||
Ok(hash) | ||
|
@@ -300,37 +300,20 @@ where | |
[r * scale, g * scale, b * scale] | ||
} | ||
|
||
fn encode_dc(value: [f64; 3]) -> usize { | ||
let rounded_r = linear_to_srgb(value[0]); | ||
let rounded_g = linear_to_srgb(value[1]); | ||
let rounded_b = linear_to_srgb(value[2]); | ||
((rounded_r << 16) + (rounded_g << 8) + rounded_b) as usize | ||
fn encode_dc([r, g, b]: [f64; 3]) -> usize { | ||
let rounded = |v| linear_to_srgb(v); | ||
((rounded(r) << 16) + (rounded(g) << 8) + rounded(b)) as usize | ||
} | ||
|
||
fn encode_ac(value: [f64; 3], maximum_value: f64) -> usize { | ||
let quant_r = f64::floor(f64::max( | ||
0f64, | ||
f64::min( | ||
18f64, | ||
f64::floor(sign_pow(value[0] / maximum_value, 0.5) * 9f64 + 9.5), | ||
), | ||
)); | ||
let quant_g = f64::floor(f64::max( | ||
0f64, | ||
f64::min( | ||
18f64, | ||
f64::floor(sign_pow(value[1] / maximum_value, 0.5) * 9f64 + 9.5), | ||
), | ||
)); | ||
let quant_b = f64::floor(f64::max( | ||
0f64, | ||
f64::min( | ||
18f64, | ||
f64::floor(sign_pow(value[2] / maximum_value, 0.5) * 9f64 + 9.5), | ||
), | ||
)); | ||
|
||
(quant_r * 19f64 * 19f64 + quant_g * 19f64 + quant_b) as usize | ||
fn encode_ac([r, g, b]: [f64; 3], maximum_value: f64) -> usize { | ||
let quant = |v| { | ||
(sign_pow(v / maximum_value, 0.5) * 9. + 9.5) | ||
.floor() | ||
.min(18.) | ||
.max(0.) | ||
}; | ||
|
||
(quant(r) * 19f64 * 19f64 + quant(g) * 19f64 + quant(b)) as usize | ||
Comment on lines
-303
to
+316
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a closure here drastically reduces the code size. It has the exact same behavior. Using a closure will not have any overhead, since the virtual function does not escape the parent function. |
||
} | ||
|
||
// Base83 | ||
|
@@ -352,11 +335,10 @@ fn decode_base83_string(string: &str) -> usize { | |
.fold(0, |value, digit| value * 83 + digit) | ||
} | ||
|
||
fn encode_base83_string(value: usize, length: u32) -> String { | ||
fn encode_base83_string(value: usize, length: u32) -> impl Iterator<Item=char> { | ||
(1..=length) | ||
.map(|i| (value / usize::pow(83, length - i)) % 83) | ||
.map(move |i| (value / 83usize.pow(length - i)) % 83) | ||
.map(|digit| ENCODE_CHARACTERS[digit]) | ||
.collect() | ||
} | ||
|
||
#[cfg(test)] | ||
|
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.
This just gets rid of a rustc warning