Summary: | text-combine needs to center glyphs within the vertical area. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||||||||
Component: | Layout and Rendering | Assignee: | Dave Hyatt <hyatt> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, esprehn+autocc, glenn, jonlee, kondapallykalyan, mmaxfield, ossy, rniwa, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Dave Hyatt
2014-10-24 13:26:53 PDT
Created attachment 240433 [details]
Patch
Created attachment 240434 [details]
Patch
Comment on attachment 240434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240434&action=review Unofficial r=me > Source/WebCore/rendering/RenderCombineText.cpp:106 > + glyphOverflow.computeBounds = true; Is there a perf regression from this? I'd imagine not because tatechuyoko is pretty rare > LayoutTests/platform/mac/fast/text/tatechuyoko-expected.txt:1 > +layer at (0,0) size 785x668 There's now way we can test this in a platform-independent way? Comment on attachment 240434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240434&action=review > Source/WebCore/rendering/RenderCombineText.cpp:144 > + m_combinedTextSize = FloatSize(combinedTextWidth, glyphOverflow.bottom + glyphOverflow.top); It's probably worth taking glyphOverflow.left and .right into consideration (since at this point they are already computed) (In reply to comment #3) > Comment on attachment 240434 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240434&action=review > > Unofficial r=me > > > Source/WebCore/rendering/RenderCombineText.cpp:106 > > + glyphOverflow.computeBounds = true; > > Is there a perf regression from this? I'd imagine not because tatechuyoko is > pretty rare > Nah, should be fine. iBooks already computes glyph bounds for all text anyway. > > LayoutTests/platform/mac/fast/text/tatechuyoko-expected.txt:1 > > +layer at (0,0) size 785x668 > > There's now way we can test this in a platform-independent way? Not that I can come up with. It's a unique rendering that can't be duplicated easily. I tried with inline-block and different writing-mode but couldn't get it to line up the same way. (In reply to comment #4) > Comment on attachment 240434 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240434&action=review > > > Source/WebCore/rendering/RenderCombineText.cpp:144 > > + m_combinedTextSize = FloatSize(combinedTextWidth, glyphOverflow.bottom + glyphOverflow.top); > > It's probably worth taking glyphOverflow.left and .right into consideration > (since at this point they are already computed) left and right are not relevant. The text is already centered in that direction. Created attachment 240448 [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
Created attachment 240449 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #9) > Landed in r175236. The new test fails on the Apple Mountain Lion and Mavericks bots. Landed pre-Yosemite results in r175264. |