| Summary: | Subpixel rendering: Move background images to device pixel boundaries. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | zalan <zalan> |
| Component: | Layout and Rendering | Assignee: | zalan <zalan> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | akkimlove88, buildbot, cmarcelo, commit-queue, esprehn+autocc, glenn, kondapallykalyan, luiz, mitz, noam, rniwa, sergio, simon.fraser |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | |||
| Bug Blocks: | 235900, 131209 | ||
| Attachments: | |||
|
Description
zalan
2014-04-02 20:46:13 PDT
Created attachment 228461 [details]
Patch
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 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
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 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
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? 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 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
Created attachment 228513 [details]
Patch
Comment on attachment 228513 [details]
Patch
EWS
(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. 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 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
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 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
Created attachment 228538 [details]
Patch
Comment on attachment 228538 [details]
Patch
EWS
Created attachment 228543 [details]
Patch
Comment on attachment 228543 [details]
Patch
EWS
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 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
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 Created attachment 228591 [details]
Patch
Comment on attachment 228591 [details]
Patch
EWS
Comment on attachment 228591 [details] Patch Clearing flags on attachment: 228591 Committed r166784: <http://trac.webkit.org/changeset/166784> 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. |