Apply gamma correction for coregraphics fonts #774
Conversation
| } | ||
|
|
||
| // Assumes the pixels here are linear values from CG | ||
| fn gamma_correct_pixels(&self, pixels: &mut Vec<u8>, width: usize, |
There was a problem hiding this comment.
nit: As far as I understand it, pixels's length cannot change, right? If so, you could probably make it a &mut [u8] so you save an indirection.
There was a problem hiding this comment.
Ahh good point. Thanks! Since I already have the dwrite gamma correction also using vector, can I fix this + dwrite + the gamma-lut library in a follow up patch? Too many patches in mid flight right now.
|
@bors-servo r+ |
|
📌 Commit 130bc43 has been approved by |
webrender/src/platform/macos/font.rs
Outdated
| use std::collections::HashMap; | ||
| use std::collections::hash_map::Entry; | ||
| use webrender_traits::{ColorU, FontKey, FontRenderMode, GlyphDimensions}; | ||
| use gamma_lut; |
There was a problem hiding this comment.
would be cleaner to use gamma_lut::{GammaLut, Color as ColorLut};
webrender/src/platform/macos/font.rs
Outdated
| match render_mode { | ||
| FontRenderMode::Alpha => { | ||
| self.gamma_lut.preblend_grayscale_bgra(pixels, width, height, | ||
| gamma_lut::Color::new(color.r, |
There was a problem hiding this comment.
could move the ColorLut initialization out of the match
webrender/src/platform/macos/font.rs
Outdated
| FontContext { | ||
| cg_fonts: HashMap::new(), | ||
| ct_fonts: HashMap::new(), | ||
| gamma_lut: gamma_lut::GammaLut::new(contrast, gamma, gamma), |
There was a problem hiding this comment.
given that you pass the same gamma as both paint gamma and device gamma, the whole LUT'ing thing becomes just a contrast application, according to the source:
if (src - dst).abs() < (1.0 / 256.0) {
let mut ii : f32 = 0.0;
for i in 0..256 {
let raw_srca = ii / 255.0;
let srca = apply_contrast(raw_srca, adjusted_contrast);
table[i] = round_to_u8(255.0 * srca);
ii += 1.0;
}
}Is this by design? Are we going to use different gamma parameters here in the long term?
There was a problem hiding this comment.
It is by design at the moment, it's just a port from Skia - http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkPaint.cpp#1498
Also, that bit of code isn't the src/dst gamma. That's after the luminance conversion - http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkMaskGamma.cpp#79. We don't actually hit that if block.
Gamma correction for dwrite fonts Same as #774 (review) but for dwrite fonts! <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/775) <!-- Reviewable:end -->
|
@kvark Did I answer your question? If so, please merge. Thanks! |
|
Thanks!
@bors-servo r+
… On Jan 25, 2017, at 12:12, Mason Chang ***@***.***> wrote:
@kvark Did I answer your question? If so, please merge. Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
📌 Commit cffd17d has been approved by |
|
🔒 Merge conflict |
|
☔ The latest upstream changes (presumably #770) made this pull request unmergeable. Please resolve the merge conflicts. |
Gamma correction for dwrite fonts Same as #774 (review) but for dwrite fonts! <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/775) <!-- Reviewable:end -->
|
@bors-servo r=kvark |
|
📌 Commit 259bab0 has been approved by |
Apply gamma correction for coregraphics fonts I finished Jeff's port of Skia's gamma mask code here - https://github.com/changm/gamma-lut-rs, which is the packaged added in this patch. The original source is here - http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkMaskGamma.cpp <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/774) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
5 participants
I finished Jeff's port of Skia's gamma mask code here - https://github.com/changm/gamma-lut-rs, which is the packaged added in this patch.
The original source is here - http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkMaskGamma.cpp
This change is