[Cocoa] Improve performance of complex text codepath
Created attachment 288749 [details] Patch
Comment on attachment 288749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288749&action=review > Source/WebCore/ChangeLog:8 > + CoreText exposes a bit on the CTRunStatus which represents if the s/if the/whether the/ > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:791 > + while (m_glyphOrigins.size() + 1 < m_adjustedBaseAdvances.size()) > + m_glyphOrigins.append(CGPointZero); This seems like a really inefficient way to zero-fill an array.
Created attachment 288845 [details] Patch
Comment on attachment 288845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288845&action=review > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:795 > + auto oldSize = m_glyphOrigins.size(); > + m_glyphOrigins.grow(m_adjustedBaseAdvances.size()); > + auto newSize = m_glyphOrigins.size(); > + memset(m_glyphOrigins.data() + oldSize, 0, sizeof(CGPoint) * (newSize - oldSize)); > + m_glyphOrigins[m_glyphOrigins.size() - 1] = origins[i]; You're still running this code for time through the "for (unsigned i = 0; i < glyphCount; i++) {" loop. Can you resized m_glyphOrigins up-front? Also if we used a class that inherited from CGPoint and had a constructor, we wouldn't need to explicitly zero-fill.
(In reply to comment #4) > Comment on attachment 288845 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288845&action=review > > > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:795 > > + auto oldSize = m_glyphOrigins.size(); > > + m_glyphOrigins.grow(m_adjustedBaseAdvances.size()); > > + auto newSize = m_glyphOrigins.size(); > > + memset(m_glyphOrigins.data() + oldSize, 0, sizeof(CGPoint) * (newSize - oldSize)); > > + m_glyphOrigins[m_glyphOrigins.size() - 1] = origins[i]; > > You're still running this code for time through the "for (unsigned i = 0; i > < glyphCount; i++) {" loop. Can you resized m_glyphOrigins up-front? The whole point of this patch is to lazily append to the vector. The vector gets populated piece by piece (this zero-filling doesn't overlap any previous zero-fill). > > Also if we used a class that inherited from CGPoint and had a constructor, > we wouldn't need to explicitly zero-fill. True, but I don't think it's worth the added cognitive load.
Committed r206466: <http://trac.webkit.org/changeset/206466>