| Summary: | Subpixel rendering: Make BorderEdge/RoundedRect::Radii LayoutUnit aware. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | zalan <zalan> |
| Component: | Layout and Rendering | Assignee: | zalan <zalan> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | buildbot, bunhere, cdumez, commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, jonlee, kondapallykalyan, rniwa, simon.fraser, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | 128056 | ||
| Bug Blocks: | 127524 | ||
| Attachments: | |||
|
Description
zalan
2014-01-31 21:03:15 PST
Created attachment 222875 [details]
Patch
Comment on attachment 222875 [details] Patch Attachment 222875 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6245396569391104 New failing tests: svg/zoom/page/zoom-background-images.html media/video-zoom.html svg/zoom/page/zoom-svg-float-border-padding.xml fast/css/bidi-override-in-anonymous-block.html fast/multicol/span/anonymous-style-inheritance.html ietestcenter/css3/bordersbackgrounds/border-top-left-radius-values-003.htm svg/zoom/page/zoom-background-image-tiled.html svg/zoom/page/zoom-svg-through-object-with-auto-size.html tables/mozilla_expected_failures/bugs/bug1055-2.html svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html ietestcenter/css3/bordersbackgrounds/border-radius-with-three-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-004.htm css1/units/length_units.html ietestcenter/css3/bordersbackgrounds/border-radius-with-two-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-initial-value-001.htm fast/repaint/repaint-during-scroll-with-zoom.html svg/custom/svg-fonts-in-html.html ietestcenter/css3/bordersbackgrounds/border-radius-style-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-002.htm Created attachment 222878 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 222875 [details] Patch Attachment 222875 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6390575053930496 New failing tests: svg/zoom/page/zoom-background-images.html media/video-zoom.html svg/zoom/page/zoom-svg-float-border-padding.xml fast/css/bidi-override-in-anonymous-block.html fast/multicol/span/anonymous-style-inheritance.html ietestcenter/css3/bordersbackgrounds/border-top-left-radius-values-003.htm svg/zoom/page/zoom-background-image-tiled.html svg/zoom/page/zoom-svg-through-object-with-auto-size.html tables/mozilla_expected_failures/bugs/bug1055-2.html svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html ietestcenter/css3/bordersbackgrounds/border-radius-with-three-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-004.htm css1/units/length_units.html ietestcenter/css3/bordersbackgrounds/border-radius-with-two-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-initial-value-001.htm fast/repaint/repaint-during-scroll-with-zoom.html svg/custom/svg-fonts-in-html.html ietestcenter/css3/bordersbackgrounds/border-radius-style-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-002.htm Created attachment 222879 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 222875 [details] Patch Attachment 222875 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6615476721418240 New failing tests: svg/zoom/page/zoom-background-images.html media/video-zoom.html svg/zoom/page/zoom-svg-float-border-padding.xml fast/css/bidi-override-in-anonymous-block.html fast/multicol/span/anonymous-style-inheritance.html ietestcenter/css3/bordersbackgrounds/border-top-left-radius-values-003.htm svg/zoom/page/zoom-background-image-tiled.html svg/zoom/page/zoom-svg-through-object-with-auto-size.html tables/mozilla_expected_failures/bugs/bug1055-2.html svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html ietestcenter/css3/bordersbackgrounds/border-radius-with-three-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-004.htm css1/units/length_units.html ietestcenter/css3/bordersbackgrounds/border-radius-with-two-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-initial-value-001.htm fast/repaint/repaint-during-scroll-with-zoom.html svg/custom/svg-fonts-in-html.html ietestcenter/css3/bordersbackgrounds/border-radius-style-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-002.htm Created attachment 222880 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 222875 [details] Patch Attachment 222875 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6437184039026688 New failing tests: svg/zoom/page/zoom-background-images.html media/video-zoom.html svg/zoom/page/zoom-svg-float-border-padding.xml fast/css/bidi-override-in-anonymous-block.html fast/multicol/span/anonymous-style-inheritance.html ietestcenter/css3/bordersbackgrounds/border-top-left-radius-values-003.htm svg/zoom/page/zoom-background-image-tiled.html svg/zoom/page/zoom-svg-through-object-with-auto-size.html tables/mozilla_expected_failures/bugs/bug1055-2.html svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html ietestcenter/css3/bordersbackgrounds/border-radius-with-three-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-004.htm css1/units/length_units.html ietestcenter/css3/bordersbackgrounds/border-radius-with-two-values-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-initial-value-001.htm fast/repaint/repaint-during-scroll-with-zoom.html svg/custom/svg-fonts-in-html.html ietestcenter/css3/bordersbackgrounds/border-radius-style-001.htm ietestcenter/css3/bordersbackgrounds/border-radius-style-002.htm Created attachment 222881 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 222875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222875&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:1459 > if (fullWidth % 3 == 2) > - outerWidth += 1; > + outerWidth += LayoutUnit::fromPixel(1); > > if (fullWidth % 3 == 1) > - innerWidth += 1; > + innerWidth += LayoutUnit::fromPixel(1); > } Why is this change needed? There’s a += operator on LayoutUnit that works with integers and does fromPixel, right? > Source/WebCore/rendering/RenderBoxModelObject.cpp:2064 > + LayoutUnit inflateValue = LayoutUnit::fromPixel(1); > + outerBorderTopWidth+= inflateValue; > + outerBorderBottomWidth+= inflateValue; > + outerBorderLeftWidth+= inflateValue; > + outerBorderRightWidth+= inflateValue; Why does this need to change at all? LayoutUnit already has an operator++ that should do the right thing. Also, formatting mistakes here. But all those failing tests means maybe this can’t land? (In reply to comment #12) > But all those failing tests means maybe this can’t land? Apparently some pixel snapping is required to make this patch pass. (as it removed implicit integer flooring by going from int to LayoutUnit) (In reply to comment #11) > (From update of attachment 222875 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222875&action=review > > > Source/WebCore/rendering/RenderBoxModelObject.cpp:1459 > > if (fullWidth % 3 == 2) > > - outerWidth += 1; > > + outerWidth += LayoutUnit::fromPixel(1); > > > > if (fullWidth % 3 == 1) > > - innerWidth += 1; > > + innerWidth += LayoutUnit::fromPixel(1); > > } > > Why is this change needed? There’s a += operator on LayoutUnit that works with integers and does fromPixel, right? You're right. Probably there's no need to be explicit here about the LayoutUnit conversion. I'll remove both. > > Source/WebCore/rendering/RenderBoxModelObject.cpp:2064
> > + LayoutUnit inflateValue = LayoutUnit::fromPixel(1);
> > + outerBorderTopWidth+= inflateValue;
> > + outerBorderBottomWidth+= inflateValue;
> > + outerBorderLeftWidth+= inflateValue;
> > + outerBorderRightWidth+= inflateValue;
>
> Why does this need to change at all? LayoutUnit already has an operator++ that should do the right thing.
Apparently LayoutUnit has broken operator++. It defines the post-increment operator++ only, but the implementation is pre-increment. I'll fix that by adding the pre-increment operator++ and change the post-increment implementation.
LayoutUnit operator++ bug 128056 Created attachment 222903 [details]
Patch
Comment on attachment 222903 [details] Patch EWS will fail as this code depends on bug 128056. I'll test it against EWS soon after the LayoutUnit patch gets landed. Created attachment 222915 [details]
Patch
Comment on attachment 222915 [details]
Patch
EWS testing.
Comment on attachment 222915 [details] Patch Clearing flags on attachment: 222915 Committed r163262: <http://trac.webkit.org/changeset/163262> All reviewed patches have been landed. Closing bug. |