Bug 152293

Summary: [Cocoa] Use faster function to calculate glyph advances
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ahmad.saleem792, ntim, vitor.roriz
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch darin: review+

Myles C. Maxfield
Reported 2015-12-14 21:29:48 PST
[Cocoa] Use faster function to calculate glyph advances
Attachments
Patch (2.93 KB, patch)
2015-12-14 21:30 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (2.93 KB, patch)
2015-12-15 13:36 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (2.85 KB, patch)
2015-12-15 16:39 PST, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (2.76 KB, patch)
2015-12-15 18:50 PST, Myles C. Maxfield 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2015-12-14 21:30:21 PST
Created attachment 267343 [details] Patch
Myles C. Maxfield
Comment 2 2015-12-14 21:45:46 PST
Comment on attachment 267343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267343&action=review > Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:653 > + CTFontGetUnsummedAdvancesForGlyphsAndStyle(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, renderingStyle(platformData()), &glyph, &advance, 1) This should probably be kCGFontRenderingStyleAntialiasing | kCGFontAntialiasingStyleUnfiltered
Myles C. Maxfield
Comment 3 2015-12-15 13:36:24 PST
Created attachment 267390 [details] Patch
Myles C. Maxfield
Comment 4 2015-12-15 16:39:12 PST
Created attachment 267411 [details] Patch
Myles C. Maxfield
Comment 5 2015-12-15 18:50:43 PST
Created attachment 267419 [details] Patch
Myles C. Maxfield
Comment 6 2015-12-15 20:05:22 PST
It looks like the existing CoreText function is fast enough now.
Sam Weinig
Comment 7 2015-12-15 20:54:31 PST
Comment on attachment 267419 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267419&action=review > Source/WebCore/ChangeLog:8 > + Covered by existing tests. This changelog could use information about what API you are switching to, and how it was performance tested (what tests? what was the speed up?)
Darin Adler
Comment 8 2015-12-16 09:21:28 PST
Comment on attachment 267419 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267419&action=review >> Source/WebCore/ChangeLog:8 >> + Covered by existing tests. > > This changelog could use information about what API you are switching to, and how it was performance tested (what tests? what was the speed up?) Could not agree more. > Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:656 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000) Should have a blank line after this line. > Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:663 > + CGSize advance = CGSizeZero; > + bool horizontal = platformData().orientation() == Horizontal; > + CTFontGetUnsummedAdvancesForGlyphsAndStyle(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, renderingStyle(platformData()), &glyph, &advance, 1); > + > + return advance.width + m_syntheticBoldOffset; Slightly peculiar paragraphing. I think this would read better without a blank line. Seems we should have a helper function that turns one orientation into the other without the intermediate step of converting to a boolean. > Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:712 > +#endif Should have a blank line before this line.
Ahmad Saleem
Comment 9 2024-02-25 09:59:31 PST
It seems that similar was implemented by following 179957@main in bug 161119. I think we can mark this as duplicate of other.
Tim Nguyen (:ntim)
Comment 10 2024-02-25 19:19:37 PST
*** This bug has been marked as a duplicate of bug 161119 ***
Note You need to log in before you can comment on or make changes to this bug.