AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Only take DWrite mutex on older versions

Browse Source 
If IDWriteFontFace4 is present on the created typeface, don't take any
extra locks. This should imply a new enough dwrite.dll to avoid thread
safety issues in the DirectWrite caches reported with Windows 8 and 8.1.

Bug: chromium:615880
See-also: https://codereview.chromium.org/1421433004
See-also: https://codereview.chromium.org/1424093002
Change-Id: I4d7adfdbd2e7ef5fd22dc7b10c9bf25eea48651e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/418416
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This commit is contained in:
Ben Wagner 2021-06-14 17:41:54 -04:00 committed by Skia Commit-Bot
parent cc2d073020
commit 597da9e5e2
1 changed files with 72 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
src/ports/SkScalerContext_win_dw.cpp
View File

@ -41,25 +41,63 @@
#include <dwrite_1.h>
#include <dwrite_3.h>
namespace {
/* Note:
* In versions 8 and 8.1 of Windows, some calls in DWrite are not thread safe.
* The mutex returned from get_dwrite_factory_mutex() protects the calls that are
* The mutex returned from maybe_dw_mutex protects the calls that are
* problematic.
*/
static SkSharedMutex& get_dwrite_factory_mutex() {
static SkSharedMutex* maybe_dw_mutex(DWriteFontTypeface& typeface) {
static SkSharedMutex mutex;
return mutex;
return typeface.fDWriteFontFace4 ? nullptr : &mutex;
}
typedef SkAutoSharedMutexExclusive Exclusive;
typedef SkAutoSharedMutexShared Shared;
class SK_SCOPED_CAPABILITY Exclusive {
public:
explicit Exclusive(SkSharedMutex* maybe_lock) SK_ACQUIRE(*maybe_lock)
: fLock(maybe_lock) {
if (fLock) {
fLock->acquire();
}
}
~Exclusive() SK_RELEASE_CAPABILITY() {
if (fLock) {
fLock->release();
}
}
private:
SkSharedMutex* fLock;
};
class SK_SCOPED_CAPABILITY Shared {
public:
explicit Shared(SkSharedMutex* maybe_lock) SK_ACQUIRE_SHARED(*maybe_lock)
: fLock(maybe_lock) {
if (fLock) {
fLock->acquireShared();
}
}
// You would think this should be SK_RELEASE_SHARED_CAPABILITY, but SK_SCOPED_CAPABILITY
// doesn't fully understand the difference between shared and exclusive.
// Please review https://reviews.llvm.org/D52578 for more information.
~Shared() SK_RELEASE_CAPABILITY() {
if (fLock) {
fLock->releaseShared();
}
}
private:
SkSharedMutex* fLock;
};
static bool isLCD(const SkScalerContextRec& rec) {
return SkMask::kLCD16_Format == rec.fMaskFormat;
}
static bool is_hinted(DWriteFontTypeface* typeface) {
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*typeface));
AutoTDWriteTable<SkOTTableMaximumProfile> maxp(typeface->fDWriteFontFace.get());
if (!maxp.fExists) {
return false;
@ -130,7 +168,7 @@ static bool is_gridfit_only(GaspRange::Behavior flags) {
}
static bool has_bitmap_strike(DWriteFontTypeface* typeface, GaspRange range) {
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*typeface));
{
AutoTDWriteTable<SkOTTableEmbeddedBitmapLocation> eblc(typeface->fDWriteFontFace.get());
if (!eblc.fExists) {
@ -215,6 +253,8 @@ static bool is_axis_aligned(const SkScalerContextRec& rec) {
both_zero(rec.fPost2x2[0][0], rec.fPost2x2[1][1]));
}
} //namespace
SkScalerContext_DW::SkScalerContext_DW(sk_sp<DWriteFontTypeface> typefaceRef,
const SkScalerContextEffects& effects,
const SkDescriptor* desc)
@ -386,8 +426,8 @@ SkScalerContext_DW::~SkScalerContext_DW() {
bool SkScalerContext_DW::generateAdvance(SkGlyph* glyph) {
glyph->fAdvanceX = 0;
glyph->fAdvanceY = 0;
uint16_t glyphId = glyph->getGlyphID();
DWriteFontTypeface* typeface = this->getDWriteTypeface();
// DirectWrite treats all out of bounds glyph ids as having the same data as glyph 0.
// For consistency with all other backends, treat out of range glyph ids as an error.
@ -400,8 +440,8 @@ bool SkScalerContext_DW::generateAdvance(SkGlyph* glyph) {
if (DWRITE_MEASURING_MODE_GDI_CLASSIC == fMeasuringMode ||
DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode)
{
Exclusive l(get_dwrite_factory_mutex());
HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetGdiCompatibleGlyphMetrics(
Exclusive l(maybe_dw_mutex(*typeface));
HRBM(typeface->fDWriteFontFace->GetGdiCompatibleGlyphMetrics(
fTextSizeMeasure,
1.0f, // pixelsPerDip
// This parameter does not act like the lpmat2 parameter to GetGlyphOutlineW.
@ -412,15 +452,15 @@ bool SkScalerContext_DW::generateAdvance(SkGlyph* glyph) {
&gm),
"Could not get gdi compatible glyph metrics.");
} else {
Exclusive l(get_dwrite_factory_mutex());
HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetDesignGlyphMetrics(&glyphId, 1, &gm),
Exclusive l(maybe_dw_mutex(*typeface));
HRBM(typeface->fDWriteFontFace->GetDesignGlyphMetrics(&glyphId, 1, &gm),
"Could not get design metrics.");
}
DWRITE_FONT_METRICS dwfm;
{
Shared l(get_dwrite_factory_mutex());
this->getDWriteTypeface()->fDWriteFontFace->GetMetrics(&dwfm);
Shared l(maybe_dw_mutex(*typeface));
typeface->fDWriteFontFace->GetMetrics(&dwfm);
}
SkScalar advanceX = fTextSizeMeasure * gm.advanceWidth / dwfm.designUnitsPerEm;
@ -444,6 +484,8 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
DWRITE_TEXTURE_TYPE textureType,
RECT* bbox)
{
DWriteFontTypeface* typeface = this->getDWriteTypeface();
//Measure raster size.
fXform.dx = SkFixedToFloat(glyph->getSubXFixed());
fXform.dy = SkFixedToFloat(glyph->getSubYFixed());
@ -459,7 +501,7 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
DWRITE_GLYPH_RUN run;
run.glyphCount = 1;
run.glyphAdvances = &advance;
run.fontFace = this->getDWriteTypeface()->fDWriteFontFace.get();
run.fontFace = typeface->fDWriteFontFace.get();
run.fontEmSize = SkScalarToFloat(fTextSizeRender);
run.bidiLevel = 0;
run.glyphIndices = &glyphId;
@ -468,13 +510,13 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
{
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*typeface));
// IDWriteFactory2::CreateGlyphRunAnalysis is very bad at aliased glyphs.
if (this->getDWriteTypeface()->fFactory2 &&
if (typeface->fFactory2 &&
(fGridFitMode == DWRITE_GRID_FIT_MODE_DISABLED ||
fAntiAliasMode == DWRITE_TEXT_ANTIALIAS_MODE_GRAYSCALE))
{
HRM(this->getDWriteTypeface()->fFactory2->CreateGlyphRunAnalysis(
HRM(typeface->fFactory2->CreateGlyphRunAnalysis(
&run,
&fXform,
renderingMode,
@ -486,7 +528,7 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
&glyphRunAnalysis),
"Could not create DW2 glyph run analysis.");
} else {
HRM(this->getDWriteTypeface()->fFactory->CreateGlyphRunAnalysis(&run,
HRM(typeface->fFactory->CreateGlyphRunAnalysis(&run,
1.0f, // pixelsPerDip,
&fXform,
renderingMode,
@ -498,7 +540,7 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
}
}
{
Shared l(get_dwrite_factory_mutex());
Shared l(maybe_dw_mutex(*typeface));
HRM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox),
"Could not get texture bounds.");
}
@ -553,7 +595,9 @@ bool SkScalerContext_DW::getColorGlyphRun(const SkGlyph& glyph,
void SkScalerContext_DW::generateColorMetrics(SkGlyph* glyph) {
SkTScopedComPtr<IDWriteColorGlyphRunEnumerator> colorLayers;
HRVM(getColorGlyphRun(*glyph, &colorLayers), "Could not get color glyph run");
if (!getColorGlyphRun(*glyph, &colorLayers)) {
return;
}
SkASSERT(colorLayers.get());
SkRect bounds = SkRect::MakeEmpty();
@ -567,7 +611,7 @@ void SkScalerContext_DW::generateColorMetrics(SkGlyph* glyph) {
HRVM(SkDWriteGeometrySink::Create(&path, &geometryToPath),
"Could not create geometry to path converter.");
{
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*this->getDWriteTypeface()));
HRVM(colorGlyph->glyphRun.fontFace->GetGlyphRunOutline(
colorGlyph->glyphRun.fontEmSize,
colorGlyph->glyphRun.glyphIndices,
@ -923,6 +967,8 @@ const void* SkScalerContext_DW::drawDWMask(const SkGlyph& glyph,
DWRITE_RENDERING_MODE renderingMode,
DWRITE_TEXTURE_TYPE textureType)
{
DWriteFontTypeface* typeface = this->getDWriteTypeface();
int sizeNeeded = glyph.width() * glyph.height();
if (DWRITE_TEXTURE_CLEARTYPE_3x1 == textureType) {
sizeNeeded *= 3;
@ -957,7 +1003,7 @@ const void* SkScalerContext_DW::drawDWMask(const SkGlyph& glyph,
{
SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
{
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*typeface));
// IDWriteFactory2::CreateGlyphRunAnalysis is very bad at aliased glyphs.
if (this->getDWriteTypeface()->fFactory2 &&
(fGridFitMode == DWRITE_GRID_FIT_MODE_DISABLED ||
@ -993,7 +1039,7 @@ const void* SkScalerContext_DW::drawDWMask(const SkGlyph& glyph,
bbox.right = glyph.left() + glyph.width();
bbox.bottom = glyph.top() + glyph.height();
{
Shared l(get_dwrite_factory_mutex());
Shared l(maybe_dw_mutex(*typeface));
HRNM(glyphRunAnalysis->CreateAlphaTexture(textureType,
&bbox,
fBits.begin(),
@ -1058,7 +1104,7 @@ void SkScalerContext_DW::generateColorGlyphImage(const SkGlyph& glyph) {
HRVM(SkDWriteGeometrySink::Create(&path, &geometryToPath),
"Could not create geometry to path converter.");
{
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*this->getDWriteTypeface()));
HRVM(colorGlyph->glyphRun.fontFace->GetGlyphRunOutline(
colorGlyph->glyphRun.fontEmSize,
colorGlyph->glyphRun.glyphIndices,
@ -1198,7 +1244,7 @@ bool SkScalerContext_DW::generatePath(SkGlyphID glyph, SkPath* path) {
"Could not create geometry to path converter.");
UINT16 glyphId = SkTo<UINT16>(glyph);
{
Exclusive l(get_dwrite_factory_mutex());
Exclusive l(maybe_dw_mutex(*this->getDWriteTypeface()));
//TODO: convert to<->from DIUs? This would make a difference if hinting.
//It may not be needed, it appears that DirectWrite only hints at em size.
HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetGlyphRunOutline(