From 683a37692bc67ef1144922b73a1e871e7e1e842e Mon Sep 17 00:00:00 2001 From: bungeman Date: Thu, 28 Aug 2014 11:42:29 -0700 Subject: [PATCH] Fix error handling in DirectWrite with tiny text. If there is an error while trying to determine the metrics, we need to bail instead of potentially using uninitialized data. R=reed@google.com, mtklein@google.com Author: bungeman@google.com Review URL: https://codereview.chromium.org/511783003 --- src/ports/SkScalerContext_win_dw.cpp | 100 ++++++++++++++++----------- src/ports/SkScalerContext_win_dw.h | 8 +-- 2 files changed, 64 insertions(+), 44 deletions(-) diff --git a/src/ports/SkScalerContext_win_dw.cpp b/src/ports/SkScalerContext_win_dw.cpp index 216c7dce2c..33ef0d55ec 100644 --- a/src/ports/SkScalerContext_win_dw.cpp +++ b/src/ports/SkScalerContext_win_dw.cpp @@ -398,10 +398,10 @@ void SkScalerContext_DW::generateAdvance(SkGlyph* glyph) { glyph->fAdvanceY = SkScalarToFixed(vecs[0].fY); } -void SkScalerContext_DW::getBoundingBox(SkGlyph* glyph, - DWRITE_RENDERING_MODE renderingMode, - DWRITE_TEXTURE_TYPE textureType, - RECT* bbox) +HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph, + DWRITE_RENDERING_MODE renderingMode, + DWRITE_TEXTURE_TYPE textureType, + RECT* bbox) { //Measure raster size. fXform.dx = SkFixedToFloat(glyph->getSubXFixed()); @@ -426,50 +426,70 @@ void SkScalerContext_DW::getBoundingBox(SkGlyph* glyph, run.glyphOffsets = &offset; SkTScopedComPtr glyphRunAnalysis; - HRVM(fTypeface->fFactory->CreateGlyphRunAnalysis( - &run, - 1.0f, // pixelsPerDip, - &fXform, - renderingMode, - fMeasuringMode, - 0.0f, // baselineOriginX, - 0.0f, // baselineOriginY, - &glyphRunAnalysis), - "Could not create glyph run analysis."); + HRM(fTypeface->fFactory->CreateGlyphRunAnalysis( + &run, + 1.0f, // pixelsPerDip, + &fXform, + renderingMode, + fMeasuringMode, + 0.0f, // baselineOriginX, + 0.0f, // baselineOriginY, + &glyphRunAnalysis), + "Could not create glyph run analysis."); - HRVM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox), - "Could not get texture bounds."); + HRM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox), + "Could not get texture bounds."); + + return S_OK; } -void SkScalerContext_DW::generateMetrics(SkGlyph* glyph) { - glyph->fWidth = 0; - - this->generateAdvance(glyph); - - RECT bbox; - this->getBoundingBox(glyph, fRenderingMode, fTextureType, &bbox); - - // GetAlphaTextureBounds succeeds but returns an empty RECT if there are no - // glyphs of the specified texture type. When this happens, try with the - // alternate texture type. - if (bbox.left == bbox.right || bbox.top == bbox.bottom) { - if (DWRITE_TEXTURE_CLEARTYPE_3x1 == fTextureType) { - this->getBoundingBox(glyph, - DWRITE_RENDERING_MODE_ALIASED, - DWRITE_TEXTURE_ALIASED_1x1, - &bbox); - if (bbox.left != bbox.right && bbox.top != bbox.bottom) { - glyph->fForceBW = 1; - } - } - // TODO: handle the case where a request for DWRITE_TEXTURE_ALIASED_1x1 - // fails, and try DWRITE_TEXTURE_CLEARTYPE_3x1. +/** GetAlphaTextureBounds succeeds but sometimes returns empty bounds like + * { 0x80000000, 0x80000000, 0x80000000, 0x80000000 } + * for small, but not quite zero, sized glyphs. + * Only set as non-empty if the returned bounds are non-empty. + */ +static bool glyph_check_and_set_bounds(SkGlyph* glyph, const RECT& bbox) { + if (bbox.left >= bbox.right || bbox.top >= bbox.bottom) { + return false; } - glyph->fWidth = SkToU16(bbox.right - bbox.left); glyph->fHeight = SkToU16(bbox.bottom - bbox.top); glyph->fLeft = SkToS16(bbox.left); glyph->fTop = SkToS16(bbox.top); + return true; +} + +void SkScalerContext_DW::generateMetrics(SkGlyph* glyph) { + glyph->fWidth = 0; + glyph->fHeight = 0; + glyph->fLeft = 0; + glyph->fTop = 0; + + this->generateAdvance(glyph); + + RECT bbox; + HRVM(this->getBoundingBox(glyph, fRenderingMode, fTextureType, &bbox), + "Requested bounding box could not be determined."); + + if (glyph_check_and_set_bounds(glyph, bbox)) { + return; + } + + // GetAlphaTextureBounds succeeds but returns an empty RECT if there are no + // glyphs of the specified texture type. When this happens, try with the + // alternate texture type. + if (DWRITE_TEXTURE_CLEARTYPE_3x1 == fTextureType) { + HRVM(this->getBoundingBox(glyph, + DWRITE_RENDERING_MODE_ALIASED, + DWRITE_TEXTURE_ALIASED_1x1, + &bbox), + "Fallback bounding box could not be determined."); + if (glyph_check_and_set_bounds(glyph, bbox)) { + glyph->fForceBW = 1; + } + } + // TODO: handle the case where a request for DWRITE_TEXTURE_ALIASED_1x1 + // fails, and try DWRITE_TEXTURE_CLEARTYPE_3x1. } void SkScalerContext_DW::generateFontMetrics(SkPaint::FontMetrics* metrics) { diff --git a/src/ports/SkScalerContext_win_dw.h b/src/ports/SkScalerContext_win_dw.h index e9eab94de1..0bd79e7d82 100644 --- a/src/ports/SkScalerContext_win_dw.h +++ b/src/ports/SkScalerContext_win_dw.h @@ -37,10 +37,10 @@ private: DWRITE_RENDERING_MODE renderingMode, DWRITE_TEXTURE_TYPE textureType); - void getBoundingBox(SkGlyph* glyph, - DWRITE_RENDERING_MODE renderingMode, - DWRITE_TEXTURE_TYPE textureType, - RECT* bbox); + HRESULT getBoundingBox(SkGlyph* glyph, + DWRITE_RENDERING_MODE renderingMode, + DWRITE_TEXTURE_TYPE textureType, + RECT* bbox); SkTDArray fBits; /** The total matrix without the text height scale. */