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

Allow subpixel with specified background color in offscreen targets. #2701

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

Always

Just for now

@@ -224,6 +224,10 @@ impl FontInstance {
platform_options: Option<FontInstancePlatformOptions>,
variations: Vec<FontVariation>,
) -> Self {
// If a background color is enabled, it only makes sense
// for it to be completely opaque.
debug_assert!(bg_color.a == 0 || bg_color.a == 255);

FontInstance {
font_key,
size,
@@ -728,7 +728,7 @@ impl TextRunPrimitiveCpu {
display_list: &BuiltDisplayList,
frame_building_state: &mut FrameBuildingState,
) {
if !allow_subpixel_aa {
if !allow_subpixel_aa && self.font.bg_color.a == 0 {

This comment has been minimized.

@Gankra

Gankra Apr 30, 2018

Contributor

shouldn't this be < 1.0 ?

This comment has been minimized.

@glennw

glennw Apr 30, 2018

Author Member

We use the != 0 check in a few places, and the comment where the field is declared says:

    /// When bg_color.a is != 0 and render_mode is FontRenderMode::Subpixel,
    /// the text will be rendered with bg_color.r/g/b as an opaque estimated
    /// background color.

So I think it's fine to leave it as is? Perhaps I should add an assert that the alpha in this case is either 0 or 255 though, since any other value doesn't really make sense here?

This comment has been minimized.

@Gankra

Gankra Apr 30, 2018

Contributor

yeah, the assert sounds like a good compromise!

This comment has been minimized.

@glennw

glennw Apr 30, 2018

Author Member

OK, rebased + added an assert in FontInstance::new.

self.font.render_mode = self.font.render_mode.limit_by(FontRenderMode::Alpha);
}

@@ -2891,6 +2891,13 @@ impl Renderer {
}

for batch in &alpha_batch_container.alpha_batches {
self.shaders
.get(&batch.key)
.bind(
&mut self.device, projection,
&mut self.renderer_errors,
);

if batch.key.blend_mode != prev_blend_mode {
match batch.key.blend_mode {
BlendMode::None => {
@@ -2924,13 +2931,6 @@ impl Renderer {
prev_blend_mode = batch.key.blend_mode;
}

self.shaders
.get(&batch.key)
.bind(
&mut self.device, projection,
&mut self.renderer_errors,
);

// Handle special case readback for composites.
if let BatchKind::Brush(BrushBatchKind::MixBlend { task_id, source_id, backdrop_id }) = batch.key.kind {
// composites can't be grouped together because
@@ -0,0 +1,8 @@
---
root:
items:
- text: "A"
origin: 30 220
size: 200
color: black
font: "FreeSans.ttf"
@@ -0,0 +1,15 @@
# verify that drawing a text run on an off-screen surface with a
# specified background color gives the same result as drawing a
# subpixel text run directly on the background.
---
root:
items:
- type: stacking-context
transform-style: preserve-3d
items:
- text: "A"
origin: 30 220
size: 200
color: black
font: "FreeSans.ttf"
bg-color: white
@@ -58,3 +58,4 @@ platform(linux) == two-shadows.yaml two-shadows.png
fuzzy(1,68) platform(linux) == shadow-transforms.yaml shadow-transforms.png
fuzzy(1,71) platform(linux) == raster-space.yaml raster-space.png
!= allow-subpixel.yaml allow-subpixel-ref.yaml
== bg-color.yaml bg-color-ref.yaml
@@ -467,6 +467,7 @@ impl Wrench {
size: Au,
flags: FontInstanceFlags,
render_mode: Option<FontRenderMode>,
bg_color: Option<ColorU>,
) -> FontInstanceKey {
let key = self.api.generate_font_instance_key();
let mut update = ResourceUpdates::new();
@@ -475,6 +476,9 @@ impl Wrench {
if let Some(render_mode) = render_mode {
options.render_mode = render_mode;
}
if let Some(bg_color) = bg_color {
options.bg_color = bg_color;
}
update.add_font_instance(key, font_key, size, Some(options), None, Vec::new());
self.api.update_resources(update);
key
@@ -198,7 +198,7 @@ pub struct YamlFrameReader {
image_map: HashMap<(PathBuf, Option<i64>), (ImageKey, LayoutSize)>,

fonts: HashMap<FontDescriptor, FontKey>,
font_instances: HashMap<(FontKey, Au, FontInstanceFlags), FontInstanceKey>,
font_instances: HashMap<(FontKey, Au, FontInstanceFlags, Option<ColorU>), FontInstanceKey>,
font_render_mode: Option<FontRenderMode>,
allow_mipmaps: bool,

@@ -544,19 +544,21 @@ impl YamlFrameReader {
&mut self,
font_key: FontKey,
size: Au,
bg_color: Option<ColorU>,
flags: FontInstanceFlags,
wrench: &mut Wrench,
) -> FontInstanceKey {
let font_render_mode = self.font_render_mode;

*self.font_instances
.entry((font_key, size, flags))
.entry((font_key, size, flags, bg_color))
.or_insert_with(|| {
wrench.add_font_instance(
font_key,
size,
flags,
font_render_mode,
bg_color,
)
})
}
@@ -1117,6 +1119,8 @@ impl YamlFrameReader {
) {
let size = item["size"].as_pt_to_au().unwrap_or(Au::from_f32_px(16.0));
let color = item["color"].as_colorf().unwrap_or(*BLACK_COLOR);
let bg_color = item["bg-color"].as_colorf().map(|c| c.into());

let mut flags = FontInstanceFlags::empty();
if item["synthetic-italics"].as_bool().unwrap_or(false) {
flags |= FontInstanceFlags::SYNTHETIC_ITALICS;
@@ -1146,6 +1150,7 @@ impl YamlFrameReader {
let font_key = self.get_or_create_font(desc, wrench);
let font_instance_key = self.get_or_create_font_instance(font_key,
size,
bg_color,
flags,
wrench);

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.