----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123259/#review78520 -----------------------------------------------------------
src/plugins/render/annotate/GroundOverlayFrame.h (line 72) <https://git.reviewboard.kde.org/r/123259/#comment53710> This naming got me irritated only now :-) Of course it was there before (with movedPoint*). But maybe we should rename the "Point" to "Handle" (hoveredPoint would then become e.g. hoveredHandle. Point otherwise makes me think of a QPoint (which it's not). src/plugins/render/annotate/GroundOverlayFrame.h (line 73) <https://git.reviewboard.kde.org/r/123259/#comment53709> I guess this should be m_editStatus to clearly convey what it's about. src/plugins/render/annotate/GroundOverlayFrame.h (line 74) <https://git.reviewboard.kde.org/r/123259/#comment53707> This variable name doesn't immediately convey to me what it's about. I guess it's not about having focus, is it? :-) src/plugins/render/annotate/GroundOverlayFrame.h (line 75) <https://git.reviewboard.kde.org/r/123259/#comment53708> "last" is usually used as a prefix for a last object in a row (opposed to first). In this case I'd rather use "previous" as a prefix. src/plugins/render/annotate/GroundOverlayFrame.cpp (line 43) <https://git.reviewboard.kde.org/r/123259/#comment53700> Could you move the arrow icons into its own subdirectory /bitmaps/editarrows ? :-) src/plugins/render/annotate/GroundOverlayFrame.cpp (line 161) <https://git.reviewboard.kde.org/r/123259/#comment53701> we prefer preincrements (++i) where possible. src/plugins/render/annotate/GroundOverlayFrame.cpp (line 163) <https://git.reviewboard.kde.org/r/123259/#comment53706> This calculation doesn't work too well for the pure positioning of the handle icons: - the edge icons appear off from the usually bent line - as soon as you drag the handles they sometimes start to jump We need something better here: Spherical Interpolation to the rescue! :-) - For the corners it would be appropriate to use the ring.at() positions - For the edges you could use GeoDataCoordinates interpolate( const GeoDataCoordinates &target, double t ) i.e. ring.at().interpolate( ring.at(), 0.5 ) This should yield better results. As a result we probably don't need the NorthernRegion, SouthernRegion, EasternRegion and WesternRegion code anymore, do we (if so, removing it might speed up things again I guess). src/plugins/render/annotate/GroundOverlayFrame.cpp (line 172) <https://git.reviewboard.kde.org/r/123259/#comment53702> Please apply transformed( trans, Qt::SmoothTransformation) ) to avoid jagginess. src/plugins/render/annotate/GroundOverlayFrame.cpp (line 174) <https://git.reviewboard.kde.org/r/123259/#comment53703> Please apply transformed( trans, Qt::SmoothTransformation) ) to avoid jagginess. src/plugins/render/annotate/GroundOverlayFrame.cpp (line 179) <https://git.reviewboard.kde.org/r/123259/#comment53704> Please apply transformed( trans, Qt::SmoothTransformation) ) to avoid jagginess. src/plugins/render/annotate/GroundOverlayFrame.cpp (line 183) <https://git.reviewboard.kde.org/r/123259/#comment53705> Please apply transformed( trans, Qt::SmoothTransformation) ) to avoid jagginess. - Torsten Rahn On April 5, 2015, 5:03 vorm., Dávid Kolozsvári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123259/ > ----------------------------------------------------------- > > (Updated April 5, 2015, 5:03 vorm.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > Until now it wasn't possible to rotate the GroundOverlayFrame manually, only > from it's properties menu, setting the angle in a spinbox. > This patch introduces a user-friendly manual rotation in edit mode, as well > as new icons(thanks to Torsten Rahn) which are shown as handles around it's > edges and corners. > Also, a bug was fixed in this patch: the GroundOverlayFrame wasn't able to > capture the mouse events that occured outside of it's regions while a > controlpoint(handle) was grabbed. > > > Diffs > ----- > > data/bitmaps/arrow-diagonal-topleft-active.png PRE-CREATION > data/bitmaps/arrow-diagonal-topleft.png PRE-CREATION > data/bitmaps/arrow-diagonal-topright-active.png PRE-CREATION > data/bitmaps/arrow-diagonal-topright.png PRE-CREATION > data/bitmaps/arrow-horizontal-active.png PRE-CREATION > data/bitmaps/arrow-horizontal.png PRE-CREATION > data/bitmaps/arrow-rotation-bottomleft-active.png PRE-CREATION > data/bitmaps/arrow-rotation-bottomleft.png PRE-CREATION > data/bitmaps/arrow-rotation-bottomright-active.png PRE-CREATION > data/bitmaps/arrow-rotation-bottomright.png PRE-CREATION > data/bitmaps/arrow-rotation-topleft-active.png PRE-CREATION > data/bitmaps/arrow-rotation-topleft.png PRE-CREATION > data/bitmaps/arrow-rotation-topright-active.png PRE-CREATION > data/bitmaps/arrow-rotation-topright.png PRE-CREATION > data/bitmaps/arrow-vertical-active.png PRE-CREATION > data/bitmaps/arrow-vertical.png PRE-CREATION > src/plugins/render/annotate/AnnotatePlugin.cpp 45e8746 > src/plugins/render/annotate/GroundOverlayFrame.h 194ade5 > src/plugins/render/annotate/GroundOverlayFrame.cpp c27eabd > src/plugins/render/annotate/SceneGraphicsItem.h c59340e > > Diff: https://git.reviewboard.kde.org/r/123259/diff/ > > > Testing > ------- > > Testing was done by me and it works fine. > > > Thanks, > > Dávid Kolozsvári > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
