fralx / LimeReport

Report generator for Qt Framework
http://limereport.ru/
Other
403 stars 154 forks source link

LinesChart::paintSerialLines optimizations #349

Closed 7ymekk closed 2 years ago

7ymekk commented 3 years ago

Hi @fralx,

I have found out that LinesChart::paintSerialLines could use some optimizations that IMO would speed up rendering quite significantly.

I'm pasting below the current maxValue & minValue code + LinesChart::paintSerialLines.

Please notice how LinesChart::paintSerialLines uses maxValue & minValue few times,

qreal AbstractSeriesChart::maxValue()
{
    if (m_chartItem->itemMode() == DesignMode) return 40;
    qreal maxValue = 0;
    foreach(SeriesItem* series, m_chartItem->series()){
        foreach(qreal value, series->data()->values()){
            if (value>maxValue) maxValue=value;
        }
    }
    return maxValue;
}

qreal AbstractSeriesChart::minValue()
{
    if (m_chartItem->itemMode() == DesignMode) return 0;
    qreal minValue = 0;
    foreach(SeriesItem* series, m_chartItem->series()){
        foreach(qreal value, series->data()->values()){
            if (value<minValue) minValue=value;
        }
    }
    return minValue;
}
void LinesChart::paintSerialLines(QPainter* painter, QRectF barsRect)
{
    if (valuesCount() == 0) return;

    painter->save();
    painter->setRenderHint(QPainter::Antialiasing,true);
    qreal delta = maxValue() - minValue(); /////////HERE
    delta = genNextValue(delta);

    qreal vStep = barsRect.height() / delta;
    qreal hStep = barsRect.width() / valuesCount();
    qreal topShift = (delta - (maxValue() - minValue())) * vStep +barsRect.top(); /////////HERE

    if (m_chartItem->itemMode() != DesignMode){
        foreach (SeriesItem* series, m_chartItem->series()) {
            QPen pen(series->color());
            pen.setWidth(m_chartItem->seriesLineWidth());
            painter->setPen(pen);
            for (int i = 0; i < series->data()->values().count()-1; ++i ){
                QPoint startPoint = QPoint((i+1) * hStep + barsRect.left() - hStep/2,
                                           (maxValue()*vStep+topShift) - series->data()->values().at(i) * vStep);/////////HERE
                QPoint endPoint = QPoint((i+2) * hStep + barsRect.left() - hStep/2,
                                         (maxValue() * vStep+topShift) - series->data()->values().at(i+1) * vStep);/////////HERE
                drawSegment(painter, startPoint, endPoint, series->color());
            }
        }
    } else {
        drawDesignMode(painter, hStep, vStep, topShift, barsRect);
    }
    painter->restore();
}

Now, I could fix all of the charts with temp variable auto maxValue = this->maxValue() but if maxValue is used in some other method, then this optimization would have to go in every place it's used. Another way around for it would be storing minValue & maxValue inside ChartItem class and changing it whenever m_series is being changed. You know the best place for it so I guess you could give me a hint for this. I plan to push this as pull request in here if you agree it's useful for you.

PS. I noticed that Bar & Pie chart are also using maxValue & minValue few times, so this fix would improve all plots rendering.

Thanks Jakub

7ymekk commented 3 years ago

performance gain is really great:

16 charts: 5 - 13 series with 1830 rows

PC with AMD Ryzen 7 4800H: 8s -> 2.4s ARMv7 Processor rev 10 (single 1GHz core) : 160s -> 38s

fralx commented 3 years ago

Another way around for it would be storing minValue & maxValue inside ChartItem class and changing it whenever m_series is being changed.

I think we can make catchable results for maxValue() and minValue() and drop them on series changes.

Cuteivist commented 2 years ago

Changes were merged. Issue can be closed.