[Marble-devel] Review Request: Fix painting of polygons with holes

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105019/
-----------------------------------------------------------

(Updated May 23, 2012, 7 p.m.)


Review request for Marble.


Changes
-------

This patch (-2.diff) is optimized for performance: It determines whether the workaround is needed at all (i.e. inner boundaries exist) and does the separate painting in that case only. For this part it reuses the already calculated polygon, painting it as a polyline (so no brush swapping is needed).

As suggested by Thibault I did some benchmarks using the code below. It paints about 6500 polygons all over the globe. I measured the paint performance using git master (P0), the first patch (P1) and the second patch (P2). Each shows the paint performance when the 6500 polygons have zero or one inner boundary each. The value in the table corresponds to the smoothed value (see code) when constantly moving the earth in longitudinal direction using an arrow key, each time using otherwise same viewport params (radius and latitude, important for the comparison).

   | 1 hole | 0 holes
---------------------
P0 | 147 ms |   49 ms
P1 | 218 ms |   86 ms
P2 | 156 ms |   49 ms

So as expected the first patch is the slowest, and even significantly slower. The second patch is also slower, but only in the case where there are actually holes in the polygon, and the overhead is small (about 6 percent).

#include <marble/MarbleWidget.h>
#include <marble/GeoPainter.h>
#include <marble/GeoDataPolygon.h>
#include <marble/GeoDataLineString.h>

#include <QtGui/QApplication>
#include <QtCore/QTime>
#include <QtCore/QDebug>

using namespace Marble;

class MyMarbleWidget : public MarbleWidget
{
public:
    MyMarbleWidget();
    
    virtual void customPaint(GeoPainter* painter);
    
private:
    QVector<GeoDataPolygon> m_polygons;
};

MyMarbleWidget::MyMarbleWidget()
{
    int const step = 3;
    for ( int lon=-180; lon<180; lon+=step) {
        for ( int lat=-80; lat<80; lat+=step ) {
            GeoDataLineString outer;
            outer << GeoDataCoordinates (lon+0.2*step, lat+0.2*step, 0.0, GeoDataCoordinates::Degree);
            outer << GeoDataCoordinates (lon+0.8*step, lat+0.2*step, 0.0, GeoDataCoordinates::Degree);
            outer << GeoDataCoordinates (lon+0.8*step, lat+0.8*step, 0.0, GeoDataCoordinates::Degree);
            outer << GeoDataCoordinates (lon+0.2*step, lat+0.8*step, 0.0, GeoDataCoordinates::Degree);

            GeoDataPolygon polygon;
            polygon.setOuterBoundary( outer );
            
#if 1           
            GeoDataLineString inner;
            inner << GeoDataCoordinates (lon+0.4*step, lat+0.4*step, 0.0, GeoDataCoordinates::Degree);
            inner << GeoDataCoordinates (lon+0.6*step, lat+0.4*step, 0.0, GeoDataCoordinates::Degree);
            inner << GeoDataCoordinates (lon+0.6*step, lat+0.6*step, 0.0, GeoDataCoordinates::Degree);
            inner << GeoDataCoordinates (lon+0.4*step, lat+0.6*step, 0.0, GeoDataCoordinates::Degree);
            polygon.appendInnerBoundary( inner );
#endif
            
            m_polygons << polygon;
        }
    }
    
}

void MyMarbleWidget::customPaint(GeoPainter* painter)
{
    painter->setPen(Qt::green);
    painter->setBrush(QBrush(Qt::red));
    QTime timer;
    timer.start();
    foreach( const GeoDataPolygon &polygon, m_polygons) {
        painter->drawPolygon(polygon);
    }
    int const elapsed = timer.elapsed();
    static double m_time = 150.0;
    m_time = 0.9 * m_time + 0.1 * elapsed;
    qDebug() << "Painting" << m_polygons.size() << "polygons tooks" << elapsed << "ms, smoothed " << m_time << "ms.";
}

int main(int argc, char** argv)
{
    QApplication app(argc,argv);
    MyMarbleWidget *mapWidget = new MyMarbleWidget;
    mapWidget->setMapThemeId("earth/openstreetmap/openstreetmap.dgml");
    mapWidget->show();
    return app.exec();
}


Description
-------

Currently GeoPainter renders Polygons wrong if they have an outline pen and are filled (think of buildings): A QPolygonF is created from the outer boundary and the inner rings are subtracted. Now QPainter paints the stroke of such Polygons by connecting each point, leading to unwanted connections between the outer boundary and the inner holes. See http://i.imgur.com/25rjs.png (from Ander).

The patch changes this to paint the background (brush) as previous, but paints the outer and inner rings' outlines individually. Please review with performance in mind. Any ideas how to paint everything in one go (should be faster)?


Diffs (updated)
-----

  src/lib/GeoPainter.cpp aa18e47 

Diff: http://git.reviewboard.kde.org/r/105019/diff/


Testing
-------


Thanks,

Dennis Nienhüser

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20120523/9230b195/attachment-0001.html>