Bug 206273 - [Win] Remove obsolete useGDI code path and FontRenderingMode enum
Summary: [Win] Remove obsolete useGDI code path and FontRenderingMode enum
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-14 19:22 PST by Fujii Hironori
Modified: 2023-03-03 03:07 PST (History)
5 users (show)

See Also:


Attachments
minibrowser.exe.plist to enable useGDI (223 bytes, application/octet-stream)
2020-01-15 00:01 PST, Fujii Hironori
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-01-14 19:22:24 PST
[Win] Remove obsolete useGDI code path and FontRenderingMode enum

The code was added by r28867.
It draws texts using ExtTextOut with ETO_GLYPH_INDEX for useGDI code path.
However, r126666 removes drawGDIGlyphs, the most important part of useGDI code path.
Comment 1 Fujii Hironori 2020-01-14 19:39:45 PST
Oh, it is being used for CGContextSetShouldUsePlatformNativeGlyphs.

https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/win/FontCGWin.cpp?rev=238253

> CGContextSetShouldUsePlatformNativeGlyphs(cgContext, usePlatformNativeGlyphs);

Does AppleWin port still use useGDI code path?
Comment 2 Fujii Hironori 2020-01-14 19:51:17 PST
Oops, usePlatformNativeGlyphs is always false.
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/win/FontCGWin.cpp?rev=254556#L177
Comment 3 Fujii Hironori 2020-01-14 19:55:09 PST
Oh, no.  comment 2 was wrong.

isPrinterFont is always false, usePlatformNativeGlyphs is font.platformData().useGDI().
Comment 4 Fujii Hironori 2020-01-15 00:01:37 PST
Created attachment 387758 [details]
minibrowser.exe.plist to enable useGDI

There are two ways to enable useGDI code path for AppleWin MiniBrowser.

1. applying the following patch

diff --git a/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp b/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp
index f81a9be39f3..a062b973aca 100644
--- a/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp
+++ b/Tools/MiniBrowser/win/WebKitLegacyBrowserWindow.cpp
@@ -297,6 +297,7 @@ bool WebKitLegacyBrowserWindow::setToDefaultPreferences()
     m_standardPreferences->setJavaScriptEnabled(TRUE);
     m_prefsPrivate->setAllowUniversalAccessFromFileURLs(FALSE);
     m_prefsPrivate->setAllowFileAccessFromFileURLs(TRUE);
+    m_standardPreferences->setFontSmoothing(FontSmoothingTypeWindows);
 
     m_prefsPrivate->setDeveloperExtrasEnabled(TRUE);
 

2. put this minibrowser.exe.plist to %userprofile%\AppData\Roaming\Apple Computer\Preferences
Comment 5 Fujii Hironori 2020-01-15 00:08:20 PST
I don't know AppleWin is still using useGDI code path, but it is still valid code path to work as expected.
Closed as INVALID.
Comment 6 Per Arne Vollan 2020-01-15 10:04:06 PST
(In reply to Fujii Hironori from comment #1)
> Oh, it is being used for CGContextSetShouldUsePlatformNativeGlyphs.
> 
> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/
> graphics/win/FontCGWin.cpp?rev=238253
> 
> > CGContextSetShouldUsePlatformNativeGlyphs(cgContext, usePlatformNativeGlyphs);
> 
> Does AppleWin port still use useGDI code path?

I don't believe it is still being used.
Comment 7 Fujii Hironori 2020-01-15 14:37:48 PST
Can i remove it ?
Comment 8 Per Arne Vollan 2020-01-15 14:43:09 PST
(In reply to Fujii Hironori from comment #7)
> Can i remove it ?

I think so, but it would be good to confirm with Brent or Alex.
Comment 9 Myles C. Maxfield 2021-12-07 15:33:40 PST
The default is FontRenderingMode::Normal, but it looks like users can override that by setting a preference. It looks like that preference is set by placing a .plist in a specific place (as Fujii found in https://bugs.webkit.org/show_bug.cgi?id=206273#c4). I think this is a tradeoff between the pros of eliminating maintenance burden against the cons of making some (specific) Windows users' text look different.

In this case, because the change is somewhat subtle, and because it's difficult to imagine someone going through the trouble of putting a plist in a specific place on the filesystem on Windows just to get their iTunes text to look slightly different, I think maintenance burden wins, and we can just delete this.
Comment 10 Fujii Hironori 2023-02-27 13:23:53 PST
Pull request: https://github.com/WebKit/WebKit/pull/10756
Comment 11 EWS 2023-03-03 03:06:08 PST
Committed 261133@main (21e5a249467d): <https://commits.webkit.org/261133@main>

Reviewed commits have been landed. Closing PR #10756 and removing active labels.
Comment 12 Radar WebKit Bug Importer 2023-03-03 03:07:16 PST
<rdar://problem/106188944>