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

Unify background placement code #19651

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

Always

Just for now

Prev

Test with fixes for background repeat: spaced

Test the interaction between background-repeat: spaced
and CSS borders. Fix tile_image_axis() function.
  • Loading branch information
pyfisch committed Jan 2, 2018
commit cf12e27364c8fe29d07ba3ba011f856c344546b8
@@ -1047,52 +1047,55 @@ fn compute_background_image_size(bg_size: BackgroundSize<LengthOrPercentageOrAut
bounds_size: Size2D<Au>,
intrinsic_size: Option<Size2D<Au>>)
-> Size2D<Au> {
let own_size = if let Some(size) = intrinsic_size {
size
} else {
return match bg_size {
BackgroundSize::Cover | BackgroundSize::Contain => bounds_size,
BackgroundSize::Explicit { width, height } => {
Size2D::new(
MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(bounds_size.width),
MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(bounds_size.height))
match intrinsic_size {
None => {
match bg_size {

This comment has been minimized.

Copy link
@emilio

emilio Jan 1, 2018

Member

Well, I would've keep the early return, just using match, but it's not a big deal, this looks fine too, thanks!

BackgroundSize::Cover | BackgroundSize::Contain => bounds_size,
BackgroundSize::Explicit { width, height } => {
Size2D::new(
MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(bounds_size.width),
MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(bounds_size.height)
)
}
}
}
};
// If `image_aspect_ratio` < `bounds_aspect_ratio`, the image is tall; otherwise, it is
// wide.
let image_aspect_ratio = own_size.width.to_f32_px() / own_size.height.to_f32_px();
let bounds_aspect_ratio = bounds_size.width.to_f32_px() / bounds_size.height.to_f32_px();
match (bg_size, image_aspect_ratio < bounds_aspect_ratio) {
(BackgroundSize::Contain, false) | (BackgroundSize::Cover, true) => {
Size2D::new(bounds_size.width,
bounds_size.width.scale_by(image_aspect_ratio.recip()))
}
Some(own_size) => {
// If `image_aspect_ratio` < `bounds_aspect_ratio`, the image is tall; otherwise, it is
// wide.
let image_aspect_ratio = own_size.width.to_f32_px() / own_size.height.to_f32_px();
let bounds_aspect_ratio = bounds_size.width.to_f32_px() / bounds_size.height.to_f32_px();
match (bg_size, image_aspect_ratio < bounds_aspect_ratio) {
(BackgroundSize::Contain, false) | (BackgroundSize::Cover, true) => {
Size2D::new(bounds_size.width,
bounds_size.width.scale_by(image_aspect_ratio.recip()))
}

(BackgroundSize::Contain, true) | (BackgroundSize::Cover, false) => {
Size2D::new(bounds_size.height.scale_by(image_aspect_ratio),
bounds_size.height)
}
(BackgroundSize::Contain, true) | (BackgroundSize::Cover, false) => {
Size2D::new(bounds_size.height.scale_by(image_aspect_ratio),
bounds_size.height)
}

(BackgroundSize::Explicit { width, height: LengthOrPercentageOrAuto::Auto }, _) => {
let width = MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(own_size.width);
Size2D::new(width, width.scale_by(image_aspect_ratio.recip()))
}
(BackgroundSize::Explicit { width, height: LengthOrPercentageOrAuto::Auto }, _) => {
let width = MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(own_size.width);
Size2D::new(width, width.scale_by(image_aspect_ratio.recip()))
}

(BackgroundSize::Explicit { width: LengthOrPercentageOrAuto::Auto, height }, _) => {
let height = MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(own_size.height);
Size2D::new(height.scale_by(image_aspect_ratio), height)
}
(BackgroundSize::Explicit { width: LengthOrPercentageOrAuto::Auto, height }, _) => {
let height = MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(own_size.height);
Size2D::new(height.scale_by(image_aspect_ratio), height)
}

(BackgroundSize::Explicit { width, height }, _) => {
Size2D::new(MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(own_size.width),
MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(own_size.height))
(BackgroundSize::Explicit { width, height }, _) => {
Size2D::new(MaybeAuto::from_style(width, bounds_size.width)
.specified_or_default(own_size.width),
MaybeAuto::from_style(height, bounds_size.height)
.specified_or_default(own_size.height))
}
}
}
}
}
@@ -1155,6 +1158,10 @@ fn tile_image(position: &mut Au,
absolute_anchor_origin: Au,
image_size: Au) {
// Avoid division by zero below!
// Images with a zero width or height are not displayed.
// Therefore the positions do not matter and can be left unchanged.
// NOTE: A possible optimization is not to build
// display items in this case at all.
if image_size == Au(0) {

This comment has been minimized.

Copy link
@emilio

emilio Dec 31, 2017

Member

I think it'd make sense to comment why the outparams are sane in this case, or why it doesn't matter..

return
}
@@ -1185,17 +1192,35 @@ fn tile_image_axis(repeat: BackgroundRepeatKeyword,
BackgroundRepeatKeyword::NoRepeat => {
*position += offset;
*size = *tile_size;
return
}
BackgroundRepeatKeyword::Repeat => (),
BackgroundRepeatKeyword::Space => tile_image_spaced(
position, size, tile_spacing, absolute_anchor_origin, *tile_size),
BackgroundRepeatKeyword::Round => tile_image_round(
position, size, absolute_anchor_origin, tile_size),
};
*position = clip_origin;
*size = clip_size;
tile_image(position, size, absolute_anchor_origin, *tile_size);
BackgroundRepeatKeyword::Repeat => {
*position = clip_origin;
*size = clip_size;
tile_image(position, size, absolute_anchor_origin, *tile_size);
}
BackgroundRepeatKeyword::Space => {
tile_image_spaced(
position,
size,
tile_spacing,
absolute_anchor_origin,
*tile_size);
let combined_tile_size = *tile_size + *tile_spacing;
*position = clip_origin;
*size = clip_size;
tile_image(position, size, absolute_anchor_origin, combined_tile_size);
}
BackgroundRepeatKeyword::Round => {
tile_image_round(
position,
size,
absolute_anchor_origin,
tile_size);
*position = clip_origin;
*size = clip_size;
tile_image(position, size, absolute_anchor_origin, *tile_size);
}
}
}

impl FragmentDisplayListBuilding for Fragment {
@@ -1411,7 +1436,8 @@ impl FragmentDisplayListBuilding for Fragment {
&mut tile_spacing.width,
pos_x,
css_clip.origin.x,
css_clip.size.width);
css_clip.size.width
);
tile_image_axis(
bg_repeat.1,
&mut bounds.origin.y,
@@ -1420,9 +1446,10 @@ impl FragmentDisplayListBuilding for Fragment {
&mut tile_spacing.height,
pos_y,
css_clip.origin.y,
css_clip.size.height);
css_clip.size.height
);

return BackgroundPlacement { bounds, tile_size, tile_spacing, css_clip }
BackgroundPlacement { bounds, tile_size, tile_spacing, css_clip }
}

fn build_display_list_for_webrender_image(&self,
@@ -1438,7 +1465,12 @@ impl FragmentDisplayListBuilding for Fragment {
Au::from_px(webrender_image.width as i32),
Au::from_px(webrender_image.height as i32));
let placement = self.compute_background_placement(
state, style, absolute_bounds, Some(image), index);
state,
style,
absolute_bounds,
Some(image),
index
);

// Create the image display item.
let base = state.create_base_display_item(&placement.bounds,
{}
]
],
"css/css-backgrounds/background-repeat/gradient-repeat-spaced-with-borders.html": [
[
"/css/css-backgrounds/background-repeat/gradient-repeat-spaced-with-borders.html",
[
[
"/css/css-backgrounds/background-repeat/reference/gradient-repeat-spaced-with-borders.html",
"=="
]
],
{}
]
],
"css/css-backgrounds/background-size-002.html": [
[
"/css/css-backgrounds/background-size-002.html",
{}
]
],
"css/css-backgrounds/background-repeat/reference/gradient-repeat-spaced-with-borders.html": [
[
{}
]
],
"css/css-backgrounds/background-repeat/reference/support/rectangle-96x60.png": [
[
{}
"0a631e1bd4a8b410edd28a51675207f591e04a55",
"reftest"
],
"css/css-backgrounds/background-repeat/gradient-repeat-spaced-with-borders.html": [
"2880f6e6ab270572d9279d04ca196bfeae30a261",
"reftest"
],
"css/css-backgrounds/background-repeat/reference/background-repeat-no-repeat.xht": [
"3e5eecf9416348440b6d23dc7a817de5ed97ede7",
"support"
"0e0e06ed0f62fbcca2f5a087e807bf2ac74b1ad6",
"support"
],
"css/css-backgrounds/background-repeat/reference/gradient-repeat-spaced-with-borders.html": [
"b6e92428d5562a83424ef050f3c57dfc128a95a2",
"support"
],
"css/css-backgrounds/background-repeat/reference/support/rectangle-96x60.png": [
"36050bffda9382cfd978dc82a2f0244a535a6a46",
"support"

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

@@ -0,0 +1,18 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Tiled gradient with spaces is repeated behind the border.</title>
<link rel="match" href="reference/gradient-repeat-spaced-with-borders.html">
<link rel="help" href="https://drafts.csswg.org/css-backgrounds-3/#valdef-background-repeat-space">
<style>
#foo {
width: 65px;
height: 65px;
border: solid 35px transparent;
background: radial-gradient(transparent 50%, #36c 50%);
background-size: 30px 30px;
background-repeat: space;
}
</style>
<body>
<div id="foo"></div>
</body>
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Spaced Gradient</title>
<style>
#foo {
width: calc(4 * 30px + 3 * 5px);
height: calc(4 * 30px + 3 * 5px);
background: radial-gradient(transparent 50%, #36c 50%);
background-size: 30px 30px;
background-repeat: space;
}
</style>
<body>
<div id="foo"></div>
</body>
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.