Bug 131144 – Subpixel rendering: Move background images to device pixel boundaries.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

Do not round background image size/position to integral.

[zalan]

Created attachment 228461 [details]
Patch

[Build Bot]

Comment on attachment 228461 [details]
Patch

Attachment 228461 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4982710040788992

New failing tests:
platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
fast/backgrounds/hidpi-bitmap-background-on-subpixel-position.html
platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
css3/background/background-repeat-space-content.html
css3/masking/mask-repeat-space-content.html
css3/masking/mask-repeat-round-content.html
css3/background/background-repeat-round-content.html
platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

[Build Bot]

Created attachment 228464 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 228461 [details]
Patch

Attachment 228461 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4877718491496448

New failing tests:
css3/masking/mask-repeat-space-padding.html
css3/masking/mask-repeat-space-content.html
css3/background/background-repeat-space-padding.html
css3/background/background-repeat-space-content.html
css3/masking/mask-repeat-round-content.html
css3/background/background-repeat-round-content.html
fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

[Build Bot]

Created attachment 228469 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Simon Fraser (smfr)]

Comment on attachment 228461 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228461&action=review

> Source/WebCore/platform/graphics/LayoutPoint.h:191
> +inline FloatSize pixelSnappedForPainting(const LayoutSize& s, const LayoutPoint& p, float pixelSnappingFactor)
> +{
> +    return FloatSize(snapSizeToDevicePixel(s.width(), p.x(), pixelSnappingFactor), snapSizeToDevicePixel(s.height(), p.y(), pixelSnappingFactor));
> +}

Is this any different from snapping a rect with location p and size s and reading the size?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1051
> +void RenderBoxModelObject::pixelSnappedBackgroundImageGeometryForPainting(BackgroundImageGeometry& geometry) const

I would call this pixelSnapBackgroundImageGeometryForPainting().

pixelSnappedBackgroundImageGeometryForPainting would be like:
BackgroundImageGeometry pixelSnappedBackgroundImageGeometryForPainting(const BackgroundImageGeometry&)

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1142
> +        int nrTiles = std::max(1, roundToInt(positioningAreaSize.width() / fillTileSize.width()));

Maybe nrTiles -> numTiles while you're here.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1154
> +        int nrTiles = std::max(1, roundToInt(positioningAreaSize.height() / fillTileSize.height()));

Ditto.

> LayoutTests/css3/background/background-repeat-round-border-expected.html:12
> -                background-size: 111px 85px;
> +                background-size: 111px 85.5px;

Hmm, should we have half pixels in non-Retina situations?

[Build Bot]

Comment on attachment 228461 [details]
Patch

Attachment 228461 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4650390167486464

New failing tests:
css3/masking/mask-repeat-space-padding.html
css3/masking/mask-repeat-space-content.html
css3/background/background-repeat-space-padding.html
css3/background/background-repeat-space-content.html
css3/masking/mask-repeat-round-content.html
css3/background/background-repeat-round-content.html
fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

[Build Bot]

Created attachment 228477 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[zalan]

Created attachment 228513 [details]
Patch

[zalan]

Comment on attachment 228513 [details]
Patch

EWS

[zalan]

(In reply to comment #6)
> (From update of attachment 228461 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228461&action=review
> 
> > Source/WebCore/platform/graphics/LayoutPoint.h:191
> > +inline FloatSize pixelSnappedForPainting(const LayoutSize& s, const LayoutPoint& p, float pixelSnappingFactor)
> > +{
> > +    return FloatSize(snapSizeToDevicePixel(s.width(), p.x(), pixelSnappingFactor), snapSizeToDevicePixel(s.height(), p.y(), pixelSnappingFactor));
> > +}
> 
> Is this any different from snapping a rect with location p and size s and reading the size?
No, there should not be any difference. I added this function to mirror the int snap version, but I think you are right, there's no need to introduce this extra helper to achieve the snapping for sizes. -removed.
 
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:1051
> > +void RenderBoxModelObject::pixelSnappedBackgroundImageGeometryForPainting(BackgroundImageGeometry& geometry) const
> 
> I would call this pixelSnapBackgroundImageGeometryForPainting().
Done.

> 
> pixelSnappedBackgroundImageGeometryForPainting would be like:
> BackgroundImageGeometry pixelSnappedBackgroundImageGeometryForPainting(const BackgroundImageGeometry&)
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:1142
> > +        int nrTiles = std::max(1, roundToInt(positioningAreaSize.width() / fillTileSize.width()));
> 
> Maybe nrTiles -> numTiles while you're here.
> 
> > Source/WebCore/rendering/RenderBoxModelObject.cpp:1154
> > +        int nrTiles = std::max(1, roundToInt(positioningAreaSize.height() / fillTileSize.height()));
> 
> Ditto.
> 
Done.

> > LayoutTests/css3/background/background-repeat-round-border-expected.html:12
> > -                background-size: 111px 85px;
> > +                background-size: 111px 85.5px;
> 
> Hmm, should we have half pixels in non-Retina situations?
Yea, that got me thinking too. I looked into the failing cases and noticed their values are very fragile. I changed them to be stable.

[Build Bot]

Comment on attachment 228513 [details]
Patch

Attachment 228513 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6755172176887808

New failing tests:
platform/mac/fast/scrolling/scroll-iframe-latched-mainframe.html
fast/backgrounds/hidpi-bitmap-background-on-subpixel-position.html
platform/mac/fast/scrolling/scroll-select-latched-mainframe.html
css3/background/background-repeat-space-content.html
platform/mac/fast/scrolling/scroll-div-latched-mainframe.html
fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

[Build Bot]

Created attachment 228522 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 228513 [details]
Patch

Attachment 228513 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6562182384844800

New failing tests:
css3/background/background-repeat-space-content.html
media/adopt-node-crash.html
fast/backgrounds/hidpi-bitmap-background-on-subpixel-position.html
fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

[Build Bot]

Created attachment 228528 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[zalan]

Created attachment 228538 [details]
Patch

[zalan]

Comment on attachment 228538 [details]
Patch

EWS

[zalan]

Created attachment 228543 [details]
Patch

[zalan]

Comment on attachment 228543 [details]
Patch

EWS

[Build Bot]

Comment on attachment 228543 [details]
Patch

Attachment 228543 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6721347161948160

New failing tests:
media/adopt-node-crash.html

[Build Bot]

Created attachment 228550 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[WebKit Commit Bot]

Comment on attachment 228543 [details]
Patch

Rejecting attachment 228543 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 228543, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ap-background-repeat-on-subpixel-position-expected.html
patching file LayoutTests/fast/backgrounds/hidpi-bitmap-background-repeat-on-subpixel-position.html
patching file LayoutTests/fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position-expected.html
patching file LayoutTests/fast/backgrounds/hidpi-generated-gradient-background-on-subpixel-position.html

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/6747390769889280

[zalan]

Created attachment 228591 [details]
Patch

[zalan]

Comment on attachment 228591 [details]
Patch

EWS

[WebKit Commit Bot]

Comment on attachment 228591 [details]
Patch

Clearing flags on attachment: 228591

Committed r166784: <http://trac.webkit.org/changeset/166784>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #25)
> (From update of attachment 228591 [details])
> Clearing flags on attachment: 228591
> 
> Committed r166784: <http://trac.webkit.org/changeset/166784>

This caused bug 131924.