Bug 61859 – Out of memory when drawing text shadow for scale < 1.0

• Status: RESOLVED
• Resolution: INVALID
• Component: Platform

[Mariusz Grzegorczyk]

Created attachment 95593 [details]
Web page: Korean text with shadow property

When drawing text shadow in EFL port(EWebLauncher) out of memory occurs.
Steps to reproduce: run EWebLauncher with attached file, zoom out, OOM occurs.

The problem is that for shadowed text(e.g. in attachment) font data for some glyphs is different than for first. In this case drawGlyphs() function is invoked(FontFastPath.cpp, line 387).

374	void Font::drawGlyphBuffer(GraphicsContext* context, const GlyphBuffer& glyphBuffer, const FloatPoint& point) const
375	{   
376	    // Draw each contiguous run of glyphs that use the same font data.
377	    const SimpleFontData* fontData = glyphBuffer.fontDataAt(0);
378	    FloatSize offset = glyphBuffer.offsetAt(0);
379	    FloatPoint startPoint(point);
380	    float nextX = startPoint.x();
381	    int lastFrom = 0;
382	    int nextGlyph = 0;
383	    while (nextGlyph < glyphBuffer.size()) {
384	        const SimpleFontData* nextFontData = glyphBuffer.fontDataAt(nextGlyph);
385	        FloatSize nextOffset = glyphBuffer.offsetAt(nextGlyph);
386	        if (nextFontData != fontData || nextOffset != offset) {  /// <-------------------------------------------
387	            drawGlyphs(context, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
388	
389	            lastFrom = nextGlyph;
390	            fontData = nextFontData;
391	            offset = nextOffset;
392	            startPoint.setX(nextX);
393	        }
394	        nextX += glyphBuffer.advanceAt(nextGlyph);
395	        nextGlyph++;
396	    }
397	
398	    drawGlyphs(context, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
399	}

Than path is as below:
FontCairo.cpp: drawGlyphs()->drawGlyphsShadow()->beginShadowLayer()
ContextShadowCairo.cpp: beginShadowLayer()->adjustBlurDistance()

86	PlatformContext ContextShadow::beginShadowLayer(GraphicsContext* context, const FloatRect& layerArea)
87	{
88	    adjustBlurDistance(context);
89	
90	    double x1, x2, y1, y2;
91	    cairo_clip_extents(context->platformContext()->cr(), &x1, &y1, &x2, &y2);
92	    IntRect layerRect = calculateLayerBoundingRect(context, layerArea, IntRect(x1, y1, x2 - x1, y2 - y1));
93	
94	    // Don't paint if we are totally outside the clip region.
95	    if (layerRect.isEmpty())
96	        return 0;
97	
98	    m_layerImage = getScratchBuffer(layerRect.size());
99	    m_layerContext = cairo_create(m_layerImage);
100	
101	    // Always clear the surface first.
102	    cairo_set_operator(m_layerContext, CAIRO_OPERATOR_CLEAR);
103	    cairo_paint(m_layerContext);
104	    cairo_set_operator(m_layerContext, CAIRO_OPERATOR_OVER);
105	
106	    cairo_translate(m_layerContext, m_layerContextTranslation.x(), m_layerContextTranslation.y());
107	    return m_layerContext;
108	}


181	void ContextShadow::adjustBlurDistance(GraphicsContext* context)
182	{
183	    const AffineTransform transform = context->getCTM();
184	
185	    // Adjust blur if we're scaling, since the radius must not be affected by transformations.
186	    if (transform.isIdentity())
187	        return;
188	
........................
206	    m_blurDistance = roundf(static_cast<float>(m_blurDistance) / scale);
207	}

Using m_blurDistance size of cairo context is calculated. 
For GTK port there is no transformation set on cairo, so transform.isIdentity() always returns true(only because of it OOM doesn't occur in GTK). But for EFL port when zoom is done following function(ewk_view.cpp) is invoked, which changes cairo_matrix.

3324	void ewk_view_paint_context_translate(Ewk_View_Paint_Context* ctxt, float x, float y)
3325	{
3326	    EINA_SAFETY_ON_NULL_RETURN(ctxt);
3327	
3328	    ctxt->gc->translate(x, y);
3329	}

In this for scale < 1.0 m_blurDistance is calculated as, e.g. for 0.5
m_blurDistance = m_blurDistance/0.5 several times, so this distance growths and growths, and cairo context is tried to be created for greater and greater rectangle, and at the end out of memory condition occurs.

[Ryuan Choi]

Created attachment 95697 [details]
Patch

[Ryuan Choi]

I'll prepare simple local patch to prevent it.
But, I'm not sure that it's way to go.

This is related to not only EFL, but also GTK.
could anyone help to find proper way?

[Alexey Proskuryakov]

Since it's an out of memory crash, the fix should be testable in a regression test.

It seems surprising if cross-platform code needs to be modified for a EFL only bug.

[Simon Fraser (smfr)]

Comment on attachment 95697 [details]
Patch

Seems like an EFL bug.

[Ryuan Choi]

Created attachment 95853 [details]
testpage.html

This html page will make gtklauncher and eweblauncher sluggish on host pc because of memory.

I tested chrome, QtTestBrowser, but they looks fine.

[Ryuan Choi]

Created attachment 95854 [details]
screenshot

[Ryuan Choi]

(In reply to comment #4)
> (From update of attachment 95697 [details])
> Seems like an EFL bug.

No, It's not related to EFL.
At least, it will be an issue of cairo port.

But, I wonder why only Cairo(GTK and EFL I tested) have this issue.
I should study other ports more.

[Mariusz Grzegorczyk]

This is general issue of cairo backend. Wrong is calculating m_blurDistance in each Font::drawGlyphs() (FontCairo.cpp) invocation. Maybe ContextShadow should be cleared at the start of drawGlyphsShadow() function.

[Leandro Pereira]

(In reply to comment #8)
> This is general issue of cairo backend. Wrong is calculating m_blurDistance in
> each Font::drawGlyphs() (FontCairo.cpp) invocation. Maybe ContextShadow should
> be cleared at the start of drawGlyphsShadow() function.

Is it also possible to reproduce this on the WinCairo port?

[Ryuan Choi]

(In reply to comment #9)
> (In reply to comment #8)
> > This is general issue of cairo backend. Wrong is calculating m_blurDistance in
> > each Font::drawGlyphs() (FontCairo.cpp) invocation. Maybe ContextShadow should
> > be cleared at the start of drawGlyphsShadow() function.
> 
> Is it also possible to reproduce this on the WinCairo port?

I'm not sure because I can't prepare WinCairo port.
I'll try to prepare and test once more.

Anyway, I found some history.

adjustBlurDistance was introduced at Bug 51161 and it is only for Qt and Cairo port.
However, Qt doesn't call drawGlyphs (I tested on qt-everywhere-opensource-src-4.7.3).

As my understanding Bug 51161, adjustBlurDistance should be called only one time.
but drawGlyphBuffer calls repeatedly adjustBlurDistance via drawGlyphs.

IMO, current patch is valid or adjustBlurDistance should check whether it was applied.

[Ryuan Choi]

In latest, this isn't reproduced.