Changeset 53131 in webkit


Ignore:
Timestamp:
Jan 12, 2010 3:28:18 AM (14 years ago)
Author:
eric@webkit.org
Message:

2010-01-12 Simon Hausmann <​simon.hausmann@nokia.com>

Reviewed by Holger Freyther.

[Qt] WebCore::Path allocates QPainterPath unnecessarily on the heap
​https://bugs.webkit.org/show_bug.cgi?id=33466

WebCore::Path is a pointer to a PlatformPath. In case of Qt that's a
QPainterPath, which itself is a pointer to the elements (QVector).
That creates unecessary allocations in PathQt.cpp.

Replaced the "PlatformPath* m_path;" with a PlatformPathPtr, which
is a plain QPainterPath.

  • platform/graphics/Path.h: (WebCore::Path::platformPath):
  • platform/graphics/qt/GraphicsContextQt.cpp: (WebCore::drawFilledShadowPath): (WebCore::GraphicsContext::fillPath): (WebCore::GraphicsContext::fillRoundedRect): (WebCore::GraphicsContext::addPath): (WebCore::GraphicsContext::clip): (WebCore::GraphicsContext::clipOut):
  • platform/graphics/qt/PathQt.cpp: (WebCore::Path::~Path): (WebCore::Path::operator=): (WebCore::Path::contains): (WebCore::Path::strokeContains): (WebCore::Path::translate): (WebCore::Path::boundingRect): (WebCore::Path::strokeBoundingRect): (WebCore::Path::moveTo): (WebCore::Path::addLineTo): (WebCore::Path::addQuadCurveTo): (WebCore::Path::addBezierCurveTo): (WebCore::Path::addArcTo): (WebCore::Path::closeSubpath): (WebCore::Path::addArc): (WebCore::Path::addRect): (WebCore::Path::addEllipse): (WebCore::Path::clear): (WebCore::Path::isEmpty): (WebCore::Path::debugString): (WebCore::Path::apply): (WebCore::Path::transform):
Location:
trunk/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r53127 r53131  
     12010-01-12  Simon Hausmann  <simon.hausmann@nokia.com>
     2
     3        Reviewed by Holger Freyther.
     4
     5        [Qt] WebCore::Path allocates QPainterPath unnecessarily on the heap
     6        https://bugs.webkit.org/show_bug.cgi?id=33466
     7
     8        WebCore::Path is a pointer to a PlatformPath. In case of Qt that's a
     9        QPainterPath, which itself is a pointer to the elements (QVector).
     10        That creates unecessary allocations in PathQt.cpp.
     11
     12        Replaced the "PlatformPath* m_path;" with a PlatformPathPtr, which
     13        is a plain QPainterPath.
     14
     15        * platform/graphics/Path.h:
     16        (WebCore::Path::platformPath):
     17        * platform/graphics/qt/GraphicsContextQt.cpp:
     18        (WebCore::drawFilledShadowPath):
     19        (WebCore::GraphicsContext::fillPath):
     20        (WebCore::GraphicsContext::fillRoundedRect):
     21        (WebCore::GraphicsContext::addPath):
     22        (WebCore::GraphicsContext::clip):
     23        (WebCore::GraphicsContext::clipOut):
     24        * platform/graphics/qt/PathQt.cpp:
     25        (WebCore::Path::~Path):
     26        (WebCore::Path::operator=):
     27        (WebCore::Path::contains):
     28        (WebCore::Path::strokeContains):
     29        (WebCore::Path::translate):
     30        (WebCore::Path::boundingRect):
     31        (WebCore::Path::strokeBoundingRect):
     32        (WebCore::Path::moveTo):
     33        (WebCore::Path::addLineTo):
     34        (WebCore::Path::addQuadCurveTo):
     35        (WebCore::Path::addBezierCurveTo):
     36        (WebCore::Path::addArcTo):
     37        (WebCore::Path::closeSubpath):
     38        (WebCore::Path::addArc):
     39        (WebCore::Path::addRect):
     40        (WebCore::Path::addEllipse):
     41        (WebCore::Path::clear):
     42        (WebCore::Path::isEmpty):
     43        (WebCore::Path::debugString):
     44        (WebCore::Path::apply):
     45        (WebCore::Path::transform):
     46
    1472010-01-12  Jakub Wieczorek  <faw217@gmail.com>
    248

  • trunk/WebCore/platform/graphics/Path.h

    r52791 r53131  
    3535typedef struct CGPath PlatformPath;
    3636#elif PLATFORM(QT)
    37 #include <qglobal.h>
    38 QT_BEGIN_NAMESPACE
    39 class QPainterPath;
    40 QT_END_NAMESPACE
     37#include <qpainterpath.h>
    4138typedef QPainterPath PlatformPath;
    4239#elif PLATFORM(WX) && USE(WXGC)
    … …  
    6057#else
    6158typedef void PlatformPath;
     59#endif
     60
     61#if PLATFORM(QT)
     62/* QPainterPath is valued based */
     63typedef PlatformPath PlatformPathPtr;
     64#else
     65typedef PlatformPath* PlatformPathPtr;
    6266#endif
    6367
    … …  
    132136        String debugString() const;
    133137
    134         PlatformPath* platformPath() const { return m_path; }
     138        PlatformPathPtr platformPath() const { return m_path; }
    135139
    136140        static Path createRoundedRectangle(const FloatRect&, const FloatSize& roundingRadii);
    … …  
    145149
    146150    private:
    147         PlatformPath* m_path;
     151        PlatformPathPtr m_path;
    148152    };
    149153

  • trunk/WebCore/platform/graphics/qt/GraphicsContextQt.cpp

    r52791 r53131  
    619619}
    620620
    621 static void inline drawFilledShadowPath(GraphicsContext* context, QPainter* p, const QPainterPath *path)
     621static void inline drawFilledShadowPath(GraphicsContext* context, QPainter* p, const QPainterPath& path)
    622622{
    623623    IntSize shadowSize;
    … …  
    626626    if (context->getShadow(shadowSize, shadowBlur, shadowColor)) {
    627627        p->translate(shadowSize.width(), shadowSize.height());
    628         p->fillPath(*path, QBrush(shadowColor));
     628        p->fillPath(path, QBrush(shadowColor));
    629629        p->translate(-shadowSize.width(), -shadowSize.height());
    630630    }
    … …  
    641641
    642642    if (m_common->state.fillPattern || m_common->state.fillGradient || fillColor().alpha()) {
    643         drawFilledShadowPath(this, p, &path);
     643        drawFilledShadowPath(this, p, path);
    644644        if (m_common->state.fillPattern) {
    645645            TransformationMatrix affine;
    … …  
    752752    QPainter* p = m_data->p();
    753753    drawFilledShadowPath(this, p, path.platformPath());
    754     p->fillPath(*path.platformPath(), QColor(color));
     754    p->fillPath(path.platformPath(), QColor(color));
    755755}
    756756
    … …  
    763763{
    764764    QPainterPath newPath = m_data->currentPath;
    765     newPath.addPath(*(path.platformPath()));
     765    newPath.addPath(path.platformPath());
    766766    m_data->currentPath = newPath;
    767767}
    … …  
    10311031        return;
    10321032
    1033     m_data->p()->setClipPath(*path.platformPath(), Qt::IntersectClip);
     1033    m_data->p()->setClipPath(path.platformPath(), Qt::IntersectClip);
    10341034}
    10351035
    … …  
    10451045
    10461046    QPainter* p = m_data->p();
    1047     QPainterPath clippedOut = *path.platformPath();
     1047    QPainterPath clippedOut = path.platformPath();
    10481048    QPainterPath newClip;
    10491049    newClip.setFillRule(Qt::OddEvenFill);

  • trunk/WebCore/platform/graphics/qt/PathQt.cpp

    r46956 r53131  
    5252
    5353Path::Path()
    54     : m_path(new QPainterPath())
    5554{
    5655}
    … …  
    5857Path::~Path()
    5958{
    60     delete m_path;
    6159}
    6260
    6361Path::Path(const Path& other)
    64     : m_path(new QPainterPath(*other.platformPath()))
     62    : m_path(other.m_path)
    6563{
    6664}
    … …  
    6866Path& Path::operator=(const Path& other)
    6967{
    70     if (&other != this) {
    71         delete m_path;
    72         m_path = new QPainterPath(*other.platformPath());
    73     }
    74 
     68    m_path = other.m_path;
    7569    return *this;
    7670}
    … …  
    7872bool Path::contains(const FloatPoint& point, WindRule rule) const
    7973{
    80     Qt::FillRule savedRule = m_path->fillRule();
    81     m_path->setFillRule(rule == RULE_EVENODD ? Qt::OddEvenFill : Qt::WindingFill);
    82 
    83     bool contains = m_path->contains(point);
    84 
    85     m_path->setFillRule(savedRule);
     74    Qt::FillRule savedRule = m_path.fillRule();
     75    const_cast<QPainterPath*>(&m_path)->setFillRule(rule == RULE_EVENODD ? Qt::OddEvenFill : Qt::WindingFill);
     76
     77    bool contains = m_path.contains(point);
     78
     79    const_cast<QPainterPath*>(&m_path)->setFillRule(savedRule);
    8680    return contains;
    8781}
    … …  
    106100    stroke.setDashOffset(pen.dashOffset());
    107101
    108     return (stroke.createStroke(*platformPath())).contains(point);
     102    return stroke.createStroke(m_path).contains(point);
    109103}
    110104
    … …  
    113107    QTransform matrix;
    114108    matrix.translate(size.width(), size.height());
    115     *m_path = (*m_path) * matrix;
     109    m_path = m_path * matrix;
    116110}
    117111
    118112FloatRect Path::boundingRect() const
    119113{
    120     return m_path->boundingRect();
     114    return m_path.boundingRect();
    121115}
    122116
    … …  
    139133        stroke.setDashOffset(pen.dashOffset());
    140134    }
    141     return (stroke.createStroke(*platformPath())).boundingRect();
     135    return stroke.createStroke(m_path).boundingRect();
    142136}
    143137
    144138void Path::moveTo(const FloatPoint& point)
    145139{
    146     m_path->moveTo(point);
     140    m_path.moveTo(point);
    147141}
    148142
    149143void Path::addLineTo(const FloatPoint& p)
    150144{
    151     m_path->lineTo(p);
     145    m_path.lineTo(p);
    152146}
    153147
    154148void Path::addQuadCurveTo(const FloatPoint& cp, const FloatPoint& p)
    155149{
    156     m_path->quadTo(cp, p);
     150    m_path.quadTo(cp, p);
    157151}
    158152
    159153void Path::addBezierCurveTo(const FloatPoint& cp1, const FloatPoint& cp2, const FloatPoint& p)
    160154{
    161     m_path->cubicTo(cp1, cp2, p);
     155    m_path.cubicTo(cp1, cp2, p);
    162156}
    163157
    164158void Path::addArcTo(const FloatPoint& p1, const FloatPoint& p2, float radius)
    165159{
    166     FloatPoint p0(m_path->currentPosition());
     160    FloatPoint p0(m_path.currentPosition());
    167161
    168162    if ((p1.x() == p0.x() && p1.y() == p0.y()) || (p1.x() == p2.x() && p1.y() == p2.y()) || radius == 0.f) {
    169         m_path->lineTo(p1);
     163        m_path.lineTo(p1);
    170164        return;
    171165    }
    … …  
    179173    // all points on a line logic
    180174    if (cos_phi == -1) {
    181         m_path->lineTo(p1);
     175        m_path.lineTo(p1);
    182176        return;
    183177    }
    … …  
    187181        double factor_max = max_length / p1p0_length;
    188182        FloatPoint ep((p0.x() + factor_max * p1p0.x()), (p0.y() + factor_max * p1p0.y()));
    189         m_path->lineTo(ep);
     183        m_path.lineTo(ep);
    190184        return;
    191185    }
    … …  
    227221        anticlockwise = true;
    228222
    229     m_path->lineTo(t_p1p0);
     223    m_path.lineTo(t_p1p0);
    230224
    231225    addArc(p, radius, sa, ea, anticlockwise);
    … …  
    234228void Path::closeSubpath()
    235229{
    236     m_path->closeSubpath();
     230    m_path.closeSubpath();
    237231}
    238232
    … …  
    276270    }
    277271
    278     m_path->moveTo(QPointF(xc + radius  * cos(sar),
     272    m_path.moveTo(QPointF(xc + radius  * cos(sar),
    279273                          yc - radius  * sin(sar)));
    280274
    281     m_path->arcTo(xs, ys, width, height, sa, span);
     275    m_path.arcTo(xs, ys, width, height, sa, span);
    282276}
    283277
    284278void Path::addRect(const FloatRect& r)
    285279{
    286     m_path->addRect(r.x(), r.y(), r.width(), r.height());
     280    m_path.addRect(r.x(), r.y(), r.width(), r.height());
    287281}
    288282
    289283void Path::addEllipse(const FloatRect& r)
    290284{
    291     m_path->addEllipse(r.x(), r.y(), r.width(), r.height());
     285    m_path.addEllipse(r.x(), r.y(), r.width(), r.height());
    292286}
    293287
    294288void Path::clear()
    295289{
    296     *m_path = QPainterPath();
     290    m_path = QPainterPath();
    297291}
    298292
    … …  
    301295    // Don't use QPainterPath::isEmpty(), as that also returns true if there's only
    302296    // one initial MoveTo element in the path.
    303     return !m_path->elementCount();
     297    return !m_path.elementCount();
    304298}
    305299
    … …  
    312306{
    313307    QString ret;
    314     for (int i = 0; i < m_path->elementCount(); ++i) {
    315         const QPainterPath::Element &cur = m_path->elementAt(i);
     308    for (int i = 0; i < m_path.elementCount(); ++i) {
     309        const QPainterPath::Element &cur = m_path.elementAt(i);
    316310
    317311        switch (cur.type) {
    … …  
    324318            case QPainterPath::CurveToElement:
    325319            {
    326                 const QPainterPath::Element &c1 = m_path->elementAt(i + 1);
    327                 const QPainterPath::Element &c2 = m_path->elementAt(i + 2);
     320                const QPainterPath::Element &c1 = m_path.elementAt(i + 1);
     321                const QPainterPath::Element &c2 = m_path.elementAt(i + 2);
    328322
    329323                Q_ASSERT(c1.type == QPainterPath::CurveToDataElement);
    … …  
    349343    FloatPoint points[3];
    350344    pelement.points = points;
    351     for (int i = 0; i < m_path->elementCount(); ++i) {
    352         const QPainterPath::Element& cur = m_path->elementAt(i);
     345    for (int i = 0; i < m_path.elementCount(); ++i) {
     346        const QPainterPath::Element& cur = m_path.elementAt(i);
    353347
    354348        switch (cur.type) {
    … …  
    365359            case QPainterPath::CurveToElement:
    366360            {
    367                 const QPainterPath::Element& c1 = m_path->elementAt(i + 1);
    368                 const QPainterPath::Element& c2 = m_path->elementAt(i + 2);
     361                const QPainterPath::Element& c1 = m_path.elementAt(i + 1);
     362                const QPainterPath::Element& c2 = m_path.elementAt(i + 2);
    369363
    370364                Q_ASSERT(c1.type == QPainterPath::CurveToDataElement);
    … …  
    388382void Path::transform(const TransformationMatrix& transform)
    389383{
    390     if (m_path) {
    391         QTransform mat = transform;
    392         QPainterPath temp = mat.map(*m_path);
    393         delete m_path;
    394         m_path = new QPainterPath(temp);
    395     }
     384    m_path = QTransform(transform).map(m_path);
    396385}
    397386
Note: See TracChangeset for help on using the changeset viewer.