Bug 126008

Summary: Subpixel layout: Missing float rounding at CSSPrimitiveValue::computeLength() can make line-height shorter (nytimes.com).
Product: WebKit Reporter: alan <zalan>
Component: Layout and RenderingAssignee: alan <zalan>
Status: RESOLVED INVALID    
Severity: Normal CC: hyatt, jonlee, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 126283    
Attachments:
Description Flags
test case
none
screenshot
none
FF vs subpixel on vs subpixel off none

alan
Reported 2013-12-19 12:17:11 PST
Created attachment 219673 [details] test case With the following CSS, we end up calculating 16px (subpixel on) vs. 17px (subpixel off) for line-height. <html> <head> <style> div { font-size:62.5%; width:10px; } p { font-size:1.2em; line-height:1.416em; } </style> </head> <body> <div> <p>foo. foo.</p> </div> </body> </html> Here we round the calculated float value of 16.99 to 17 with subpixel off, while we return 16.99 with subpixel on. This float value later gets clamped to int -> 16 vs 17 -> line-height difference. template<> Length CSSPrimitiveValue::computeLength(const RenderStyle* style, const RenderStyle* rootStyle, float multiplier, bool computingFontSize) const { #if ENABLE(SUBPIXEL_LAYOUT) return Length(clampTo<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize), minValueForCssLength, maxValueForCssLength), Fixed); #else return Length(clampTo<float>(roundForImpreciseConversion<float>(computeLengthDouble(style, rootStyle, multiplier, computingFontSize)), minValueForCssLength, maxValueForCssLength), Fixed); #endif } observer on nytimes.com frame #0: 0x000000010affc37f WebCore`WebCore::ApplyPropertyLineHeight::applyValue(=CSSPropertyLineHeight, styleResolver=0x00007f8279f91560, value=0x00007f8279f43110) + 303 at DeprecatedStyleBuilder.cpp:1458 frame #1: 0x000000010c44eeab WebCore`WebCore::PropertyHandler::applyValue(this=0x00007f827a04b9b0, propertyID=CSSPropertyLineHeight, styleResolver=0x00007f8279f91560, value=0x00007f8279f43110) const + 107 at DeprecatedStyleBuilder.h:48 frame #2: 0x000000010c43ec7f WebCore`WebCore::StyleResolver::applyProperty(this=0x00007f8279f91560, id=CSSPropertyLineHeight, value=0x00007f8279f43110) + 1007 at StyleResolver.cpp:2060 frame #3: 0x000000010c43d156 WebCore`WebCore::StyleResolver::applyMatchedProperties(this=0x00007f8279f91560, matchResult=0x00007fff58757bf8, element=0x00007f827bc99010) + 854 at StyleResolver.cpp:1739 frame #4: 0x000000010c43afb8 WebCore`WebCore::StyleResolver::styleForElement(this=0x00007f8279f91560, element=0x00007f827bc99010, defaultParent=0x0000000000000000, sharingBehavior=AllowStyleSharing, matchingBehavior=MatchAllRules, regionForStyling=0x0000000000000000) + 1208 at StyleResolver.cpp:844 frame #5: 0x000000010b1ad7e5 WebCore`WebCore::Element::styleForRenderer(this=0x00007f827bc99010) + 293 at Element.cpp:1480 frame #6: 0x000000010ad11a9f WebCore`WebCore::Style::createRendererIfNeeded(element=0x00007f827bc99010, resolvedStyle=0x00007fff587584b0) + 223 at StyleResolveTree.cpp:219 frame #7: 0x000000010ad1158f WebCore`WebCore::Style::attachRenderTree(current=0x00007f827bc99010, resolvedStyle=0x00007fff587584f0) + 127 at StyleResolveTree.cpp:528 frame #8: 0x000000010ad114e7 WebCore`WebCore::Style::attachRenderTree(element=0x00007f827bc99010) + 55 at StyleResolveTree.cpp:894 frame #9: 0x000000010b46f55a WebCore`WebCore::executeTask(task=0x00007fff58758590) + 458 at HTMLConstructionSite.cpp:99 frame #10: 0x000000010b46f366 WebCore`WebCore::HTMLConstructionSite::executeQueuedTasks(this=0x00007f8279ff2738) + 134 at HTMLConstructionSite.cpp:145 frame #11: 0x000000010b56ff05 WebCore`WebCore::HTMLTreeBuilder::constructTree(this=0x00007f8279ff2720, token=0x00007fff58758630) + 389 at HTMLTreeBuilder.cpp:385 frame #12: 0x000000010b49435e WebCore`WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(this=0x00007f8279fdf940, rawToken=0x00007f827b022c00) + 110 at HTMLDocumentParser.cpp:586 frame #13: 0x000000010b493b3b WebCore`WebCore::HTMLDocumentParser::pumpTokenizer(this=0x00007f8279fdf940, mode=AllowYield) + 1371 at HTMLDocumentParser.cpp:543 frame #14: 0x000000010b4933d9 WebCore`WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(this=0x00007f8279fdf940, mode=AllowYield) + 169 at HTMLDocumentParser.cpp:227 frame #15: 0x000000010b4948e6 WebCore`WebCore::HTMLDocumentParser::append(this=0x00007f8279fdf940, inputSource=0x00007fff587589e8) + 582 at HTMLDocumentParser.cpp:733 frame #16: 0x000000010af8e2c9 WebCore`WebCore::DecodedDataDocumentParser::flush(this=0x00007f8279fdf940, writer=0x00007f827b01b6a0) + 137 at DecodedDataDocumentParser.cpp:60 frame #17: 0x000000010b09a6de WebCore`WebCore::DocumentWriter::end(this=0x00007f827b01b6a0) + 254 at DocumentWriter.cpp:242 frame #18: 0x000000010b066d66 WebCore`WebCore::DocumentLoader::finishedLoading(this=0x00007f827b01b600, finishTime=0) + 598 at DocumentLoader.cpp:408 frame #19: 0x000000010b066a7e WebCore`WebCore::DocumentLoader::notifyFinished(this=0x00007f827b01b600, resource=0x00007f827f519710) + 270 at DocumentLoader.cpp:345 frame #20: 0x000000010ac8858d WebCore`WebCore::CachedResource::checkNotify(this=0x00007f827f519710) + 109 at CachedResource.cpp:369 frame #21: 0x000000010ac886a4 WebCore`WebCore::CachedResource::finishLoading(this=0x00007f827f519710, =0x00007f8279f11c10) + 52 at CachedResource.cpp:385 frame #22: 0x000000010ac82dab WebCore`WebCore::CachedRawResource::finishLoading(this=0x00007f827f519710, data=0x00007f8279f11c10) + 187 at CachedRawResource.cpp:94 frame #23: 0x000000010c483298 WebCore`WebCore::SubresourceLoader::didFinishLoading(this=0x00007f827b09dc00, finishTime=0) + 440 at SubresourceLoader.cpp:279 frame #24: 0x000000010c285815 WebCore`WebCore::ResourceLoader::didFinishLoading(this=0x00007f827b09dc00, =0x00007f8279fc6a60, finishTime=0) + 53 at ResourceLoader.cpp:487
Attachments
test case (206 bytes, text/html)
2013-12-19 12:17 PST, alan 		no flags
Details
screenshot (345.48 KB, image/gif)
2013-12-19 12:21 PST, alan 		no flags
Details
FF vs subpixel on vs subpixel off (501.56 KB, image/jpeg)
2014-01-09 14:58 PST, alan 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
alan
Comment 1 2013-12-19 12:19:28 PST
<rdar://problem/15701643>
alan
Comment 2 2013-12-19 12:21:12 PST
Created attachment 219675 [details] screenshot anim gif to demonstrate the line-height issue.
alan
Comment 3 2014-01-08 15:17:45 PST
check what FF does with such content.
alan
Comment 4 2014-01-09 14:58:22 PST
Created attachment 220772 [details] FF vs subpixel on vs subpixel off FireFox (v26.0) matches the 'subpixel on' rendering.
Note You need to log in before you can comment on or make changes to this bug.