Bug 48537

Summary: Fonts with no vertical metrics should synthesize baselines
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, buildbot, eric, mrobinson, rniwa, webkit-ews, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 46123    
Attachments:
Description Flags
Patch
none
Patch sam: review+

Dave Hyatt
Reported 2010-10-28 11:45:59 PDT
Fonts with no vertical metrics should synthesize baselines when they appear on lines that do use fonts with vertical metrics. Basically we want to make sure that English text behaves like vertical-align:middle, i.e., its baseline should be treated as though it cuts through the middle of the ascent+descent.
Attachments
Patch (256.24 KB, patch)
2010-11-17 02:49 PST, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (256.01 KB, patch)
2010-11-17 09:16 PST, Dave Hyatt 		sam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adele Peterson
Comment 1 2010-10-29 15:12:20 PDT
<rdar://problem/8612034>
Dave Hyatt
Comment 2 2010-11-17 02:49:09 PST
Created attachment 74096 [details] Patch
Eric Seidel (no email)
Comment 3 2010-11-17 03:05:06 PST
Attachment 74096 [details] did not build on mac: Build output: http://queues.webkit.org/results/6238014
Early Warning System Bot
Comment 4 2010-11-17 03:06:41 PST
Attachment 74096 [details] did not build on qt: Build output: http://queues.webkit.org/results/6177018
Build Bot
Comment 5 2010-11-17 03:22:58 PST
Attachment 74096 [details] did not build on win: Build output: http://queues.webkit.org/results/6165018
Dave Hyatt
Comment 6 2010-11-17 09:16:29 PST
Created attachment 74124 [details] Patch
Sam Weinig
Comment 7 2010-11-17 11:45:36 PST
Comment on attachment 74124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=74124&action=review > WebCore/rendering/InlineFlowBox.cpp:484 > + if (verticalAlign == SUB) > + verticalPosition += fontSize / 5 + 1; > + else if (verticalAlign == SUPER) > + verticalPosition -= fontSize / 3 + 1; > + else if (verticalAlign == TEXT_TOP) > + verticalPosition += renderer->baselinePosition(baselineType, firstLine, lineDirection) - font.ascent(baselineType); > + else if (verticalAlign == MIDDLE) > + verticalPosition += -static_cast<int>(font.xHeight() / 2) - renderer->lineHeight(firstLine, lineDirection) / 2 + renderer->baselinePosition(baselineType, firstLine, lineDirection); > + else if (verticalAlign == TEXT_BOTTOM) { > + verticalPosition += font.descent(baselineType); > + // lineHeight - baselinePosition is always 0 for replaced elements (except inline blocks), so don't bother wasting time in that case. > + if (!renderer->isReplaced() || renderer->isInlineBlockOrInlineTable()) > + verticalPosition -= (renderer->lineHeight(firstLine, lineDirection) - renderer->baselinePosition(baselineType, firstLine, lineDirection)); > + } else if (verticalAlign == BASELINE_MIDDLE) > + verticalPosition += -renderer->lineHeight(firstLine, lineDirection) / 2 + renderer->baselinePosition(baselineType, firstLine, lineDirection); > + else if (verticalAlign == LENGTH) > + verticalPosition -= renderer->style()->verticalAlignLength().calcValue(renderer->lineHeight(firstLine, lineDirection)); > + } If you wanted, you could change this to a switch statement.
Dave Hyatt
Comment 8 2010-11-17 12:56:20 PST
Fixed in r72235.
WebKit Review Bot
Comment 9 2010-11-17 14:07:55 PST
http://trac.webkit.org/changeset/72235 might have broken SnowLeopard Intel Release (Tests) The following tests are not passing: fast/blockflow/vertical-font-fallback.html fast/repaint/repaint-across-writing-mode-boundary.html
Ryosuke Niwa
Comment 10 2010-11-17 20:00:07 PST
It seems like this change requires rebaselines for the following tests on GTK: fast/css/line-height-determined-by-primary-font.html fast/css/negative-leading.html +mrobinson for GTK rebaseline.
Eric Seidel (no email)
Comment 11 2010-11-17 22:26:07 PST
This broke snow leopard tests, and thus the commit-queue: SUCCESS: Build 20999 (r72235) was the first to show failures: set([u'fast/blockflow/vertical-font-fallback.html', u'fast/repaint/repaint-across-writing-mode-boundary.html'])
Note You need to log in before you can comment on or make changes to this bug.