Bug 241800 - Wrong image display for Non-integer sized elements and background-position specified as %
Summary: Wrong image display for Non-integer sized elements and background-position sp...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-06-21 03:03 PDT by Andrew Collier
Modified: 2022-06-24 21:47 PDT (History)
4 users (show)

See Also:


Attachments
webkit rendering at 1x (44.54 KB, image/png)
2022-06-21 03:03 PDT, Andrew Collier
no flags Details
webkit rendering at 2x (164.65 KB, image/png)
2022-06-21 03:05 PDT, Andrew Collier
no flags Details
correct 2x rendering by Edge (157.81 KB, image/png)
2022-06-21 03:05 PDT, Andrew Collier
no flags Details
Test reduction (957 bytes, text/html)
2022-06-22 08:20 PDT, zalan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Collier 2022-06-21 03:03:56 PDT
Created attachment 460367 [details]
webkit rendering at 1x

If all of:

 * the size of an element has non-integer size in pixels,
 * `background-position` is specified as a percentage
 * `background-position` is specified in units of pixels

then webkit renders the wrong part of the background image.


Tested with 251689@main, Safari Technology Preview Release 146 (Safari 15.4, WebKit 17614.1.14.10.6), Safari iPhone 15.5

This issue occurs in the production site boardgamearena.com - which is rather too complex to use as a demo, but in summary: many of the 400 games there draw their sprites incorrectly in Safari (desktop and mobile) because of this issue. 

Demo

1. Visit https://jsfiddle.net/shadowphiar/yLq36mzt/

Observe the results area in the bottom right. 

The four boxes are drawn by selecting a small area of a large background image (as one might do to combine many small icons into a single large image which is requested only once and cached). The boxes should each display the 125th square from a row of 126 squares. This is defined by a square div and a background-position: -12400% 0%;.
The left two boxes, the background-size is defined as a percentage (126 times the size of the displayed div). The right two boxes it is defined in pixels, calculated as 126 times the size of the displayed div. These should be the same size.
The top two boxes are drawn with a png image (fetched), the bottom two with an svg image (embedded in the css). The images are the same apart from file format.

Expected Result: There should be four identical orange boxes each with a white outline.

Actual Result:

Safari, including Safari Technology Preview Release 140 (Safari 15.4, WebKit 17614.1.1.5), display the boxes on the right incorrectly - the white border is somewhere in the middle of the square.

2. Open Chrome, or Microsoft Edge, and visit the same page. Observe that the four boxes look identical (or very close to being identical. The top right box may be off by a pixel or two).

3. Open Firefox and visit the same page. Observe the the top left and right boxes are displayed correctly. [But, for a different reason, the bottom boxes are wrong]

4. Back in Safari, enter responsive design mode and change the view between 1x, 2x and 3x modes. (Or drag the window from a Retina display to a non-retina external monitor).  Observe that the position of the white line inside the boxes changes. Observe that the card image below the boxes is incorrectly clipped, and shows another card’s artwork on the left or right side (on the right size at 1x, and on the left side at 2x)

Hypothesis:
This appears to occur when the size of the element is a non-integer number of pixels. It looks like Safari, when it calculates background-position or background-size property as a percentage of the element’s height or width, is rounding the div width to an integer.
If background-size is not specified with the same calculation path, and specifically one in which that rounding step did not occur, then the part of the image actually displayed can vary. At large percentages, half a pixel in the size of the div can result in really quite a significant difference in the portion of the image that gets displayed.
Comment 1 Andrew Collier 2022-06-21 03:05:04 PDT
Created attachment 460368 [details]
webkit rendering at 2x
Comment 2 Andrew Collier 2022-06-21 03:05:44 PDT
Created attachment 460369 [details]
correct 2x rendering by Edge
Comment 3 zalan 2022-06-22 08:20:46 PDT
Created attachment 460416 [details]
Test reduction

Webkit computes the same position for 0px - 0.25px, 0.25px - 0.75px and 0.75px - 1px ranges. It looks like we use a "pixel snapped" value (0px, 0.5px and 1px on a 2x display) when computing the background position as opposed to just using the "raw" value.
Comment 4 zalan 2022-06-22 16:19:19 PDT
Yeah, so we apparently pixel-snap too early in the process of generating background images.

@@ -1258,7 +1258,6 @@ BackgroundImageGeometry RenderBoxModelObject::calculateBackgroundImageGeometry(c
             }
         } else {
             positioningAreaSize = borderBoxRect.size() - LayoutSize(left + right, top + bottom);
-            positioningAreaSize = LayoutSize(snapRectToDevicePixels(LayoutRect(paintOffset, positioningAreaSize), deviceScaleFactor).size());
         }
     } else {
         LayoutRect viewportRect;

^^ fixes the issue.
Comment 5 zalan 2022-06-22 17:10:26 PDT
> ^^ fixes the issue.
the png background painting but not the svg.
Comment 6 zalan 2022-06-22 20:48:57 PDT
(In reply to zalan from comment #4)
> Yeah, so we apparently pixel-snap too early in the process of generating
> background images.
> 
> @@ -1258,7 +1258,6 @@ BackgroundImageGeometry
> RenderBoxModelObject::calculateBackgroundImageGeometry(c
>              }
>          } else {
>              positioningAreaSize = borderBoxRect.size() - LayoutSize(left +
> right, top + bottom);
> -            positioningAreaSize =
> LayoutSize(snapRectToDevicePixels(LayoutRect(paintOffset,
> positioningAreaSize), deviceScaleFactor).size());
>          }
>      } else {
>          LayoutRect viewportRect;
> 
> ^^ fixes the issue.
Sadly even with this change, the rendered content is slightly different from what Blink and Gecko have. Due to the LayoutUnit's fixed point denominator (64),  32.15234234px is resolved to 34.140625px which makes the background image shift slightly to the right.
Comment 7 Sam Sneddon [:gsnedders] 2022-06-23 07:12:57 PDT
rdar://89549731
Comment 8 zalan 2022-06-24 21:47:09 PDT
(In reply to zalan from comment #5)
> > ^^ fixes the issue.
> the png background painting but not the svg.
It also breaks bug 144986 (broken in Chrome too). I think someone with image background (swidt) should be looking into this.