[Marble-devel] Review Request 124511: Merged the decoration(124154) and the label placement(124498) review requests.

Dennis Nienhüser dennis at nienhueser.de
Tue Aug 11 17:54:42 UTC 2015



> On Aug. 9, 2015, 8:49 p.m., Dennis Nienhüser wrote:
> > In https://git.reviewboard.kde.org/r/124154/ I suggested to change the algorithm to compute the offsets to avoid that they get too large for small buildings. Looking at the result of that I realized that my suggestion was a bad one: We now have offsets that vary quite a lot and give the impression of different building heights that altogether look a bit strange. So I sat down to come up with a better suggestion, and while playing with it ran into other issues with the current approach. Here's a screenshot that shows what I mean (left image part):
> > 
> > ![3D effect issues](http://nienhueser.de/marble/marble-3d-building-comparison.png)
> > - A is the result of my bad suggestion: The offset now depends on the area of the polygon, which makes no real sense
> > - B is a rendering placement error. The decoration resembles the walls, which are based on the floor, while the polygon itself resembles the roof of the building. To achieve the 3D-effect, the roof needs to be moved, not the walls/floor.
> > - C is the result of the "cheap" rendering where we do not really calculate the walls, but just a shifted polygon. This is the hardest one to fix.
> > 
> > To fix A, we can use a more realistic offset calculation. A good approach is to calculate the height (in pixels) of the virtual camera observing the scene, relate it to the (assumed) height of the building (again in pixels) and calculate the wall size from that. Fixing B is simpler, we just need to shift the painter of the "real" polygon instead of the decoration polygon. Fixing C is the hardest part and requires us to calculate all visible building walls as polygons and paint them. Here is a patch that does all that:
> > 
> > ```
> > diff --git a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
> > index 0e0f881..e4321a7 100644
> > --- a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
> > +++ b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
> > @@ -41,6 +41,10 @@ const float GeoPolygonGraphicsItem::s_maximumOffset = 8;
> >  
> >  void GeoPolygonGraphicsItem::createDecorations()
> >  {
> > +    if (isDecoration()) {
> > +        return;
> > +    }
> > +
> >      GeoPolygonGraphicsItem *fake3D;
> >  
> >      switch ( feature()->visualCategory() ) {
> > @@ -104,6 +108,28 @@ void GeoPolygonGraphicsItem::createDecorations()
> >      this->addDecoration(fake3D);
> >  }
> >  
> > +QPointF GeoPolygonGraphicsItem::buildingOffset(const QPointF &point, const ViewportParams *viewport) const
> > +{
> > +    qreal const cameraFactor = 0.5 * tan(0.5 * 110 * DEG2RAD);
> > +    qreal const buildingHeightMeter = 8.0;
> > +    qreal const buildingFactor = buildingHeightMeter / EARTH_RADIUS;
> > +
> > +    qreal const cameraHeightPixel = viewport->width() * cameraFactor;
> > +    qreal const buildingHeightPixel = viewport->radius() * buildingFactor;
> > +
> > +    qreal const offsetX = point.x() - viewport->width() / 2.0;
> > +    qreal const alpha1 = atan2(offsetX, cameraHeightPixel);
> > +    qreal const alpha2 = atan2(offsetX, cameraHeightPixel-buildingHeightPixel);
> > +    qreal const shiftX = 2 * (cameraHeightPixel-buildingHeightPixel) * sin(0.5*(alpha2-alpha1));
> > +
> > +    qreal const offsetY = point.y() - viewport->height() / 2.0;
> > +    qreal const beta1 = atan2(offsetY, cameraHeightPixel);
> > +    qreal const beta2 = atan2(offsetY, cameraHeightPixel-buildingHeightPixel);
> > +    qreal const shiftY = 2 * (cameraHeightPixel-buildingHeightPixel) * sin(0.5*(beta2-beta1));
> > +
> > +    return QPointF(shiftX, shiftY);
> > +}
> > +
> >  
> >  const GeoDataLatLonAltBox& GeoPolygonGraphicsItem::latLonAltBox() const
> >  {
> > @@ -118,9 +144,9 @@ const GeoDataLatLonAltBox& GeoPolygonGraphicsItem::latLonAltBox() const
> >  
> >  void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* viewport )
> >  {
> > -    Q_UNUSED( viewport );
> > -
> >      painter->save();
> > +    bool const isBuildingFrame = isDecoration();
> > +    bool const isBuildingRoof = !decorations().isEmpty();
> >  
> >      QPen currentPen = painter->pen();
> >  
> > @@ -128,7 +154,7 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
> >          painter->setPen( QPen() );
> >      }
> >      else {
> > -        if ( !style()->polyStyle().outline() || isDecoration() ) {
> > +        if ( !style()->polyStyle().outline() || isBuildingFrame ) {
> >              currentPen.setColor( Qt::transparent );
> >          }
> >          else {
> > @@ -143,13 +169,6 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
> >  
> >              if ( currentPen.style() != style()->lineStyle().penStyle() )
> >                  currentPen.setStyle( style()->lineStyle().penStyle() );
> > -
> > -            if ( painter->mapQuality() != Marble::HighQuality
> > -                    && painter->mapQuality() != Marble::PrintQuality ) {
> > -                QColor penColor = currentPen.color();
> > -                penColor.setAlpha( 255 );
> > -                currentPen.setColor( penColor );
> > -            }
> >          }
> >  
> >          if ( painter->pen() != currentPen )
> > @@ -160,7 +179,7 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
> >                  painter->setBrush( QColor( Qt::transparent ) );
> >          }
> >          else {
> > -            if ( isDecoration() ) {
> > +            if ( isBuildingFrame ) {
> >                  painter->setBrush( style()->polyStyle().paintedColor().darker(150) );
> >              } else if ( painter->brush().color() != style()->polyStyle().paintedColor() ) {
> >                  painter->setBrush( style()->polyStyle().paintedColor() );
> > @@ -168,36 +187,58 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
> >          }
> >      }
> >  
> > -    if ( isDecoration() ) {
> > -        qreal polygonCenterX, polygonCenterY, cameraX, cameraY,
> > -              polygonLeft, polygonTop;
> > -        viewport->screenCoordinates( latLonAltBox().center(), polygonCenterX, polygonCenterY );
> > -        viewport->screenCoordinates( viewport->focusPoint(), cameraX, cameraY );
> > -        viewport->screenCoordinates( latLonAltBox().west(), latLonAltBox().north(),
> > -                                     polygonLeft, polygonTop);
> > -
> > -        qreal polygonWidth = (polygonCenterX - polygonLeft) * 2;
> > -        qreal polygonHeight = (polygonCenterY - polygonTop) * 2;
> > -
> > -        QVector2D polygonCenter(polygonCenterX, polygonCenterY);
> > -        QVector2D cameraPosition(cameraX, cameraY);
> > -        QVector2D translate = cameraPosition - polygonCenter;
> > -
> > -        qreal offset = qSqrt( (qPow(polygonWidth, 2) + qPow(polygonHeight, 2)) /
> > -                              (qPow(viewport->width(), 2) + qPow(viewport->height(), 2)) ) *
> > -                       qPow( s_maximumOffset, 2 );
> > -        if (offset > s_maximumOffset) offset = s_maximumOffset;
> > -
> > -        translate.setX( ((translate.x() / (viewport->width() / 2))) * offset);
> > -        translate.setY( ((translate.y() / (viewport->height() / 2))) * offset);
> > -
> > -        painter->translate( translate.toPointF() );
> > +    if ( isBuildingFrame ) {
> > +        QVector<QPolygonF*> polygons;
> > +        if (m_polygon) {
> > +            viewport->screenCoordinates(m_polygon->outerBoundary(), polygons);
> > +        } else if (m_ring) {
> > +            viewport->screenCoordinates(*m_ring, polygons);
> > +        }
> > +        foreach(QPolygonF* polygon, polygons) {
> > +            if (polygon->isEmpty()) {
> > +                continue;
> > +            }
> > +            int const size = polygon->size();
> > +            for (int i=1; i<size; ++i) {
> > +                QPointF const & a = (*polygon)[i-1];
> > +                QPointF const & b = (*polygon)[i];
> > +                QPointF const shiftA = buildingOffset(a, viewport);
> > +                if (polygon->contains(shiftA)) {
> > +                    continue;
> > +                }
> > +                QPointF const shiftB = buildingOffset(b, viewport);
> > +                if (polygon->contains(shiftB)) {
> > +                    continue;
> > +                }
> > +                painter->drawPolygon(*polygon);
> > +                QPolygonF buildingSide = QPolygonF() << a << a+shiftA << b+shiftB << b;
> > +                painter->drawPolygon(buildingSide);
> > +            }
> > +        }
> > +        qDeleteAll(polygons);
> >      }
> > -
> > -    if ( m_polygon ) {
> > -        painter->drawPolygon( *m_polygon );
> > -    } else if ( m_ring ) {
> > -        painter->drawPolygon( *m_ring );
> > +    else if (isBuildingRoof) {
> > +        QVector<QPolygonF*> polygons;
> > +        if (m_polygon) {
> > +            viewport->screenCoordinates(m_polygon->outerBoundary(), polygons);
> > +        } else if (m_ring) {
> > +            viewport->screenCoordinates(*m_ring, polygons);
> > +        }
> > +        foreach(QPolygonF* polygon, polygons) {
> > +            QPolygonF buildingRoof;
> > +            foreach(const QPointF &point, *polygon) {
> > +                buildingRoof << point + buildingOffset(point, viewport);
> > +            }
> > +            painter->drawPolygon(buildingRoof);
> > +        }
> > +        qDeleteAll(polygons);
> > +    }
> > +    else {
> > +        if ( m_polygon ) {
> > +            painter->drawPolygon( *m_polygon );
> > +        } else if ( m_ring ) {
> > +            painter->drawPolygon( *m_ring );
> > +        }
> >      }
> >  
> >      painter->restore();
> > diff --git a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
> > index 7c4bc9a..90f3bb9 100644
> > --- a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
> > +++ b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
> > @@ -30,8 +30,9 @@ public:
> >  
> >      virtual void paint( GeoPainter* painter, const ViewportParams *viewport );
> >  
> > -protected:
> > +private:
> >      virtual void createDecorations();
> > +    QPointF buildingOffset(const QPointF &point, const ViewportParams *viewport) const;
> >  
> >      const GeoDataPolygon *const m_polygon;
> >      const GeoDataLinearRing *const m_ring;
> > ```
> > 
> > It works very well, even performance-wise. This is because there is little overhead in geo->screen position calculation. Nevertheless because of C several more polygons are being painted (all visible walls), so performance is worse than before. It does run quite well on my Nexus 4 though even with high quality painting enabled during animations, here is an awesome screencast showing it: http://nienhueser.de/marble/marble-android-3d-buildings.mp4 and a screenshot:
> > 
> > ![3D buildings](http://nienhueser.de/marble/marble-android-3d-buildings.png)
> > 
> > For a final implementation we might look into the following approach:
> > - At higher zoom, disable all decorations
> > - At medium zoom, use the fake 3D approach
> > - At high zoom, use the real 3D approach from here
> >  
> > I'd prefer that to taking the map quality into account: Map quality is never going to be changed by the vast majority of all users. I'd also like to enable the decorations during animations (e.g. while panning the map).
> 
> Dávid Kolozsvári wrote:
>     This is very cool, especially the video!
>     Shifting the "base"(real) polygon was a great idea, it works much better this way. Though the math in GeoPolygonGraphicsItem::buildingOffset() is a "little" more advanced than my approach, but I understand it at least, and I even learned some math this way :)
>     
>     I will update this patch with this solution, and with your suggestion of taking into account the zoom and map quality.
> 
> Torsten Rahn wrote:
>     In the screenshots it looks like some inner rings are missing in the polygon ... is this a fault of the patch or is it due to the data?
> 
> Torsten Rahn wrote:
>     Does the tan() calculation for determining the cameraFactor get optimized away? I also wonder whether it's possible to develop sin(atan(x)-atan(y)) into a power series to improve performance. Oh and I'd really like to get rid of EARTH_RADIUS whereever possible.
> 
> Dennis Nienhüser wrote:
>     The compiler should take care of tan() when optimizations are enabled. Alternatively I could make the variable static to enforce a one-time evaluation only, but I'm pretty sure the compiler handles it fine on its own.
>     
>     In order to get rid of EARTH_RADIUS I'd need access to the planet, which viewportparams does not yet offer. Similarly I'd rather share '110' (camera viewing angle) with the same value used in MarbleAbstractPresenter. Do you think it makes sense to make both available in ViewportParams? Or by some other means?
> 
> Dennis Nienhüser wrote:
>     Re inner rings missing: Which ones are you missing? I can't spot any in this part of the map, see http://www.openstreetmap.org/relation/62518#map=19/48.99820/8.40971
> 
> Dennis Nienhüser wrote:
>     Here's an update for the isBuildingFrame rendering part that improves performance a lot: The building floor does not need to be painted, the buildingOffset calculation is just needed once for every polygon point, the containment check did not have an effect (wrong argument and wrong logic).
>     
>     ```
>     diff --git a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
>     index 0e0f881..bb55f0b 100644
>     --- a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
>     +++ b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp
>     @@ -41,6 +41,10 @@ const float GeoPolygonGraphicsItem::s_maximumOffset = 8;
>      
>      void GeoPolygonGraphicsItem::createDecorations()
>      {
>     +    if (isDecoration()) {
>     +        return;
>     +    }
>     +
>          GeoPolygonGraphicsItem *fake3D;
>      
>          switch ( feature()->visualCategory() ) {
>     @@ -104,6 +108,28 @@ void GeoPolygonGraphicsItem::createDecorations()
>          this->addDecoration(fake3D);
>      }
>      
>     +QPointF GeoPolygonGraphicsItem::buildingOffset(const QPointF &point, const ViewportParams *viewport) const
>     +{
>     +    qreal const cameraFactor = 0.5 * tan(0.5 * 110 * DEG2RAD);
>     +    qreal const buildingHeightMeter = 8.0;
>     +    qreal const buildingFactor = buildingHeightMeter / EARTH_RADIUS;
>     +
>     +    qreal const cameraHeightPixel = viewport->width() * cameraFactor;
>     +    qreal const buildingHeightPixel = viewport->radius() * buildingFactor;
>     +
>     +    qreal const offsetX = point.x() - viewport->width() / 2.0;
>     +    qreal const alpha1 = atan2(offsetX, cameraHeightPixel);
>     +    qreal const alpha2 = atan2(offsetX, cameraHeightPixel-buildingHeightPixel);
>     +    qreal const shiftX = 2 * (cameraHeightPixel-buildingHeightPixel) * sin(0.5*(alpha2-alpha1));
>     +
>     +    qreal const offsetY = point.y() - viewport->height() / 2.0;
>     +    qreal const beta1 = atan2(offsetY, cameraHeightPixel);
>     +    qreal const beta2 = atan2(offsetY, cameraHeightPixel-buildingHeightPixel);
>     +    qreal const shiftY = 2 * (cameraHeightPixel-buildingHeightPixel) * sin(0.5*(beta2-beta1));
>     +
>     +    return QPointF(shiftX, shiftY);
>     +}
>     +
>      
>      const GeoDataLatLonAltBox& GeoPolygonGraphicsItem::latLonAltBox() const
>      {
>     @@ -118,9 +144,9 @@ const GeoDataLatLonAltBox& GeoPolygonGraphicsItem::latLonAltBox() const
>      
>      void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* viewport )
>      {
>     -    Q_UNUSED( viewport );
>     -
>          painter->save();
>     +    bool const isBuildingFrame = isDecoration();
>     +    bool const isBuildingRoof = !decorations().isEmpty();
>      
>          QPen currentPen = painter->pen();
>      
>     @@ -128,7 +154,7 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
>              painter->setPen( QPen() );
>          }
>          else {
>     -        if ( !style()->polyStyle().outline() || isDecoration() ) {
>     +        if ( !style()->polyStyle().outline() || isBuildingFrame ) {
>                  currentPen.setColor( Qt::transparent );
>              }
>              else {
>     @@ -143,13 +169,6 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
>      
>                  if ( currentPen.style() != style()->lineStyle().penStyle() )
>                      currentPen.setStyle( style()->lineStyle().penStyle() );
>     -
>     -            if ( painter->mapQuality() != Marble::HighQuality
>     -                    && painter->mapQuality() != Marble::PrintQuality ) {
>     -                QColor penColor = currentPen.color();
>     -                penColor.setAlpha( 255 );
>     -                currentPen.setColor( penColor );
>     -            }
>              }
>      
>              if ( painter->pen() != currentPen )
>     @@ -160,7 +179,7 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
>                      painter->setBrush( QColor( Qt::transparent ) );
>              }
>              else {
>     -            if ( isDecoration() ) {
>     +            if ( isBuildingFrame ) {
>                      painter->setBrush( style()->polyStyle().paintedColor().darker(150) );
>                  } else if ( painter->brush().color() != style()->polyStyle().paintedColor() ) {
>                      painter->setBrush( style()->polyStyle().paintedColor() );
>     @@ -168,36 +187,53 @@ void GeoPolygonGraphicsItem::paint( GeoPainter* painter, const ViewportParams* v
>              }
>          }
>      
>     -    if ( isDecoration() ) {
>     -        qreal polygonCenterX, polygonCenterY, cameraX, cameraY,
>     -              polygonLeft, polygonTop;
>     -        viewport->screenCoordinates( latLonAltBox().center(), polygonCenterX, polygonCenterY );
>     -        viewport->screenCoordinates( viewport->focusPoint(), cameraX, cameraY );
>     -        viewport->screenCoordinates( latLonAltBox().west(), latLonAltBox().north(),
>     -                                     polygonLeft, polygonTop);
>     -
>     -        qreal polygonWidth = (polygonCenterX - polygonLeft) * 2;
>     -        qreal polygonHeight = (polygonCenterY - polygonTop) * 2;
>     -
>     -        QVector2D polygonCenter(polygonCenterX, polygonCenterY);
>     -        QVector2D cameraPosition(cameraX, cameraY);
>     -        QVector2D translate = cameraPosition - polygonCenter;
>     -
>     -        qreal offset = qSqrt( (qPow(polygonWidth, 2) + qPow(polygonHeight, 2)) /
>     -                              (qPow(viewport->width(), 2) + qPow(viewport->height(), 2)) ) *
>     -                       qPow( s_maximumOffset, 2 );
>     -        if (offset > s_maximumOffset) offset = s_maximumOffset;
>     -
>     -        translate.setX( ((translate.x() / (viewport->width() / 2))) * offset);
>     -        translate.setY( ((translate.y() / (viewport->height() / 2))) * offset);
>     -
>     -        painter->translate( translate.toPointF() );
>     +    if ( isBuildingFrame ) {
>     +        QVector<QPolygonF*> polygons;
>     +        if (m_polygon) {
>     +            viewport->screenCoordinates(m_polygon->outerBoundary(), polygons);
>     +        } else if (m_ring) {
>     +            viewport->screenCoordinates(*m_ring, polygons);
>     +        }
>     +        foreach(QPolygonF* polygon, polygons) {
>     +            if (polygon->isEmpty()) {
>     +                continue;
>     +            }
>     +            int const size = polygon->size();
>     +            QPointF & a = (*polygon)[0];
>     +            QPointF shiftA = a + buildingOffset(a, viewport);
>     +            for (int i=1; i<size; ++i) {
>     +                QPointF const & b = (*polygon)[i];
>     +                QPointF const shiftB = b + buildingOffset(b, viewport);
>     +                QPolygonF buildingSide = QPolygonF() << a << shiftA << shiftB << b;
>     +                painter->drawPolygon(buildingSide);
>     +                a = b;
>     +                shiftA = shiftB;
>     +            }
>     +        }
>     +        qDeleteAll(polygons);
>          }
>     -
>     -    if ( m_polygon ) {
>     -        painter->drawPolygon( *m_polygon );
>     -    } else if ( m_ring ) {
>     -        painter->drawPolygon( *m_ring );
>     +    else if (isBuildingRoof) {
>     +        QVector<QPolygonF*> polygons;
>     +        if (m_polygon) {
>     +            viewport->screenCoordinates(m_polygon->outerBoundary(), polygons);
>     +        } else if (m_ring) {
>     +            viewport->screenCoordinates(*m_ring, polygons);
>     +        }
>     +        foreach(QPolygonF* polygon, polygons) {
>     +            QPolygonF buildingRoof;
>     +            foreach(const QPointF &point, *polygon) {
>     +                buildingRoof << point + buildingOffset(point, viewport);
>     +            }
>     +            painter->drawPolygon(buildingRoof);
>     +        }
>     +        qDeleteAll(polygons);
>     +    }
>     +    else {
>     +        if ( m_polygon ) {
>     +            painter->drawPolygon( *m_polygon );
>     +        } else if ( m_ring ) {
>     +            painter->drawPolygon( *m_ring );
>     +        }
>          }
>      
>          painter->restore();
>     diff --git a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
>     index 7c4bc9a..90f3bb9 100644
>     --- a/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
>     +++ b/src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h
>     @@ -30,8 +30,9 @@ public:
>      
>          virtual void paint( GeoPainter* painter, const ViewportParams *viewport );
>      
>     -protected:
>     +private:
>          virtual void createDecorations();
>     +    QPointF buildingOffset(const QPointF &point, const ViewportParams *viewport) const;
>      
>          const GeoDataPolygon *const m_polygon;
>          const GeoDataLinearRing *const m_ring;
>     ```
> 
> Dennis Nienhüser wrote:
>     Note that the fake 3D rendering can easily be implemented with the help of the buildingOffset method: Pass the center of the bounding box in screen coordinates as the point and use the result to translate the painter.
> 
> Torsten Rahn wrote:
>     I just found something exciting: :-)
>     
>     Ignoring the difference between atan2(a,b) and atan(a/b) (which we could easily handle by adding pi for negative a and considering the a = 0 case, see https://en.wikipedia.org/wiki/Atan2) we could drastically simplify the complex trigonometric calculation by using its algebraic representation:
>     
>     sin(atan(x)-atan(y)) = 
>     
>     (x+y)/sqrt((x^2+1)*(y^2+1))  (where x = offsetX / chp and y = offsetX / (chp-bhp))
>     
>     (check http://www.wolframalpha.com/input/?i=sin%28arctan%28x%29+%2B+arctan%28y%29%29 vs. http://www.wolframalpha.com/input/?i=%28x%2By%29%2Fsqrt%28%28x^2%2B1%29%28y^2%2B1%29%29)
>     
>     This would mostly just require calculating a single square root instead of a sine from two nested atans inside. I'm quite sure that this term can be better optimized and calculated in less clock cycles. Let's check whether this is as fast as I hope :)
> 
> Torsten Rahn wrote:
>     Bah, the second wolfram alpha link only works properly if you copy the link (excluding the closing bracket). 
>     But still, in the end this just results in a single inverse square root which should be efficiently handled by the reciprocal square root NEON SIMD instruction.

Just tried it. When I return QPointF(0,0) in buildingOffset directly to disable any calculations in it, I get a --runtimeTrace of 35 ms for the Geometries layer in a fixed scene (same position, zoom). When I calculate the buildingOffset using either methods, the --runtimeTrace becomes 40 ms. So there's no measurable benefit at least on my i7 5600U.

Digging into it a bit further I found this little gem: http://taviso.decsystem.org/files/cpml.c
It tells me that on my system sin() takes 30 cycles, atan2 22 cycles and sqrt 6 cycles (without compiler optimizations the numbers are 34, 81, 6). So the sin,atan2,atan2 combination is 74 cycles versus 6 for sqrt. This is just the trigonometric functions however; the optimized calculation involves more elementary arithmetic computations and therefore touches more memory. Pipeline flushing and cache effects may have their effects as well. So in the end I wouldn't want to make a guess which one of the two calculations will perform better on any hardware.


- Dennis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124511/#review83626
-----------------------------------------------------------


On Aug. 5, 2015, 7:31 p.m., Dávid Kolozsvári wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124511/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 7:31 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> I changed a little bit the decoration creating method, it now uses a QList to store the decorations, so multiple decorations can be added this way. It was an idea for the street labeling, but it makes sense without that too.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/GeoPainter.h 7a757b9 
>   src/lib/marble/GeoPainter.cpp d04138c 
>   src/lib/marble/GeoPainter_p.h f0c4f9b 
>   src/lib/marble/MarbleGlobal.h cf2768f 
>   src/lib/marble/geodata/data/GeoDataFeature.h ea23cd8 
>   src/lib/marble/geodata/data/GeoDataFeature.cpp 6f330fb 
>   src/lib/marble/geodata/data/GeoDataFeature_p.h 496c356 
>   src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.h 4842809 
>   src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.cpp 4320c07 
>   src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h f469dfb 
>   src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp 81cfe9a 
>   src/lib/marble/graphicsview/GeoGraphicsItem.h 4ca4727 
>   src/lib/marble/graphicsview/GeoGraphicsItem.cpp b8fa693 
>   src/lib/marble/graphicsview/GeoGraphicsItem_p.h 01becfc 
>   src/lib/marble/layers/GeometryLayer.cpp 9eb3f50 
> 
> Diff: https://git.reviewboard.kde.org/r/124511/diff/
> 
> 
> Testing
> -------
> 
> It works on a freshly pulled version of Marble.
> 
> 
> Thanks,
> 
> Dávid Kolozsvári
> 
>

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


More information about the Marble-devel mailing list