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

ignore glyph color data when doing fast shadows #2223

Merged
merged 1 commit into from Dec 18, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -7,6 +7,7 @@
flat varying vec4 vColor;
varying vec3 vUv;
flat varying vec4 vUvBorder;
flat varying vec2 vMaskSwizzle;

#ifdef WR_FEATURE_GLYPH_TRANSFORM
varying vec4 vUvClip;
@@ -125,27 +126,29 @@ void main(void) {

write_clip(vi.screen_pos, prim.clip_area);

#ifdef WR_FEATURE_SUBPX_BG_PASS1
vColor = vec4(text.color.a) * text.bg_color;
#else
switch (uMode) {
case MODE_ALPHA:
case MODE_BITMAP:
vMaskSwizzle = vec2(0.0, 1.0);
vColor = text.color;
break;
case MODE_SUBPX_PASS1:
case MODE_SUBPX_BG_PASS2:
case MODE_BITMAP:
vMaskSwizzle = vec2(1.0, 0.0);
vColor = text.color;
break;
case MODE_SUBPX_CONST_COLOR:
case MODE_SUBPX_PASS0:
case MODE_SUBPX_BG_PASS0:
case MODE_COLOR_BITMAP:
vMaskSwizzle = vec2(1.0, 0.0);
vColor = vec4(text.color.a);
break;
case MODE_SUBPX_BG_PASS1:
// This should never be reached.
vMaskSwizzle = vec2(-1.0, 1.0);
vColor = vec4(text.color.a) * text.bg_color;
break;
}
#endif

vec2 texture_size = vec2(textureSize(sColor0, 0));
vec2 st0 = res.uv_rect.xy / texture_size;
@@ -160,16 +163,13 @@ void main(void) {
void main(void) {
vec3 tc = vec3(clamp(vUv.xy, vUvBorder.xy, vUvBorder.zw), vUv.z);
vec4 mask = texture(sColor0, tc);
mask.rgb = mask.rgb * vMaskSwizzle.x + mask.aaa * vMaskSwizzle.y;

float alpha = do_clip();
#ifdef WR_FEATURE_GLYPH_TRANSFORM
alpha *= float(all(greaterThanEqual(vUvClip, vec4(0.0))));
#endif

#ifdef WR_FEATURE_SUBPX_BG_PASS1
mask.rgb = vec3(mask.a) - mask.rgb;
#endif

oFragColor = vColor * mask * alpha;
}
#endif
@@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use api::{BorderDetails, BorderDisplayItem, BuiltDisplayList};
use api::{ClipAndScrollInfo, ClipId, ColorF, PropertyBinding};
use api::{ClipAndScrollInfo, ClipId, ColorF, ColorU, PropertyBinding};
use api::{DeviceUintPoint, DeviceUintRect, DeviceUintSize};
use api::{DocumentLayer, ExtendMode, FontRenderMode, LayoutTransform};
use api::{GlyphInstance, GlyphOptions, GradientStop, HitTestFlags, HitTestItem, HitTestResult};
@@ -1345,6 +1345,7 @@ impl FrameBuilder {
glyph_gpu_blocks: Vec::new(),
glyph_keys: Vec::new(),
offset: run_offset,
shadow_color: ColorU::new(0, 0, 0, 0),

This comment has been minimized.

@kvark

kvark Dec 14, 2017

Member

Would be best (but not required) to have the TextRunPrimitiveCpu in a separate commit, since it's technically independent of the mask swizzling.

};

// Text shadows that have a blur radius of 0 need to be rendered as normal
@@ -1362,7 +1363,7 @@ impl FrameBuilder {
match picture_prim.kind {
PictureKind::TextShadow { offset, color, blur_radius, .. } if blur_radius == 0.0 => {
let mut text_prim = prim.clone();
text_prim.font.color = color.into();
text_prim.shadow_color = color.into();
text_prim.offset += offset;
fast_shadow_prims.push((idx, text_prim));
}
@@ -221,6 +221,17 @@ pub enum GlyphFormat {
ColorBitmap,
}

impl GlyphFormat {
pub fn ignore_color(self) -> Self {
match self {
GlyphFormat::Subpixel => GlyphFormat::Alpha,
GlyphFormat::TransformedSubpixel => GlyphFormat::TransformedAlpha,
GlyphFormat::ColorBitmap => GlyphFormat::Bitmap,
_ => self,
}
}
}

pub struct RasterizedGlyph {
pub top: f32,
pub left: f32,
@@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use api::{BorderRadius, BuiltDisplayList, ClipAndScrollInfo, ClipId, ClipMode, ColorF};
use api::{BorderRadius, BuiltDisplayList, ClipAndScrollInfo, ClipId, ClipMode, ColorF, ColorU};
use api::{ComplexClipRegion, DeviceIntRect, DevicePoint, ExtendMode, FontRenderMode};
use api::{GlyphInstance, GlyphKey, GradientStop, ImageKey, ImageRendering, ItemRange, ItemTag};
use api::{LayerPoint, LayerRect, LayerSize, LayerToWorldTransform, LayerVector2D, LineOrientation};
@@ -740,9 +740,9 @@ pub struct TextRunPrimitiveCpu {
pub glyph_count: usize,
pub glyph_keys: Vec<GlyphKey>,
pub glyph_gpu_blocks: Vec<GpuBlockData>,
pub shadow_color: ColorU,
}


impl TextRunPrimitiveCpu {
pub fn get_font(
&self,
@@ -762,6 +762,10 @@ impl TextRunPrimitiveCpu {
font
}

pub fn is_shadow(&self) -> bool {
self.shadow_color.a != 0
}

fn prepare_for_render(
&mut self,
resource_cache: &mut ResourceCache,
@@ -813,7 +817,12 @@ impl TextRunPrimitiveCpu {

fn write_gpu_blocks(&self, request: &mut GpuDataRequest) {
let bg_color = ColorF::from(self.font.bg_color);
request.push(ColorF::from(self.font.color).premultiplied());
let color = ColorF::from(if self.is_shadow() {
self.shadow_color
} else {
self.font.color
});
request.push(color.premultiplied());
// this is the only case where we need to provide plain color to GPU
request.extend_from_slice(&[
GpuBlockData { data: [bg_color.r, bg_color.g, bg_color.b, 1.0] }
@@ -1597,7 +1597,6 @@ pub struct Renderer {
// output, and the cache_image shader blits the results of
// a cache shader (e.g. blur) to the screen.
ps_text_run: TextShader,
ps_text_run_subpx_bg_pass1: TextShader,

This comment has been minimized.

@kvark

kvark Dec 14, 2017

Member

nice! 👍

ps_image: Vec<Option<PrimitiveShader>>,
ps_yuv_image: Vec<Option<PrimitiveShader>>,
ps_border_corner: PrimitiveShader,
@@ -1863,13 +1862,6 @@ impl Renderer {
options.precache_shaders)
};

let ps_text_run_subpx_bg_pass1 = try!{
TextShader::new("ps_text_run",
&mut device,
&["SUBPX_BG_PASS1"],
options.precache_shaders)
};

// All image configuration.
let mut image_features = Vec::new();
let mut ps_image: Vec<Option<PrimitiveShader>> = Vec::new();
@@ -2244,7 +2236,6 @@ impl Renderer {
cs_clip_border,
cs_clip_image,
ps_text_run,
ps_text_run_subpx_bg_pass1,
ps_image,
ps_yuv_image,
ps_border_corner,
@@ -3561,7 +3552,7 @@ impl Renderer {

self.device.set_blend_mode_subpixel_with_bg_color_pass1();

self.ps_text_run_subpx_bg_pass1.bind(
self.ps_text_run.bind(
&mut self.device,
glyph_format,
transform_kind,
@@ -4437,7 +4428,6 @@ impl Renderer {
self.cs_clip_image.deinit(&mut self.device);
self.cs_clip_border.deinit(&mut self.device);
self.ps_text_run.deinit(&mut self.device);
self.ps_text_run_subpx_bg_pass1.deinit(&mut self.device);
for shader in self.ps_image {
if let Some(shader) = shader {
shader.deinit(&mut self.device);
@@ -570,7 +570,7 @@ fn add_to_batch(
&text_cpu.glyph_keys,
glyph_fetch_buffer,
gpu_cache,
|texture_id, glyph_format, glyphs| {
|texture_id, mut glyph_format, glyphs| {
debug_assert_ne!(texture_id, SourceTexture::Invalid);

let textures = BatchTextures {
@@ -581,6 +581,11 @@ fn add_to_batch(
],
};

// Ignore color and only sample alpha when shadowing.
if text_cpu.is_shadow() {
glyph_format = glyph_format.ignore_color();
}

let kind = BatchKind::Transformable(
transform_kind,
TransformBatchKind::TextRun(glyph_format),
@@ -1,5 +1,5 @@
The "Proggy.fon" font was downloaded from https://proggyfonts.net/download/ on Dec 5, 2017.
The following license applies to "Proggy.fon":
The "Proggy.ttf" font was downloaded from https://proggyfonts.net/download/ on Dec 5, 2017.
The following license applies to "Proggy.ttf":

Copyright (c) 2004, 2005 Tristan Grimmer

Binary file not shown.
Binary file not shown.
@@ -5,5 +5,5 @@ root:
origin: 20 30
bounds: [0, 0, 710, 50]
size: 8.25
font: "Proggy.fon"
font: "Proggy.ttf"
embedded-bitmaps: true
@@ -21,7 +21,7 @@ fuzzy(1,100) == decorations-suite.yaml decorations-suite.png
fuzzy(1,735) == shadow-grey.yaml shadow-grey-ref.yaml
fuzzy(1,614) == shadow-grey-transparent.yaml shadow-grey-ref.yaml
== subtle-shadow.yaml subtle-shadow-ref.yaml
== shadow-atomic.yaml shadow-atomic-ref.yaml
options(disable-aa) == shadow-atomic.yaml shadow-atomic-ref.yaml

This comment has been minimized.

@glennw

glennw Dec 15, 2017

Member

Why did this need to change? Could it mean that we're accidentally drawing subpixel in the shadow or something like that?

This comment has been minimized.

@lsalzman

lsalzman Dec 15, 2017

Author Contributor

That is one part of it, yes. Then the further part is that due to differing preblending applied depending on text color, it won't necessarily match either even with grayscale. The only mode that can actually match on all backends for a fast shadow of a given color and normal text of that color, is mono, essentially.

This comment has been minimized.

@glennw

glennw Dec 15, 2017

Member

@gankro do you see any issues with this change?

options(disable-aa) == shadow-ordering.yaml shadow-ordering-ref.yaml
# enable when synthetic-italics implemented on mac
platform(linux) != synthetic-italics.yaml synthetic-italics-ref.yaml
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.