> On Aug. 9, 2015, 8:49 nachm., 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): > > > >  > > - 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: > > > >  > > > > 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?
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 ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124511/#review83626 ----------------------------------------------------------- On Aug. 5, 2015, 7:31 nachm., 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 nachm.) > > > 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 > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
