Bug 126008

Summary:

Subpixel layout: Missing float rounding at CSSPrimitiveValue::computeLength() can make line-height shorter (nytimes.com).

Product:

WebKit

Reporter:

zalan <zalan>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

hyatt, jonlee, simon.fraser, webkit-bug-importer

Priority:

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

Description zalan 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

Comment 1 zalan 2013-12-19 12:19:28 PST

<rdar://problem/15701643>

Comment 2 zalan 2013-12-19 12:21:12 PST

Created attachment 219675 [details]
screenshot

anim gif to demonstrate the line-height issue.

Comment 3 zalan 2014-01-08 15:17:45 PST

check what FF does with such content.

Comment 4 zalan 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.