New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 594580 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Background cover don't match the padding-box

Reported by mem...@lenglet.name, Mar 14 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0

Example URL:
https://jsfiddle.net/a46rL16c/

Steps to reproduce the problem:
1. Create an element
2. Set a background cover image
3. Set a background color or a border

What is the expected behavior?
The background image should cover entirely the padding-box 

What went wrong?
A tiny gap (1csspx line) appears for specific sizes.
The provided example show it for the following dimensions.
with a background image: 374 × 120, and the padding box (non exhaustive list):
- 472 × 150
- 479 × 150
- 481 × 150
- 488 × 150
- 495 × 150
- 502 × 150
- 509 × 150

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? Yes v48

Does this work in other browsers? Yes 

Chrome version: <Copy from: 'about:version'>  Channel: n/a
OS Version: OS X 10.11
Flash Version: Shockwave Flash 21.0 r0

Notice the red line on the jsfiddle example

Appears on v49, v50 (beta), v51 (canary)
 
Capture d’écran 2016-03-14 à 17.09.33.png
10.5 KB View Download
Cc: kavvaru@chromium.org
Labels: -Type-Bug M-51 hasbisect OS-Linux OS-Windows Type-Bug-Regression
Owner: le...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.11.3 using chrome version 49.0.2623.87 and canary 51.0.2677.0.Observed the red line at the end of the box.

Please find the bisect information as below
Narrow Bisect::
Good::49.0.2585.0  --  (official build 363690)
Bad:: 49.0.2586.0  --  (official build 363935)

CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/42290cb91a66566637a1715737b1961a8480b934..b5daa8ef58d6f02e446ad19cc44dc2c0d192db98

Possible suspect from the above CL
https://codereview.chromium.org/1504463002

leviw @ Could you please look into this issue if it is related to your change,else please route this to an appropriate owner for this issue.

Thanks,

Components: -Blink Blink>Layout
Owner: schenney@chromium.org
I think I just landed a fix for this. See https://bugs.chromium.org/p/chromium/issues/detail?id=593861
Nope, not fixed. I'll sort it out.
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f65f0639cf80474313ad86ed43a9216a6991dd3

commit 3f65f0639cf80474313ad86ed43a9216a6991dd3
Author: schenney <schenney@chromium.org>
Date: Fri Mar 18 19:53:22 2016

Fix for lack of cover with background-size: cover

We were sizing images for background-size:cover using floating
point computations, then putting the result into a LayoutUnit
then rounding down to align to pixels. The floating point
approximation might give 99.997 instead of 100.0, then the
conversion to LayoutUnit would make that 99.825 or something,
which would then be rounded to 99, leaving a 1 pixel gap when
the image should fill a 100 pixels wide space. The result is
the background color appearing behind the image.

This patch re-arranges the computation to avoid any rounding,
making the dimension of interest exactly match the target rect.

R=leviw@chromium.org
BUG= 594580 

Review URL: https://codereview.chromium.org/1811943003

Cr-Commit-Position: refs/heads/master@{#382055}

[add] https://crrev.com/3f65f0639cf80474313ad86ed43a9216a6991dd3/third_party/WebKit/LayoutTests/fast/backgrounds/background-cover-rounding-expected.html
[add] https://crrev.com/3f65f0639cf80474313ad86ed43a9216a6991dd3/third_party/WebKit/LayoutTests/fast/backgrounds/background-cover-rounding.html
[modify] https://crrev.com/3f65f0639cf80474313ad86ed43a9216a6991dd3/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp

Comment 7 Deleted

Worth merging this back to m50?

Comment 9 by le...@chromium.org, Mar 18 2016

I'd say it's worth it and safe enough.
Labels: Merge-Request-50

Comment 11 by tin...@google.com, Mar 21 2016

Labels: -Merge-Request-50 Merge-Approved-50 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M50 (branch: 2661)
Status: Fixed (was: Started)
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 21 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecc5e56ebccf2a0f4f1dea476b3a9d46194d52e7

commit ecc5e56ebccf2a0f4f1dea476b3a9d46194d52e7
Author: Stephen Chenney <schenney@chromium.org>
Date: Mon Mar 21 15:16:07 2016

Fix for lack of cover with background-size: cover

Merge for m50.

We were sizing images for background-size:cover using floating
point computations, then putting the result into a LayoutUnit
then rounding down to align to pixels. The floating point
approximation might give 99.997 instead of 100.0, then the
conversion to LayoutUnit would make that 99.825 or something,
which would then be rounded to 99, leaving a 1 pixel gap when
the image should fill a 100 pixels wide space. The result is
the background color appearing behind the image.

This patch re-arranges the computation to avoid any rounding,
making the dimension of interest exactly match the target rect.

TBR=leviw@chromium.org
BUG= 594580 

Review URL: https://codereview.chromium.org/1811943003

Cr-Commit-Position: refs/heads/master@{#382055}
(cherry picked from commit 3f65f0639cf80474313ad86ed43a9216a6991dd3)

Review URL: https://codereview.chromium.org/1823593003 .

Cr-Commit-Position: refs/branch-heads/2661@{#309}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[add] https://crrev.com/ecc5e56ebccf2a0f4f1dea476b3a9d46194d52e7/third_party/WebKit/LayoutTests/fast/backgrounds/background-cover-rounding-expected.html
[add] https://crrev.com/ecc5e56ebccf2a0f4f1dea476b3a9d46194d52e7/third_party/WebKit/LayoutTests/fast/backgrounds/background-cover-rounding.html
[modify] https://crrev.com/ecc5e56ebccf2a0f4f1dea476b3a9d46194d52e7/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp

Comment 14 by ajha@chromium.org, Mar 22 2016

Labels: TE-Verified-M50 TE-Verified-50.0.2661.48
Verified the merge on the latest M-50(50.0.2661.48- 2661@{#335}) on Windows-7, Mac OS 10.11.3 and Linux Ubuntu 14.04. This is working as intended and there is no red line seen on the border of the box.

Sign in to add a comment