Diff
Modified: branches/safari-606-branch/Source/WebCore/ChangeLog (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/ChangeLog 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/ChangeLog 2018-07-27 18:39:19 UTC (rev 234322)
@@ -1,3 +1,111 @@
+2018-07-27 Babak Shafiei <[email protected]>
+
+ Cherry-pick r234318. rdar://problem/42467016
+
+ [WIN] Crash when trying to access store pages
+ https://bugs.webkit.org/show_bug.cgi?id=188032
+ <rdar://problem/42467016>
+
+ Reviewed by Brent Fulgham.
+
+ The Windows implementation of GlyphBuffer has an additional member, m_offsets, which represents
+ an additional offset to the position to paint each glyph. It also has two add() functions, one
+ which appends to this vector, and one which doesn't. The one that doesn't append to the vector
+ should never be called on Windows (because Windows requires this vector to be full).
+
+ There were two situations where it was getting called:
+ 1) Inside ComplexTextController
+ 2) Inside display list playback
+
+ Windows shouldn't be using ComplexTextController because the Windows implementation of this
+ class isn't ready yet; instead it should be using UniscribeController. The display list playback
+ code should be used on Windows.
+
+ Rather than fix the function to append an offset, we actually don't need the m_offsets vector
+ in the first place. Instead, we can do it the same way that the Cocoa ports do it, which is to
+ bake the offsets into the glyph advances. This is possible because the GlyphBuffer doesn't need
+ to distinguish between layout advances and paint advances, so we can bake them together and
+ just put paint advances in the GlyphBuffer. This should be a small (probably within-the-noise)
+ performance and memory improvement.
+
+ * platform/graphics/ComplexTextController.cpp:
+ (WebCore::ComplexTextController::ComplexTextController): Make sure that ComplexTextController
+ isn't used on Windows.
+ * platform/graphics/FontCascade.cpp:
+ (WebCore::FontCascade::widthOfTextRange const): Switch from ComplexTextController to
+ UniscribeController on Windows.
+ (WebCore::FontCascade::drawGlyphBuffer const): After deleting the m_offsets vector, there's
+ no reason to consult it when drawing.
+ * platform/graphics/GlyphBuffer.h: Remove m_offsets
+ (WebCore::GlyphBuffer::clear):
+ (WebCore::GlyphBuffer::advanceAt const):
+ (WebCore::GlyphBuffer::add):
+ (WebCore::GlyphBuffer::expandLastAdvance):
+ (WebCore::GlyphBuffer::shrink):
+ (WebCore::GlyphBuffer::swap):
+ (WebCore::GlyphBuffer::offsetAt const): Deleted.
+ * platform/graphics/win/FontCGWin.cpp:
+ (WebCore::FontCascade::drawGlyphs): After deleting the m_offsets vector, there's no reason
+ to consult it when drawing.
+ * platform/graphics/win/FontCascadeDirect2D.cpp:
+ (WebCore::FontCascade::drawGlyphs): Ditto.
+ * platform/graphics/win/UniscribeController.cpp:
+ (WebCore::UniscribeController::shapeAndPlaceItem): Bake in the offsets into the glyph advances.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234318 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-07-27 Myles C. Maxfield <[email protected]>
+
+ [WIN] Crash when trying to access store pages
+ https://bugs.webkit.org/show_bug.cgi?id=188032
+ <rdar://problem/42467016>
+
+ Reviewed by Brent Fulgham.
+
+ The Windows implementation of GlyphBuffer has an additional member, m_offsets, which represents
+ an additional offset to the position to paint each glyph. It also has two add() functions, one
+ which appends to this vector, and one which doesn't. The one that doesn't append to the vector
+ should never be called on Windows (because Windows requires this vector to be full).
+
+ There were two situations where it was getting called:
+ 1) Inside ComplexTextController
+ 2) Inside display list playback
+
+ Windows shouldn't be using ComplexTextController because the Windows implementation of this
+ class isn't ready yet; instead it should be using UniscribeController. The display list playback
+ code should be used on Windows.
+
+ Rather than fix the function to append an offset, we actually don't need the m_offsets vector
+ in the first place. Instead, we can do it the same way that the Cocoa ports do it, which is to
+ bake the offsets into the glyph advances. This is possible because the GlyphBuffer doesn't need
+ to distinguish between layout advances and paint advances, so we can bake them together and
+ just put paint advances in the GlyphBuffer. This should be a small (probably within-the-noise)
+ performance and memory improvement.
+
+ * platform/graphics/ComplexTextController.cpp:
+ (WebCore::ComplexTextController::ComplexTextController): Make sure that ComplexTextController
+ isn't used on Windows.
+ * platform/graphics/FontCascade.cpp:
+ (WebCore::FontCascade::widthOfTextRange const): Switch from ComplexTextController to
+ UniscribeController on Windows.
+ (WebCore::FontCascade::drawGlyphBuffer const): After deleting the m_offsets vector, there's
+ no reason to consult it when drawing.
+ * platform/graphics/GlyphBuffer.h: Remove m_offsets
+ (WebCore::GlyphBuffer::clear):
+ (WebCore::GlyphBuffer::advanceAt const):
+ (WebCore::GlyphBuffer::add):
+ (WebCore::GlyphBuffer::expandLastAdvance):
+ (WebCore::GlyphBuffer::shrink):
+ (WebCore::GlyphBuffer::swap):
+ (WebCore::GlyphBuffer::offsetAt const): Deleted.
+ * platform/graphics/win/FontCGWin.cpp:
+ (WebCore::FontCascade::drawGlyphs): After deleting the m_offsets vector, there's no reason
+ to consult it when drawing.
+ * platform/graphics/win/FontCascadeDirect2D.cpp:
+ (WebCore::FontCascade::drawGlyphs): Ditto.
+ * platform/graphics/win/UniscribeController.cpp:
+ (WebCore::UniscribeController::shapeAndPlaceItem): Bake in the offsets into the glyph advances.
+
2018-07-26 Babak Shafiei <[email protected]>
Cherry-pick r234291. rdar://problem/42650439
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/ComplexTextController.cpp (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/ComplexTextController.cpp 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/ComplexTextController.cpp 2018-07-27 18:39:19 UTC (rev 234322)
@@ -144,6 +144,10 @@
, m_mayUseNaturalWritingDirection(mayUseNaturalWritingDirection)
, m_forTextEmphasis(forTextEmphasis)
{
+#if PLATFORM(WIN)
+ ASSERT_NOT_REACHED();
+#endif
+
computeExpansionOpportunity();
collectComplexTextRuns();
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/FontCascade.cpp (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/FontCascade.cpp 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/FontCascade.cpp 2018-07-27 18:39:19 UTC (rev 234322)
@@ -41,6 +41,10 @@
#include <wtf/text/AtomicStringHash.h>
#include <wtf/text/StringBuilder.h>
+#if PLATFORM(WIN)
+#include "UniscribeController.h"
+#endif
+
namespace WebCore {
using namespace WTF;
using namespace Unicode;
@@ -343,6 +347,15 @@
auto codePathToUse = codePath(run);
if (codePathToUse == Complex) {
+#if PLATFORM(WIN)
+ UniscribeController it(this, run);
+ it.advance(from);
+ offsetBeforeRange = it.runWidthSoFar();
+ it.advance(to);
+ offsetAfterRange = it.runWidthSoFar();
+ it.advance(to);
+ totalWidth = it.runWidthSoFar();
+#else
ComplexTextController complexIterator(*this, run, false, fallbackFonts);
complexIterator.advance(from, nullptr, IncludePartialGlyphs, fallbackFonts);
offsetBeforeRange = complexIterator.runWidthSoFar();
@@ -350,6 +363,7 @@
offsetAfterRange = complexIterator.runWidthSoFar();
complexIterator.advance(run.length(), nullptr, IncludePartialGlyphs, fallbackFonts);
totalWidth = complexIterator.runWidthSoFar();
+#endif
} else {
WidthIterator simpleIterator(this, run, fallbackFonts);
simpleIterator.advance(from, nullptr);
@@ -1418,7 +1432,7 @@
{
// Draw each contiguous run of glyphs that use the same font data.
const Font* fontData = glyphBuffer.fontAt(0);
- FloatSize offset = glyphBuffer.offsetAt(0);
+ // FIXME: Why do we subtract the initial advance's height but not its width???
FloatPoint startPoint(point.x(), point.y() - glyphBuffer.initialAdvance().height());
float nextX = startPoint.x() + glyphBuffer.advanceAt(0).width();
float nextY = startPoint.y() + glyphBuffer.advanceAt(0).height();
@@ -1426,15 +1440,13 @@
unsigned nextGlyph = 1;
while (nextGlyph < glyphBuffer.size()) {
const Font* nextFontData = glyphBuffer.fontAt(nextGlyph);
- FloatSize nextOffset = glyphBuffer.offsetAt(nextGlyph);
- if (nextFontData != fontData || nextOffset != offset) {
+ if (nextFontData != fontData) {
if (shouldDrawIfLoading(*fontData, customFontNotReadyAction))
context.drawGlyphs(*fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint, m_fontDescription.fontSmoothing());
lastFrom = nextGlyph;
fontData = nextFontData;
- offset = nextOffset;
startPoint.setX(nextX);
startPoint.setY(nextY);
}
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/GlyphBuffer.h (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/GlyphBuffer.h 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/GlyphBuffer.h 2018-07-27 18:39:19 UTC (rev 234322)
@@ -27,8 +27,7 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef GlyphBuffer_h
-#define GlyphBuffer_h
+#pragma once
#include "FloatSize.h"
#include "Glyph.h"
@@ -85,9 +84,6 @@
m_advances.clear();
if (m_offsetsInString)
m_offsetsInString->clear();
-#if PLATFORM(WIN)
- m_offsets.clear();
-#endif
}
GlyphBufferGlyph* glyphs(unsigned from) { return m_glyphs.data() + from; }
@@ -110,44 +106,17 @@
{
return m_advances[index];
}
-
- FloatSize offsetAt(unsigned index) const
- {
-#if PLATFORM(WIN)
- return m_offsets[index];
-#else
- UNUSED_PARAM(index);
- return FloatSize();
-#endif
- }
static const unsigned noOffset = UINT_MAX;
- void add(Glyph glyph, const Font* font, float width, unsigned offsetInString = noOffset, const FloatSize* offset = 0)
+ void add(Glyph glyph, const Font* font, float width, unsigned offsetInString = noOffset)
{
- m_font.append(font);
- m_glyphs.append(glyph);
+ GlyphBufferAdvance advance;
+ advance.setWidth(width);
+ advance.setHeight(0);
-#if USE(CG)
- CGSize advance = { width, 0 };
- m_advances.append(advance);
-#else
- m_advances.append(FloatSize(width, 0));
-#endif
+ add(glyph, font, advance, offsetInString);
+ }
-#if PLATFORM(WIN)
- if (offset)
- m_offsets.append(*offset);
- else
- m_offsets.append(FloatSize());
-#else
- UNUSED_PARAM(offset);
-#endif
-
- if (offsetInString != noOffset && m_offsetsInString)
- m_offsetsInString->append(offsetInString);
- }
-
-#if !USE(WINGDI)
void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString = noOffset)
{
m_font.append(font);
@@ -158,7 +127,6 @@
if (offsetInString != noOffset && m_offsetsInString)
m_offsetsInString->append(offsetInString);
}
-#endif
void reverse(unsigned from, unsigned length)
{
@@ -172,6 +140,14 @@
GlyphBufferAdvance& lastAdvance = m_advances.last();
lastAdvance.setWidth(lastAdvance.width() + width);
}
+
+ void expandLastAdvance(GlyphBufferAdvance expansion)
+ {
+ ASSERT(!isEmpty());
+ GlyphBufferAdvance& lastAdvance = m_advances.last();
+ lastAdvance.setWidth(lastAdvance.width() + expansion.width());
+ lastAdvance.setHeight(lastAdvance.height() + expansion.height());
+ }
void saveOffsetsInString()
{
@@ -191,9 +167,6 @@
m_advances.shrink(truncationPoint);
if (m_offsetsInString)
m_offsetsInString->shrink(truncationPoint);
-#if PLATFORM(WIN)
- m_offsets.shrink(truncationPoint);
-#endif
}
private:
@@ -210,12 +183,6 @@
GlyphBufferAdvance s = m_advances[index1];
m_advances[index1] = m_advances[index2];
m_advances[index2] = s;
-
-#if PLATFORM(WIN)
- FloatSize offset = m_offsets[index1];
- m_offsets[index1] = m_offsets[index2];
- m_offsets[index2] = offset;
-#endif
}
Vector<const Font*, 2048> m_font;
@@ -223,10 +190,6 @@
Vector<GlyphBufferAdvance, 2048> m_advances;
GlyphBufferAdvance m_initialAdvance;
std::unique_ptr<Vector<unsigned, 2048>> m_offsetsInString;
-#if PLATFORM(WIN)
- Vector<FloatSize, 2048> m_offsets;
-#endif
};
}
-#endif
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCGWin.cpp (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCGWin.cpp 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCGWin.cpp 2018-07-27 18:39:19 UTC (rev 234322)
@@ -174,9 +174,6 @@
CGAffineTransform savedMatrix = CGContextGetTextMatrix(cgContext);
CGContextSetTextMatrix(cgContext, matrix);
- // Uniscribe gives us offsets to help refine the positioning of combining glyphs.
- FloatSize translation = glyphBuffer.offsetAt(from);
-
CGContextSetFontSize(cgContext, platformData.size());
wkSetCGContextFontRenderingStyle(cgContext, font.platformData().isSystemFont(), false, font.platformData().useGDI());
@@ -192,22 +189,22 @@
Color fillColor = graphicsContext.fillColor();
Color shadowFillColor(shadowColor.red(), shadowColor.green(), shadowColor.blue(), shadowColor.alpha() * fillColor.alpha() / 255);
graphicsContext.setFillColor(shadowFillColor);
- float shadowTextX = point.x() + translation.width() + shadowOffset.width();
+ float shadowTextX = point.x() + shadowOffset.width();
// If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
- float shadowTextY = point.y() + translation.height() + shadowOffset.height() * (graphicsContext.shadowsIgnoreTransforms() ? -1 : 1);
+ float shadowTextY = point.y() + shadowOffset.height() * (graphicsContext.shadowsIgnoreTransforms() ? -1 : 1);
CGContextSetTextPosition(cgContext, shadowTextX, shadowTextY);
CGContextShowGlyphsWithAdvances(cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
if (font.syntheticBoldOffset()) {
- CGContextSetTextPosition(cgContext, point.x() + translation.width() + shadowOffset.width() + font.syntheticBoldOffset(), point.y() + translation.height() + shadowOffset.height());
+ CGContextSetTextPosition(cgContext, point.x() + shadowOffset.width() + font.syntheticBoldOffset(), point.y() + shadowOffset.height());
CGContextShowGlyphsWithAdvances(cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
}
graphicsContext.setFillColor(fillColor);
}
- CGContextSetTextPosition(cgContext, point.x() + translation.width(), point.y() + translation.height());
+ CGContextSetTextPosition(cgContext, point.x(), point.y());
CGContextShowGlyphsWithAdvances(cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
if (font.syntheticBoldOffset()) {
- CGContextSetTextPosition(cgContext, point.x() + translation.width() + font.syntheticBoldOffset(), point.y() + translation.height());
+ CGContextSetTextPosition(cgContext, point.x() + font.syntheticBoldOffset(), point.y());
CGContextShowGlyphsWithAdvances(cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
}
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp 2018-07-27 18:39:19 UTC (rev 234322)
@@ -83,9 +83,6 @@
context->SetTransform(matrix * skewMatrix);
}
- // Uniscribe gives us offsets to help refine the positioning of combining glyphs.
- FloatSize translation = glyphBuffer.offsetAt(from);
-
RELEASE_ASSERT(platformData.dwFont());
RELEASE_ASSERT(platformData.dwFontFace());
@@ -125,9 +122,9 @@
graphicsContext.clearShadow();
Color fillColor = graphicsContext.fillColor();
Color shadowFillColor(shadowColor.red(), shadowColor.green(), shadowColor.blue(), shadowColor.alpha() * fillColor.alpha() / 255);
- float shadowTextX = point.x() + translation.width() + shadowOffset.width();
+ float shadowTextX = point.x() + shadowOffset.width();
// If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
- float shadowTextY = point.y() + translation.height() + shadowOffset.height() * (graphicsContext.shadowsIgnoreTransforms() ? -1 : 1);
+ float shadowTextY = point.y() + shadowOffset.height() * (graphicsContext.shadowsIgnoreTransforms() ? -1 : 1);
auto shadowBrush = graphicsContext.brushWithColor(shadowFillColor);
context->DrawGlyphRun(D2D1::Point2F(shadowTextX, shadowTextY), &glyphRun, shadowBrush);
@@ -135,9 +132,9 @@
context->DrawGlyphRun(D2D1::Point2F(shadowTextX + font.syntheticBoldOffset(), shadowTextY), &glyphRun, shadowBrush);
}
- context->DrawGlyphRun(D2D1::Point2F(point.x() + translation.width(), point.y() + translation.height()), &glyphRun, graphicsContext.solidFillBrush());
+ context->DrawGlyphRun(D2D1::Point2F(point.x(), point.y()), &glyphRun, graphicsContext.solidFillBrush());
if (font.syntheticBoldOffset())
- context->DrawGlyphRun(D2D1::Point2F(point.x() + translation.width() + font.syntheticBoldOffset(), point.y() + translation.height()), &glyphRun, graphicsContext.solidFillBrush());
+ context->DrawGlyphRun(D2D1::Point2F(point.x() + font.syntheticBoldOffset(), point.y()), &glyphRun, graphicsContext.solidFillBrush());
if (hasSimpleShadow)
graphicsContext.setShadow(shadowOffset, shadowBlur, shadowColor);
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/win/UniscribeController.cpp (234321 => 234322)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/win/UniscribeController.cpp 2018-07-27 18:31:38 UTC (rev 234321)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/win/UniscribeController.cpp 2018-07-27 18:39:19 UTC (rev 234322)
@@ -364,8 +364,13 @@
// as well, so that when the time comes to draw those glyphs, we can apply the appropriate
// translation.
if (glyphBuffer) {
- FloatSize size(offsetX, -offsetY);
- glyphBuffer->add(glyph, fontData, advance, GlyphBuffer::noOffset, &size);
+ GlyphBufferAdvance origin(offsetX, -offsetY);
+ if (!glyphBuffer->advancesCount())
+ glyphBuffer->setInitialAdvance(origin);
+ else
+ glyphBuffer->expandLastAdvance(origin);
+ GlyphBufferAdvance advance(-origin.width() + advance, -origin.height());
+ glyphBuffer->add(glyph, fontData, advance);
}
FloatRect glyphBounds = fontData->boundsForGlyph(glyph);
