Attachment #362558 for bug #191368

View Details Raw Unified Return to Bug 191368

| Differences between



2019-02-20  Per Arne Vollan  <pvollan@apple.com>

        Layout Test fast/text/international/khmer-selection.html is crashing
        https://bugs.webkit.org/show_bug.cgi?id=191368

        Reviewed by NOBODY (OOPS!).

        GlyphBuffer's offset array wasn't getting filled by UniscribeController.
        Our underlining code requires this array.

        Uniscribe gives us a character -> glyph mapping, so we just have to compute
        the inverse and give it to the GlyphBuffer.

        This patch is written by Myles C. Maxfield.

        Test: fast/text/international/khmer-selection.html.

        * platform/graphics/GlyphBuffer.h:
        (WebCore::GlyphBuffer::add):
        * platform/graphics/displaylists/DisplayListItems.cpp:
        (WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
        * platform/graphics/win/UniscribeController.cpp:
        (WebCore::UniscribeController::advance):
        (WebCore::UniscribeController::itemizeShapeAndPlace):
        (WebCore::UniscribeController::shapeAndPlaceItem):
        * platform/graphics/win/UniscribeController.h:

2019-02-20  Simon Fraser  <simon.fraser@apple.com>

        REGRESSION (r241788>): ASSERTION FAILED: !m_normalFlowListDirty in TestWebKitAPI.WebKit.ResizeReversePaginatedWebView test

Lines 117-123 public: Source/WebCore/platform/graphics/GlyphBuffer.h_sec1
- Source/WebCore/platform/graphics/GlyphBuffer.h -1 / +1 lines
117	add(glyph, font, advance, offsetInString);	117	add(glyph, font, advance, offsetInString);
118	}	118	}
119		119
120	void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString = noOffset)	120	void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString)
121	{	121	{
122	m_font.append(font);	122	m_font.append(font);
123	m_glyphs.append(glyph);	123	m_glyphs.append(glyph);

Lines 364-370 inline GlyphBuffer DrawGlyphs::generateG Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp_sec1
- Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp -1 / +1 lines
364	{	364	{
365	GlyphBuffer result;	365	GlyphBuffer result;
366	for (size_t i = 0; i < m_glyphs.size(); ++i) {	366	for (size_t i = 0; i < m_glyphs.size(); ++i) {
367	result.add(m_glyphs[i], &m_font.get(), m_advances[i]);	367	result.add(m_glyphs[i], &m_font.get(), m_advances[i], GlyphBuffer::noOffset);
368	}	368	}
369	return result;	369	return result;
370	}	370	}



            int itemStart = m_run.rtl() ? index + 1 : indexOfFontTransition;
            int itemLength = m_run.rtl() ? indexOfFontTransition - index : index - indexOfFontTransition;
            m_currentCharacter = baseCharacter + itemStart;
            itemizeShapeAndPlace((isSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, fontData, glyphBuffer);
            indexOfFontTransition = index;
        }
    }


        int itemStart = m_run.rtl() ? 0 : indexOfFontTransition;
        m_currentCharacter = baseCharacter + itemStart;
        itemizeShapeAndPlace((nextIsSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, nextFontData, glyphBuffer);
    }

    m_currentCharacter = baseCharacter + length;
}

void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned stringOffset, unsigned length, const Font* fontData, GlyphBuffer* glyphBuffer)
{
    // ScriptItemize (in Windows XP versions prior to SP2) can overflow by 1.  This is why there is an extra empty item
    // hanging out at the end of the array


    if (m_run.rtl()) {
        for (int i = m_items.size() - 2; i >= 0; i--) {
            if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
                return;
        }
    } else {
        for (unsigned i = 0; i < m_items.size() - 1; i++) {
            if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
                return;
        }
    }

    m_state.fOverrideDirection = m_run.directionalOverride();
}

bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned stringOffset, unsigned i, const Font* fontData, GlyphBuffer* glyphBuffer)
{
    // Determine the string for this item.
    const UChar* str = cp + m_items[i].iCharPos;

    if (!shape(str, len, item, fontData, glyphs, clusters, visualAttributes))
        return true;

    Vector<Optional<unsigned>> stringOffsets(glyphs.size());
    for (unsigned i = 0; i < len; ++i) {
        if (stringOffsets[clusters[i]])
            stringOffsets[clusters[i]] = std::min(*stringOffsets[clusters[i]], i);
        else
            stringOffsets[clusters[i]] = i;
    }

    // We now have a collection of glyphs.
    Vector<GOFFSET> offsets;
    Vector<int> advances;

            else
                glyphBuffer->expandLastAdvance(origin);
            GlyphBufferAdvance glyphAdvance(-origin.width() + advance, -origin.height());
            glyphBuffer->add(glyph, fontData, glyphAdvance, stringOffsets[k].valueOr(0) + stringOffset);
        }

        FloatRect glyphBounds = fontData->boundsForGlyph(glyph);



private:    
    void resetControlAndState();

    void itemizeShapeAndPlace(const UChar*, unsigned stringOffset, unsigned length, const Font*, GlyphBuffer*);
    bool shapeAndPlaceItem(const UChar*, unsigned stringOffset, unsigned index, const Font*, GlyphBuffer*);
    bool shape(const UChar* str, int len, SCRIPT_ITEM, const Font*,
               Vector<WORD>& glyphs, Vector<WORD>& clusters,
               Vector<SCRIPT_VISATTR>& visualAttributes);

Return to Bug 191368

Lines 171-177 void UniscribeController::advance(unsign Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec1
- Source/WebCore/platform/graphics/win/UniscribeController.cpp -7 / +15 lines
171	int itemStart = m_run.rtl() ? index + 1 : indexOfFontTransition;	171	int itemStart = m_run.rtl() ? index + 1 : indexOfFontTransition;
172	int itemLength = m_run.rtl() ? indexOfFontTransition - index : index - indexOfFontTransition;	172	int itemLength = m_run.rtl() ? indexOfFontTransition - index : index - indexOfFontTransition;
173	m_currentCharacter = baseCharacter + itemStart;	173	m_currentCharacter = baseCharacter + itemStart;
174	itemizeShapeAndPlace((isSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemLength, fontData, glyphBuffer);	174	itemizeShapeAndPlace((isSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, fontData, glyphBuffer);
175	indexOfFontTransition = index;	175	indexOfFontTransition = index;
176	}	176	}
177	}	177	}
Lines 183-195 void UniscribeController::advance(unsign Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec2
183		183
184	int itemStart = m_run.rtl() ? 0 : indexOfFontTransition;	184	int itemStart = m_run.rtl() ? 0 : indexOfFontTransition;
185	m_currentCharacter = baseCharacter + itemStart;	185	m_currentCharacter = baseCharacter + itemStart;
186	itemizeShapeAndPlace((nextIsSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemLength, nextFontData, glyphBuffer);	186	itemizeShapeAndPlace((nextIsSmallCaps ? smallCapsBuffer.data() : cp) + itemStart, itemStart, itemLength, nextFontData, glyphBuffer);
187	}	187	}
188		188
189	m_currentCharacter = baseCharacter + length;	189	m_currentCharacter = baseCharacter + length;
190	}	190	}
191		191
192	void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned length, const Font* fontData, GlyphBuffer* glyphBuffer)	192	void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned stringOffset, unsigned length, const Font* fontData, GlyphBuffer* glyphBuffer)
193	{	193	{
194	// ScriptItemize (in Windows XP versions prior to SP2) can overflow by 1. This is why there is an extra empty item	194	// ScriptItemize (in Windows XP versions prior to SP2) can overflow by 1. This is why there is an extra empty item
195	// hanging out at the end of the array	195	// hanging out at the end of the array
Lines 208-219 void UniscribeController::itemizeShapeAn Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec3
208		208
209	if (m_run.rtl()) {	209	if (m_run.rtl()) {
210	for (int i = m_items.size() - 2; i >= 0; i--) {	210	for (int i = m_items.size() - 2; i >= 0; i--) {
211	if (!shapeAndPlaceItem(cp, i, fontData, glyphBuffer))	211	if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
212	return;	212	return;
213	}	213	}
214	} else {	214	} else {
215	for (unsigned i = 0; i < m_items.size() - 1; i++) {	215	for (unsigned i = 0; i < m_items.size() - 1; i++) {
216	if (!shapeAndPlaceItem(cp, i, fontData, glyphBuffer))	216	if (!shapeAndPlaceItem(cp, stringOffset, i, fontData, glyphBuffer))
217	return;	217	return;
218	}	218	}
219	}	219	}
Lines 231-237 void UniscribeController::resetControlAn Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec4
231	m_state.fOverrideDirection = m_run.directionalOverride();	231	m_state.fOverrideDirection = m_run.directionalOverride();
232	}	232	}
233		233
234	bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const Font* fontData, GlyphBuffer* glyphBuffer)	234	bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned stringOffset, unsigned i, const Font* fontData, GlyphBuffer* glyphBuffer)
235	{	235	{
236	// Determine the string for this item.	236	// Determine the string for this item.
237	const UChar* str = cp + m_items[i].iCharPos;	237	const UChar* str = cp + m_items[i].iCharPos;
Lines 253-258 bool UniscribeController::shapeAndPlaceI Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec5
253	if (!shape(str, len, item, fontData, glyphs, clusters, visualAttributes))	253	if (!shape(str, len, item, fontData, glyphs, clusters, visualAttributes))
254	return true;	254	return true;
255		255
		256	Vector<Optional<unsigned>> stringOffsets(glyphs.size());
		257	for (unsigned i = 0; i < len; ++i) {
		258	if (stringOffsets[clusters[i]])
		259	stringOffsets[clusters[i]] = std::min(*stringOffsets[clusters[i]], i);
		260	else
		261	stringOffsets[clusters[i]] = i;
		262	}
		263
256	// We now have a collection of glyphs.	264	// We now have a collection of glyphs.
257	Vector<GOFFSET> offsets;	265	Vector<GOFFSET> offsets;
258	Vector<int> advances;	266	Vector<int> advances;
Lines 367-373 bool UniscribeController::shapeAndPlaceI Source/WebCore/platform/graphics/win/UniscribeController.cpp_sec6
367	else	375	else
368	glyphBuffer->expandLastAdvance(origin);	376	glyphBuffer->expandLastAdvance(origin);
369	GlyphBufferAdvance glyphAdvance(-origin.width() + advance, -origin.height());	377	GlyphBufferAdvance glyphAdvance(-origin.width() + advance, -origin.height());
370	glyphBuffer->add(glyph, fontData, glyphAdvance);	378	glyphBuffer->add(glyph, fontData, glyphAdvance, stringOffsets[k].valueOr(0) + stringOffset);
371	}	379	}
372		380
373	FloatRect glyphBounds = fontData->boundsForGlyph(glyph);	381	FloatRect glyphBounds = fontData->boundsForGlyph(glyph);

Lines 1-3 Source/WebCore/ChangeLog_sec1
- Source/WebCore/ChangeLog +27 lines
		1	2019-02-20 Per Arne Vollan <pvollan@apple.com>
		2
		3	Layout Test fast/text/international/khmer-selection.html is crashing
		4	https://bugs.webkit.org/show_bug.cgi?id=191368
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	GlyphBuffer's offset array wasn't getting filled by UniscribeController.
		9	Our underlining code requires this array.
		10
		11	Uniscribe gives us a character -> glyph mapping, so we just have to compute
		12	the inverse and give it to the GlyphBuffer.
		13
		14	This patch is written by Myles C. Maxfield.
		15
		16	Test: fast/text/international/khmer-selection.html.
		17
		18	* platform/graphics/GlyphBuffer.h:
		19	(WebCore::GlyphBuffer::add):
		20	* platform/graphics/displaylists/DisplayListItems.cpp:
		21	(WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
		22	* platform/graphics/win/UniscribeController.cpp:
		23	(WebCore::UniscribeController::advance):
		24	(WebCore::UniscribeController::itemizeShapeAndPlace):
		25	(WebCore::UniscribeController::shapeAndPlaceItem):
		26	* platform/graphics/win/UniscribeController.h:
		27
1	2019-02-20 Simon Fraser <simon.fraser@apple.com>	28	2019-02-20 Simon Fraser <simon.fraser@apple.com>
2		29
3	REGRESSION (r241788>): ASSERTION FAILED: !m_normalFlowListDirty in TestWebKitAPI.WebKit.ResizeReversePaginatedWebView test	30	REGRESSION (r241788>): ASSERTION FAILED: !m_normalFlowListDirty in TestWebKitAPI.WebKit.ResizeReversePaginatedWebView test

Lines 54-61 public: Source/WebCore/platform/graphics/win/UniscribeController.h_sec1
- Source/WebCore/platform/graphics/win/UniscribeController.h -2 / +2 lines
54	private:	54	private:
55	void resetControlAndState();	55	void resetControlAndState();
56		56
57	void itemizeShapeAndPlace(const UChar, unsigned length, const Font, GlyphBuffer*);	57	void itemizeShapeAndPlace(const UChar, unsigned stringOffset, unsigned length, const Font, GlyphBuffer*);
58	bool shapeAndPlaceItem(const UChar, unsigned index, const Font, GlyphBuffer*);	58	bool shapeAndPlaceItem(const UChar, unsigned stringOffset, unsigned index, const Font, GlyphBuffer*);
59	bool shape(const UChar* str, int len, SCRIPT_ITEM, const Font*,	59	bool shape(const UChar* str, int len, SCRIPT_ITEM, const Font*,
60	Vector<WORD>& glyphs, Vector<WORD>& clusters,	60	Vector<WORD>& glyphs, Vector<WORD>& clusters,
61	Vector<SCRIPT_VISATTR>& visualAttributes);	61	Vector<SCRIPT_VISATTR>& visualAttributes);