[Libreoffice-commits] core.git: external/skia vcl/skia
Luboš Luňák (via logerrit)
logerrit at kemper.freedesktop.org
Fri Mar 27 14:56:50 UTC 2020
external/skia/UnpackedTarball_skia.mk | 2
external/skia/windows-do-not-modify-logfont.patch.0 | 29 ++++
external/skia/windows-hfont-typeface.patch.0 | 143 --------------------
vcl/skia/win/gdiimpl.cxx | 26 ++-
4 files changed, 46 insertions(+), 154 deletions(-)
New commits:
commit 7b23e0f3ae6b2fb1cebe97c6517eb18f9ed4a76e
Author: Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Fri Mar 27 12:51:19 2020 +0100
Commit: Luboš Luňák <l.lunak at collabora.com>
CommitDate: Fri Mar 27 15:56:10 2020 +0100
fix LOGFONTA/GetObjectW() mismatch and remove Skia HFONT hack (tdf#131426)
The GetObjectA/W() functions are type-unsafe, and since we #undef GetObject
I accidentally used GetObjectW() with LOGFONT that was LOGFONTA. This
caused the font name to be broken, which made Skia use a different font.
This means that Skia doesn't actually need the HFONT passing hack.
Change-Id: I67b8d18fef6a92f8839b1652e051da96d01c3a4e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/91202
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk
index fd05e8d5b3e3..63ab183e3ebf 100644
--- a/external/skia/UnpackedTarball_skia.mk
+++ b/external/skia/UnpackedTarball_skia.mk
@@ -29,7 +29,7 @@ skia_patches := \
clang11-flax-vector-conversion.patch.0 \
clang-attributes-warning.patch.1 \
fontconfig-get-typeface.patch.0 \
- windows-hfont-typeface.patch.0 \
+ windows-do-not-modify-logfont.patch.0 \
windows-text-gamma.patch.0 \
$(eval $(call gb_UnpackedTarball_set_patchlevel,skia,1))
diff --git a/external/skia/windows-do-not-modify-logfont.patch.0 b/external/skia/windows-do-not-modify-logfont.patch.0
new file mode 100644
index 000000000000..30c5c1e96e56
--- /dev/null
+++ b/external/skia/windows-do-not-modify-logfont.patch.0
@@ -0,0 +1,29 @@
+--- ./src/ports/SkFontHost_win.cpp
++++ ./src/ports/SkFontHost_win.cpp
+@@ -349,7 +349,7 @@ static bool FindByLogFont(SkTypeface* face, void* ctx) {
+ */
+ SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF) {
+ LOGFONT lf = origLF;
+- make_canonical(&lf);
++// make_canonical(&lf);
+ sk_sp<SkTypeface> face = SkTypefaceCache::FindByProcAndRef(FindByLogFont, &lf);
+ if (!face) {
+ face = LogFontTypeface::Make(lf);
+@@ -363,7 +363,7 @@ SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF) {
+ */
+ sk_sp<SkTypeface> SkCreateFontMemResourceTypefaceFromLOGFONT(const LOGFONT& origLF, HANDLE fontMemResource) {
+ LOGFONT lf = origLF;
+- make_canonical(&lf);
++// make_canonical(&lf);
+ // We'll never get a cache hit, so no point in putting this in SkTypefaceCache.
+ return FontMemResourceTypeface::Make(lf, fontMemResource);
+ }
+@@ -686,7 +686,7 @@ SkScalerContext_GDI::SkScalerContext_GDI(sk_sp<LogFontTypeface> rawTypeface,
+
+ LOGFONT lf = typeface->fLogFont;
+ lf.lfHeight = -SkScalarTruncToInt(gdiTextSize);
+- lf.lfQuality = compute_quality(fRec);
++// lf.lfQuality = compute_quality(fRec);
+ fFont = CreateFontIndirect(&lf);
+ if (!fFont) {
+ return;
diff --git a/external/skia/windows-hfont-typeface.patch.0 b/external/skia/windows-hfont-typeface.patch.0
deleted file mode 100644
index e4dff5f34a52..000000000000
--- a/external/skia/windows-hfont-typeface.patch.0
+++ /dev/null
@@ -1,143 +0,0 @@
---- ./include/ports/SkTypeface_win.h.sav 2019-09-19 11:38:00.943185300 +0200
-+++ ./include/ports/SkTypeface_win.h 2020-03-16 15:11:38.347067100 +0100
-@@ -11,6 +11,8 @@
- #include "include/core/SkTypeface.h"
- #include "include/core/SkTypes.h"
-
-+#include <windows.h>
-+
- #ifdef SK_BUILD_FOR_WIN
-
- #ifdef UNICODE
-@@ -28,6 +30,8 @@
- */
- SK_API SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT&);
-
-+SK_API SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT&, HFONT);
-+
- /**
- * Copy the LOGFONT associated with this typeface into the lf parameter. Note
- * that the lfHeight will need to be set afterwards, since the typeface does
---- ./src/ports/SkFontHost_win.cpp.sav 2020-03-16 15:22:02.620518100 +0100
-+++ ./src/ports/SkFontHost_win.cpp 2020-03-16 15:27:12.733594400 +0100
-@@ -215,6 +215,11 @@
- , fFont(::CreateFontIndirect(&lf))
- , fSavefont((HFONT)::SelectObject(fHdc, fFont))
- { }
-+ explicit SkAutoHDC(const HFONT hf)
-+ : fHdc(::CreateCompatibleDC(nullptr))
-+ , fFont(nullptr)
-+ , fSavefont((HFONT)::SelectObject(fHdc, hf))
-+ { }
- ~SkAutoHDC() {
- if (fHdc) {
- ::SelectObject(fHdc, fSavefont);
-@@ -238,8 +243,22 @@
- : SkTypeface(style, false)
- , fLogFont(lf)
- , fSerializeAsStream(serializeAsStream)
-+ , hFont(nullptr)
- {
- SkAutoHDC hdc(fLogFont);
-+ init(hdc, style, lf);
-+ }
-+ LogFontTypeface(const SkFontStyle& style, const LOGFONT& lf, HFONT hf, bool serializeAsStream)
-+ : SkTypeface(style, false)
-+ , fLogFont(lf)
-+ , fSerializeAsStream(serializeAsStream)
-+ , hFont(hf)
-+ {
-+ SkAutoHDC hdc(hFont);
-+ init(hdc, style, lf);
-+ }
-+ void init(SkAutoHDC& hdc, const SkFontStyle& style, const LOGFONT& lf)
-+ {
- TEXTMETRIC textMetric;
- if (0 == GetTextMetrics(hdc, &textMetric)) {
- call_ensure_accessible(lf);
-@@ -260,6 +279,7 @@
- }
-
- LOGFONT fLogFont;
-+ HFONT hFont;
- bool fSerializeAsStream;
- bool fCanBeLCD;
-
-@@ -267,6 +287,10 @@
- return sk_sp<LogFontTypeface>(new LogFontTypeface(get_style(lf), lf, false));
- }
-
-+ static sk_sp<LogFontTypeface> Make(const LOGFONT& lf, HFONT hf) {
-+ return sk_sp<LogFontTypeface>(new LogFontTypeface(get_style(lf), lf, hf, false));
-+ }
-+
- static void EnsureAccessible(const SkTypeface* face) {
- call_ensure_accessible(static_cast<const LogFontTypeface*>(face)->fLogFont);
- }
-@@ -348,7 +372,7 @@
- */
- SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF) {
- LOGFONT lf = origLF;
-- make_canonical(&lf);
-+// make_canonical(&lf);
- sk_sp<SkTypeface> face = SkTypefaceCache::FindByProcAndRef(FindByLogFont, &lf);
- if (!face) {
- face = LogFontTypeface::Make(lf);
-@@ -357,12 +381,33 @@
- return face.release();
- }
-
-+static bool FindByLogFontAndHFont(SkTypeface* face, void* ctx) {
-+ LogFontTypeface* lface = static_cast<LogFontTypeface*>(face);
-+ const std::pair<LOGFONT*,HFONT>* data = reinterpret_cast<const std::pair<LOGFONT*,HFONT>*>(ctx);
-+ const LOGFONT* lf = data->first;
-+ const HFONT hf = data->second;
-+
-+ return !memcmp(&lface->fLogFont, lf, sizeof(LOGFONT)) && lface->hFont == hf;
-+}
-+
-+SkTypeface* SkCreateTypefaceFromLOGFONT(const LOGFONT& origLF, HFONT hFont) {
-+ LOGFONT lf = origLF;
-+// make_canonical(&lf);
-+ std::pair<LOGFONT*,HFONT> data = std::make_pair(&lf,hFont);
-+ sk_sp<SkTypeface> face = SkTypefaceCache::FindByProcAndRef(FindByLogFontAndHFont, &data);
-+ if (!face) {
-+ face = LogFontTypeface::Make(lf,hFont);
-+ SkTypefaceCache::Add(face);
-+ }
-+ return face.release();
-+}
-+
- /**
- * The created SkTypeface takes ownership of fontMemResource.
- */
- sk_sp<SkTypeface> SkCreateFontMemResourceTypefaceFromLOGFONT(const LOGFONT& origLF, HANDLE fontMemResource) {
- LOGFONT lf = origLF;
-- make_canonical(&lf);
-+// make_canonical(&lf);
- // We'll never get a cache hit, so no point in putting this in SkTypefaceCache.
- return FontMemResourceTypeface::Make(lf, fontMemResource);
- }
-@@ -686,7 +731,10 @@
- LOGFONT lf = typeface->fLogFont;
- lf.lfHeight = -SkScalarTruncToInt(gdiTextSize);
- lf.lfQuality = compute_quality(fRec);
-- fFont = CreateFontIndirect(&lf);
-+ if(typeface->hFont != nullptr)
-+ fFont = typeface->hFont;
-+ else
-+ fFont = CreateFontIndirect(&lf);
- if (!fFont) {
- return;
- }
-@@ -788,7 +836,9 @@
- ::DeleteDC(fDDC);
- }
- if (fFont) {
-- ::DeleteObject(fFont);
-+ LogFontTypeface* typeface = static_cast<LogFontTypeface*>(this->getTypeface());
-+ if(typeface->hFont != fFont)
-+ ::DeleteObject(fFont);
- }
- if (fSC) {
- ::ScriptFreeCache(&fSC);
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index 221819ca1c4f..e7d0b576e37a 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -118,29 +118,35 @@ bool WinSkiaSalGraphicsImpl::DrawTextLayout(const GenericSalLayout& rLayout)
const WinFontInstance* pWinFont = static_cast<const WinFontInstance*>(&rLayout.GetFont());
const HFONT hLayoutFont = pWinFont->GetHFONT();
LOGFONT logFont;
- if (::GetObjectW(hLayoutFont, sizeof(logFont), &logFont) == 0)
+// Bring back GetObject that got #undef-ed in include/postwin.hxx .
+// The GetObjectA/W() functions are type-unsafe, so they should match the LOGFONTA/W,
+// otherwise the font name will be incorrect and Skia will choose an incorrect font.
+#ifdef UNICODE
+#define GetObject GetObjectW
+#else
+#define GetObject GetObjectA
+#endif
+ if (GetObject(hLayoutFont, sizeof(logFont), &logFont) == 0)
{
assert(false);
return false;
}
- // Wrap the font in Skia's SkTypeFace subclass that's been patched
- // to use it.
- sk_sp<SkTypeface> typeface(SkCreateTypefaceFromLOGFONT(logFont, hLayoutFont));
+ sk_sp<SkTypeface> typeface(SkCreateTypefaceFromLOGFONT(logFont));
// lfHeight actually depends on DPI, so it's not really font height as such,
// but for LOGFONT-based typefaces Skia simply sets lfHeight back to this value
// directly.
- // This is probably not necessary since we pass also the HFONT itself, but better
- // forward that information too, in case SkFont uses it somehow.
double fontHeight = logFont.lfHeight;
if (fontHeight < 0)
fontHeight = -fontHeight;
SkFont font(typeface, fontHeight, fHScale, 0);
// Skia needs to be explicitly told what kind of antialiasing should be used,
// get it from system settings. This does not actually matter for the text
- // rendering itself, since it will use the font passed to Skia in the code above
- // (and that one uses DEFAULT_QUALITY, so Windows will select the appropriate AA setting),
- // but Skia internally chooses the format to which the glyphs will be rendered
- // based on this setting (subpixel AA requires colors, others do not).
+ // rendering itself, since Skia has been patched to simply use the setting
+ // from the LOGFONT, which gets set by VCL's ImplGetLogFontFromFontSelect()
+ // and that one normally uses DEFAULT_QUALITY, so Windows will select
+ // the appropriate AA setting. But Skia internally chooses the format to which
+ // the glyphs will be rendered based on this setting (subpixel AA requires colors,
+ // others do not).
BOOL set;
if (SystemParametersInfo(SPI_GETFONTSMOOTHING, 0, &set, 0) && set)
{
More information about the Libreoffice-commits
mailing list