<rdar://problem/12470680>
Created attachment 168320 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes)
Comment on attachment 168320 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168320 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14256698
Created attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes)
Comment on attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168322 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14252745
Comment on attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168322 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14258756
Comment on attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168322 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14256710
Comment on attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168322 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14249969
Comment on attachment 168322 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168322 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14249988
Created attachment 168438 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes)
Comment on attachment 168438 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) Attachment 168438 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14265369
Created attachment 168442 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes)
Created attachment 168465 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes)
Comment on attachment 168465 [details] Add kerning and ligature support on the fast code path (this patch excludes WebKitLibraries changes) View in context: https://bugs.webkit.org/attachment.cgi?id=168465&action=review > Source/WebCore/platform/graphics/WidthIterator.cpp:104 > +static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const SimpleFontData* fontData, TypesettingFeatures typesettingFeatures, CharactersTreatedAsSpace& charactersTreatedAsSpace) Do we not have/use an enum for writing direction? > Source/WebCore/platform/graphics/WidthIterator.cpp:130 > + if (!ltr) { > + for (int i = 0, end = glyphBuffer->size() - 1; i < glyphBuffer->size() / 2; ++i, --end) > + glyphBuffer->swap(i, end); > + } DRY? There's so little code here though, I guess it's ok.
(In reply to comment #13) > (From update of attachment 168465 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168465&action=review > > > Source/WebCore/platform/graphics/WidthIterator.cpp:104 > > +static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const SimpleFontData* fontData, TypesettingFeatures typesettingFeatures, CharactersTreatedAsSpace& charactersTreatedAsSpace) > > Do we not have/use an enum for writing direction? There is one, but WidthIterator is not using it yet. > > > Source/WebCore/platform/graphics/WidthIterator.cpp:130 > > + if (!ltr) { > > + for (int i = 0, end = glyphBuffer->size() - 1; i < glyphBuffer->size() / 2; ++i, --end) > > + glyphBuffer->swap(i, end); > > + } > > DRY? There's so little code here though, I guess it's ok. I will follow up with a patch moving this into GlyphBuffer.h, as it’s done in two more places. Thanks for the review!
Fixed in <http://trac.webkit.org/r131365>.
This has broken the chromium mac build. It should be pretty easy to fix. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder/builds/33006/steps/compile/logs/stdio The skia code needs to use width() as a function now. Height is not exposed on the advance struct anymore however. I could add it but that seems to be jumping the gun. Can you please take a look and revert if this isn't easy to fix? Thanks.
nvm! http://trac.webkit.org/changeset/131374 thanks!
The following tests have started asserting on debug bots after this change: Regressions: Unexpected crashes : (22) fast/css/first-letter-capitalized-edit-select-crash.html [ Crash ] fast/css/text-rendering.html [ Crash ] fast/ruby/modify-positioned-ruby-text-crash.html [ Crash ] fast/text/complex-preferred-logical-widths.html [ Crash ] fast/text/emphasis.html [ Crash ] fast/text/font-kerning.html [ Crash ] fast/text/font-variant-ligatures.html [ Crash ] fast/text/international/text-spliced-font.html [ Crash ] fast/text/international/vertical-text-glyph-test.html [ Crash ] fast/text/international/vertical-text-metrics-test.html [ Crash ] fast/text/ipa-tone-letters.html [ Crash ] fast/text/justification-padding-mid-word.html [ Crash ] fast/text/justify-ideograph-complex.html [ Crash ] fast/text/justify-ideograph-leading-expansion.html [ Crash ] fast/text/offsetForPosition-complex-fallback.html [ Crash ] fast/text/regional-indicator-symobls.html [ Crash ] fast/text/word-space-with-kerning-2.html [ Crash ] fast/text/word-space-with-kerning.html [ Crash ] fast/writing-mode/text-orientation-basic.html [ Crash ] platform/chromium-linux/svg/text/text-with-geometric-precision.svg [ Crash ] svg/W3C-SVG-1.1/filters-conv-01-f.svg [ Crash ] svg/text/scaling-font-with-geometric-precision.html [ Crash ] The stacktrace for fast/css/first-letter-capitalized-edit-select-crash.html: crash log for DumpRenderTree (pid 25383): STDOUT: <empty> STDERR: SHOULD NEVER BE REACHED STDERR: third_party/WebKit/Source/WebCore/platform/graphics/SimpleFontData.h(206) : bool WebCore::SimpleFontData::applyTransforms(WebCore::GlyphBufferGlyph*, WebCore::GlyphBufferAdvance*, size_t, WebCore::TypesettingFeatures) const STDERR: [25383:25383:17802909680655:ERROR:process_util_posix.cc(144)] Received signal 11 STDERR: base::debug::StackTrace::StackTrace() [0x7fccefb28fda] STDERR: base::(anonymous namespace)::StackDumpSignalHandler() [0x7fccefb93579] STDERR: 0x7fcce8ac1af0 STDERR: WebCore::SimpleFontData::applyTransforms() [0x7fccf2de897e] STDERR: WebCore::applyFontTransforms() [0x7fccf2de8474] STDERR: WebCore::WidthIterator::advanceInternal<>() [0x7fccf2deae51] STDERR: WebCore::WidthIterator::advance() [0x7fccf2de8749] STDERR: WebCore::Font::floatWidthForSimpleText() [0x7fccf2da6bfa] STDERR: WebCore::Font::width() [0x7fccf2d8ff28] STDERR: WebCore::RenderText::computePreferredLogicalWidths() [0x7fccf38d6f66] STDERR: WebCore::RenderText::width() [0x7fccf38d89bf] STDERR: WebCore::textWidth() [0x7fccf37878dd] STDERR: WebCore::RenderBlock::LineBreaker::nextLineBreak() [0x7fccf378a9fe] STDERR: WebCore::RenderBlock::layoutRunsAndFloatsInRange() [0x7fccf3783265] STDERR: WebCore::RenderBlock::layoutRunsAndFloats() [0x7fccf3782b00] STDERR: WebCore::RenderBlock::layoutInlineChildren() [0x7fccf378517a] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fccf37351fd] STDERR: WebCore::RenderBlock::layout() [0x7fccf3734776] STDERR: WebCore::RenderListItem::layout() [0x7fccf38588b8] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fccf373a44a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fccf3739fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fccf373521e] STDERR: WebCore::RenderBlock::layout() [0x7fccf3734776] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fccf373a44a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fccf3739fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fccf373521e] STDERR: WebCore::RenderBlock::layout() [0x7fccf3734776] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fccf373a44a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fccf3739fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fccf373521e] STDERR: WebCore::RenderBlock::layout() [0x7fccf3734776] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fccf373a44a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fccf3739fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fccf373521e] STDERR: WebCore::RenderBlock::layout() [0x7fccf3734776] STDERR: WebCore::RenderView::layoutContent() [0x7fccf38f2222] STDERR: WebCore::RenderView::layout() [0x7fccf38f27d6] For fast/text/emphasis.html crash log for DumpRenderTree (pid 27545): STDOUT: <empty> STDERR: SHOULD NEVER BE REACHED STDERR: third_party/WebKit/Source/WebCore/platform/graphics/SimpleFontData.h(206) : bool WebCore::SimpleFontData::applyTransforms(WebCore::GlyphBufferGlyph*, WebCore::GlyphBufferAdvance*, size_t, WebCore::TypesettingFeatures) const STDERR: [27545:27545:17803018430928:ERROR:process_util_posix.cc(144)] Received signal 11 STDERR: base::debug::StackTrace::StackTrace() [0x7fe5dd006fda] STDERR: base::(anonymous namespace)::StackDumpSignalHandler() [0x7fe5dd071579] STDERR: 0x7fe5d5f9faf0 STDERR: WebCore::SimpleFontData::applyTransforms() [0x7fe5e02c697e] STDERR: WebCore::applyFontTransforms() [0x7fe5e02c6474] STDERR: WebCore::WidthIterator::advanceInternal<>() [0x7fe5e02c8e51] STDERR: WebCore::WidthIterator::advance() [0x7fe5e02c6749] STDERR: WebCore::Font::floatWidthForSimpleText() [0x7fe5e0284bfa] STDERR: WebCore::Font::width() [0x7fe5e026df28] STDERR: WebCore::RenderBlock::LineBreaker::nextLineBreak() [0x7fe5e0c67804] STDERR: WebCore::RenderBlock::layoutRunsAndFloatsInRange() [0x7fe5e0c61265] STDERR: WebCore::RenderBlock::layoutRunsAndFloats() [0x7fe5e0c60b00] STDERR: WebCore::RenderBlock::layoutInlineChildren() [0x7fe5e0c6317a] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fe5e0c131fd] STDERR: WebCore::RenderBlock::layout() [0x7fe5e0c12776] STDERR: WebCore::RenderObject::layoutIfNeeded() [0x7fe5e0be372b] STDERR: WebCore::RenderBlock::insertFloatingObject() [0x7fe5e0c214f8] STDERR: WebCore::RenderBlock::LineBreaker::skipLeadingWhitespace() [0x7fe5e0c655da] STDERR: WebCore::RenderBlock::LineBreaker::nextLineBreak() [0x7fe5e0c664de] STDERR: WebCore::RenderBlock::layoutRunsAndFloatsInRange() [0x7fe5e0c61265] STDERR: WebCore::RenderBlock::layoutRunsAndFloats() [0x7fe5e0c60b00] STDERR: WebCore::RenderBlock::layoutInlineChildren() [0x7fe5e0c6317a] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fe5e0c131fd] STDERR: WebCore::RenderBlock::layout() [0x7fe5e0c12776] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fe5e0c1844a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fe5e0c17fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fe5e0c1321e] STDERR: WebCore::RenderBlock::layout() [0x7fe5e0c12776] STDERR: WebCore::RenderBlock::layoutBlockChild() [0x7fe5e0c1844a] STDERR: WebCore::RenderBlock::layoutBlockChildren() [0x7fe5e0c17fc1] STDERR: WebCore::RenderBlock::layoutBlock() [0x7fe5e0c1321e] STDERR: WebCore::RenderBlock::layout() [0x7fe5e0c12776] Here's a subset: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Ftext%2Fjustify-ideograph-leading-expansion.html%2Cfast%2Ftext%2Fword-space-with-kerning-2.html%2Cfast%2Ftext%2Fcomplex-preferred-logical-widths.html%2Cfast%2Ftext%2Ffont-variant-ligatures.html%2Cfast%2Ftext%2Fjustification-padding-mid-word.html%2Cfast%2Ftext%2Fregional-indicator-symobls.html%2Cfast%2Ftext%2Fword-space-with-kerning.html%2Cfast%2Ftext%2Fjustify-ideograph-complex.html%2Cfast%2Ftext%2Fipa-tone-letters.html%2Cfast%2Ftext%2Femphasis.html%2Cfast%2Ftext%2Ffont-kerning.html%2Cfast%2Ftext%2FoffsetForPosition-complex-fallback.html
(In reply to comment #18) > The following tests have started asserting on debug bots after this change: I tried to fix that in <http://trac.webkit.org/r131375>.
(In reply to comment #19) > (In reply to comment #18) > > The following tests have started asserting on debug bots after this change: > > I tried to fix that in <http://trac.webkit.org/r131375>. This made things worse. I’ll fix that.
(In reply to comment #20) > (In reply to comment #19) > > (In reply to comment #18) > > > The following tests have started asserting on debug bots after this change: > > > > I tried to fix that in <http://trac.webkit.org/r131375>. > > This made things worse. I’ll fix that. <http://trac.webkit.org/r131377>.