Bug 228536 - [Win][Uniscribe] Remove the code rounding off glyph advances and offsets for non system fonts
Summary: [Win][Uniscribe] Remove the code rounding off glyph advances and offsets for ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 228363
  Show dependency treegraph
 
Reported: 2021-07-27 23:25 PDT by Fujii Hironori
Modified: 2021-08-03 23:26 PDT (History)
2 users (show)

See Also:


Attachments
WIP patch (3.24 KB, patch)
2021-07-27 23:52 PDT, Fujii Hironori
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP patch (4.78 KB, patch)
2021-07-28 22:48 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2021-07-27 23:25:44 PDT
[Win][Uniscribe] Remove the code rounding off glyph advances and offsets for system fonts

ComplexTextController::collectComplexTextRunsForCharacters has the following code.

> // Match AppKit's rules for the integer vs. non-integer rendering modes.
> if (!font->platformData().isSystemFont()) {
>     advance = roundf(advance);
>     offsetX = roundf(offsetX);
>     offsetY = roundf(offsetY);
> }

This was added by r18359.

See also: Bug#228363 Comment#2
Comment 1 Fujii Hironori 2021-07-27 23:31:31 PDT
(In reply to Fujii Hironori from comment #0)
> This was added by r18359.

No. This code was added by r23462.
Comment 2 Fujii Hironori 2021-07-27 23:36:41 PDT
(In reply to Fujii Hironori from comment #1)
> No. This code was added by r23462.

More precisely r23199 and r23154.
Comment 3 Fujii Hironori 2021-07-27 23:52:45 PDT
Created attachment 434405 [details]
WIP patch
Comment 4 Fujii Hironori 2021-07-28 14:55:05 PDT
In WinCairo, this patch makes the following ref tests fail.

  fast/text/emoji-gender-3.html [ ImageOnlyFailure ]
  fast/text/emoji-gender-4.html [ ImageOnlyFailure ]
  fast/text/emoji-gender-5.html [ ImageOnlyFailure ]
  fast/text/emoji-gender-6.html [ ImageOnlyFailure ]
  fast/text/emoji-gender-8.html [ ImageOnlyFailure ]

This is caused by the difference of how to get glyph advance between simple text code path and complex text code path.
The simple text code path is using cairo_scaled_font_glyph_extents, the complex text code path is using Uniscribe to get the glyph advance.

I think the simple text code path also should use Uniscribe.

However, the argument of Font::platformWidthForGlyph is Glyph not UChar.
Uniscribe (ScriptShape) takes a string, not glyphs.

Another approach is implementing Font::applyTransforms with Uniscribe (Bug 228363).
However, Font::applyTransforms is not applied to a single character.
This approach needs some modification to WidthIterator.
Comment 5 Fujii Hironori 2021-07-28 17:16:28 PDT
I confirmed WinCairo doesn't have the comment#4 problem if it always uses the complex text code path.
In the complex text code path, it consistently uses Uniscribe.
Comment 6 Fujii Hironori 2021-07-28 22:48:52 PDT
Created attachment 434495 [details]
WIP patch

The WinCairo problem is fixed by reimplementing Font::platformWidthForGlyph with GetCharABCWidthsI.
Comment 7 Radar WebKit Bug Importer 2021-08-03 23:26:17 PDT
<rdar://problem/81500144>