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

Issue 683789 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 578252



Sign in to add a comment

CSS Paint API: Browser zoom affects geometry

Project Member Reported by dstockwell@chromium.org, Jan 23 2017

Issue description

http://jsbin.com/kamuririgo/1/edit?html,js,output

When I zoom to 33%, the geom passed to the paint function changes from 200x200 to 199x199. My device pixel ratio is 1.7 at 100%.
 
Owner: jstenback@chromium.org
Taking this issue.
Cc: ikilpatrick@chromium.org
The fundamental problem here is that the CSS Paint API code is given a size of the region to pain, which it then translates to the geometry that is passed to the paint API. That give size has by that point already been scaled up or down depending on the zoom level, and possibly by device pixel ratios as well, so the given size is scaled back by the inverse of the zoom factor. The given size at that point is an integer, and thus we don't have enough precision to compute the exact original geometry and we end up exposing rounding errors through the CSS Paint API. There are various approaches to fixing that and I'm slowly but surely exploring them to find the one that makes the most sense.
Shouldn't the bounds include device pixel ratio and zoom? That is what
the C++ paint code does (plus snapping). IOW with a DPR of 1.7, it'd be:

zoom=100% paint bounds: 200 * 1.7 = 340x on each side
zoom=33% pant bounds: 0.33 * 200 * 1.7 = 112.2px on each side

Since 112.2px is not an integer, we should paint into the pixelSnappedIntRect of it.

WDYT?
@chris - so we spec'd this such that the pixels are "CSS" pixels, not internal pixels. I.e.

<div style="background: paint(foo); width: 50px; height: 50px;"></div>
DPR = 1.7

Will result in a size of (50x50). Underneath the image buffer will have a size of 85x85, but we pre-scale the canvas for you.

This is primarily because everything through the styleMap is in CSS pixels.

We could give web developers additional information here, (like DPR / SF / nativeWidth)? But would still want to have things by default in CSS pixels for simplicity.
I see your point, however: if no provision is made to paint in device pixels,
then custom paint will always have poor quality in some situations on screens
with non-integer DPR, or equivalent zoom settings.

Also: there are even more complications than that in the presence of transform,
which is not communicated either to the paint code, even for non-custom
objects.

Finally, I don't see how you will be able to get around pixel snapping in order
to have elements paint into the bounds they are given. These elements need to
snap consistently with others around them.

If it's desirable to stick with CSS pixels, I conclude from the paragraph above
that this is probably a WontFix bug. ?
So we tried with device pixels initially with the implementation - and it confused folks who tried it.

This isn't quite WontFix as there is an underlying bug - it a precision loss in the conversions with the sizes.
I.e. LayoutSize -> FloatSize -> IntSize and I think jstenback@ confirmed that this was fixed with a direct LayoutSize -> IntSize conversion?
(jstenback@ can confirm).




There might be a precision issue, but sometimes the box really will be 199px
because of pixel snapping, no?
Cc: jstenback@chromium.org
Owner: ----
Status: Available (was: Assigned)
Looks like jstenback@ is no longer active. If that's incorrect, reassign.
Cc: xidac...@chromium.org
Hi xidachen@, 

ccing you as an FYI
Cc: -xidac...@chromium.org
Owner: xidac...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 28 2017

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

commit b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Sep 28 20:55:01 2017

[PaintWorklet] Fix geometry when browser zooms

It is discovered in the bug that when the device pixel ratio is 1.7 and
the browser zoom is 1/3, the geometry of the background is 199*199 where
it should be 200*200. There are two reasons:
1. The ApplySubPixelHeuristicToImageSize in BackgroundImageGeometry
   makes the fractional part lost.
2. The conversion from FloatSize to IntSize also has impact.

This CL caches a logical_tile_size_ in the BackgroundImageGeometry, which
is the tile size without applying subpixel heuristics. And then we pass this
size all the way to the CSSPaintDefinition::Paint.

Bug:  683789 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4a24d7afe5408d3fbfbef4b63e3c221f13ed1612
Reviewed-on: https://chromium-review.googlesource.com/678019
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505150}
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/LayoutTests/http/tests/csspaint/hidpi/geometry-with-hidpi-zoom-expected.html
[add] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/LayoutTests/http/tests/csspaint/hidpi/geometry-with-hidpi-zoom.html
[add] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/LayoutTests/virtual/scalefactor200/http/tests/csspaint/hidpi/README.txt
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/css/CSSImageGeneratorValue.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/css/CSSImageGeneratorValue.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/css/CSSPaintImageGenerator.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/css/CSSPaintValue.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/css/CSSPaintValue.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/layout/shapes/ShapeOutsideInfo.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/paint/BackgroundImageGeometry.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/paint/ListMarkerPainter.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/paint/NinePieceImagePainter.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleFetchedImage.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleFetchedImage.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleFetchedImageSet.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleFetchedImageSet.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleGeneratedImage.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleGeneratedImage.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleImage.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StyleInvalidImage.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/core/style/StylePendingImage.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.h
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/PaintWorklet.cpp
[modify] https://crrev.com/b6d27042dd93aabdddcdddddffefe6a0f7d3a7a1/third_party/WebKit/Source/modules/csspaint/PaintWorklet.h

Status: Fixed (was: Assigned)

Sign in to add a comment