Bug 215661

Summary:

Simple text codepath ignores y-component of glyph advances

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

Text

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

changseok, darin, dbarton, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, mmaxfield, pdr, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

206208

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-

Myles C. Maxfield

Reported 2020-08-19 14:17:03 PDT

It has: float Font::platformWidthForGlyph(Glyph glyph) const { CGSize advance = CGSizeZero; ... populate advance ... return advance.width + [unrelated stuff]; } and float width = font->widthForGlyph(glyph);

Attachments
Patch (22.00 KB, patch) 2021-07-25 21:28 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (26.57 KB, patch) 2021-07-26 21:33 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (26.76 KB, patch) 2021-07-27 10:14 PDT, Myles C. Maxfield	ews-feeder: commit-queue-	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2020-08-26 14:18:16 PDT

<rdar://problem/67828756>

Myles C. Maxfield

Comment 2 2021-07-25 21:28:09 PDT

Created attachment 434191 [details] Patch

Myles C. Maxfield

Comment 3 2021-07-26 21:33:55 PDT

Created attachment 434269 [details] Patch

Darin Adler

Comment 4 2021-07-27 09:04:13 PDT

WebCore\platform\graphics\win\SimpleFontDataCGWin.cpp(121,16): error C3861: 'makeGyphBufferAdvance': identifier not found

Myles C. Maxfield

Comment 5 2021-07-27 10:14:54 PDT

Created attachment 434295 [details] Patch

Myles C. Maxfield

Comment 6 2021-07-27 12:53:34 PDT

Comment on attachment 434295 [details] Patch We might not want to do this.

Darin Adler

Comment 7 2021-07-27 13:03:19 PDT

(In reply to Myles C. Maxfield from comment #6) > We might not want to do this. y?

Myles C. Maxfield

Comment 8 2021-07-27 13:21:16 PDT

(In reply to Darin Adler from comment #7) > (In reply to Myles C. Maxfield from comment #6) > > We might not want to do this. > > y? Because this data comes from the hmtx and vmtx tables in the font, but the data contained inside those tables is one-dimensional. This means that the y-components of these advances is always 0 (or almost always 0, if CoreText has some corner case somewhere which might set something in some circumstance to nonzero). These advances are retained inside caches, and it would be kind of unfortunate to double the size of our caches just so we can store a bunch of 0 values. So, I need to determine if CTFontGetAdvancesForCharacters() will ever return a nonzero value for the Y-components. If it does, I should rework this patch to only store the nonzero values in our caches (maybe by using a unique_ptr which is usually null or something). If it doesn't, then we should just straight up not do this patch.

Myles C. Maxfield

Comment 9 2021-07-29 01:11:10 PDT

There’s a chance that bitmap fonts might actually be capable of having nonzero y advances. I’m making a test font to see. https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6bdat.html

Myles C. Maxfield

Comment 10 2021-08-06 18:36:43 PDT

I believe I've proved that CoreText will always report 0 for one of the two fields when you ask it for advances. So, it looks like this bug is unnecessary.

Note You need to log in before you can comment on or make changes to this bug.