Bug 215661 - Simple text codepath ignores y-component of glyph advances
Summary: Simple text codepath ignores y-component of glyph advances
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks: 206208
  Show dependency treegraph
 
Reported: 2020-08-19 14:17 PDT by Myles C. Maxfield
Modified: 2021-08-06 18:37 PDT (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 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);
Comment 1 Radar WebKit Bug Importer 2020-08-26 14:18:16 PDT
<rdar://problem/67828756>
Comment 2 Myles C. Maxfield 2021-07-25 21:28:09 PDT
Created attachment 434191 [details]
Patch
Comment 3 Myles C. Maxfield 2021-07-26 21:33:55 PDT
Created attachment 434269 [details]
Patch
Comment 4 Darin Adler 2021-07-27 09:04:13 PDT
WebCore\platform\graphics\win\SimpleFontDataCGWin.cpp(121,16): error C3861: 'makeGyphBufferAdvance': identifier not found
Comment 5 Myles C. Maxfield 2021-07-27 10:14:54 PDT
Created attachment 434295 [details]
Patch
Comment 6 Myles C. Maxfield 2021-07-27 12:53:34 PDT
Comment on attachment 434295 [details]
Patch

We might not want to do this.
Comment 7 Darin Adler 2021-07-27 13:03:19 PDT
(In reply to Myles C. Maxfield from comment #6)
> We might not want to do this.

y?
Comment 8 Myles C. Maxfield 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.
Comment 9 Myles C. Maxfield 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
Comment 10 Myles C. Maxfield 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.