Bug 34874 – [Qt] Unnecessary QBrush construction for doing a solid color stroke

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ariya Hidayat]

Created attachment 48614 [details]
Patch

Use the similar trick like in r54347, i.e. use the special brush for
solid color to avoid expensive QBrush constructor.

[Ariya Hidayat]

Created attachment 48616 [details]
2010-02-11  Ariya Hidayat  <[e-mail]>

[Kenneth Rohde Christiansen]

Comment on attachment 48616 [details]
2010-02-11  Ariya Hidayat  <[e-mail]>


> +++ b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> @@ -1112,7 +1112,8 @@ void GraphicsContext::setPlatformStrokeColor(const Color& color, ColorSpace colo
>          return;
>      QPainter* p = m_data->p();
>      QPen newPen(p->pen());
> -    newPen.setColor(color);
> +    m_data->solidColor.setColor(color);
> +    newPen.setBrush(m_data->solidColor);
>      p->setPen(newPen);
>  }
>  

For me this code is not so clear, so maybe a comment would help?

[Ariya Hidayat]

(In reply to comment #2)
> (From update of attachment 48616 [details])
> 
> > +++ b/WebCore/platform/graphics/qt/GraphicsContextQt.cpp
> > @@ -1112,7 +1112,8 @@ void GraphicsContext::setPlatformStrokeColor(const Color& color, ColorSpace colo
> >          return;
> >      QPainter* p = m_data->p();
> >      QPen newPen(p->pen());
> > -    newPen.setColor(color);
> > +    m_data->solidColor.setColor(color);
> > +    newPen.setBrush(m_data->solidColor);
> >      p->setPen(newPen);
> >  }
> >  
> 
> For me this code is not so clear, so maybe a comment would help?

This is common QBrush problem,please see http://trac.webkit.org/changeset/54347 which refers also to http://trac.webkit.org/changeset/37421. Basically it avoids QBrush::QBrush() because the said constructor is expensive, involving allocating QTransform from the heap, necessary for transformed gradient/pattern brush but completely useless for a solid color brush. So we keep our own brush for the solid color which we reuse for this purpose (just change the color, but not construct a new QBrush).

[Kenneth Rohde Christiansen]

Comment on attachment 48616 [details]
2010-02-11  Ariya Hidayat  <[e-mail]>

I would have liked the comment in the actual code, or an inline method with a self described name

[Ariya Hidayat]

(Added one line comment per Kenneth's request)

Manually committed in r54746: http://trac.webkit.org/changeset/54746