Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add node editor widget and change node tool to click-click mode #2217

Merged
merged 13 commits into from
Jul 24, 2015

Conversation

mhugent
Copy link
Contributor

@mhugent mhugent commented Jul 22, 2015

No description provided.

@NathanW2
Copy link
Member

On Wed, Jul 22, 2015 at 7:35 PM, mhugent notifications@github.com wrote:

Switch node tool to click-click mode

What is click-click mode?

@mhugent
Copy link
Contributor Author

mhugent commented Jul 22, 2015

@NathanW2: select vertex with one click and place it with a second click (rather than a drag). This allows to use the advanced digitize toolbar also for the node tool.

@NathanW2
Copy link
Member

Ok cool. Yes this is good idea. More CAD like. I haven't tested the code
is there visual feedback e.g a line showing the start to cursor location?

On Wed, Jul 22, 2015 at 7:38 PM, mhugent notifications@github.com wrote:

@NathanW2 https://github.com/NathanW2: select vertex with one click and
place it with a second click (rather than a drag). This allows to use the
advanced digitize toolbar also for the node tool.


Reply to this email directly or view it on GitHub
#2217 (comment).

if ( !m.isValid() )
if ( mMoveRubberBands.empty() )
{
QgsGeometryRubberBand* rb = new QgsGeometryRubberBand( mCanvas, mSelectedFeature->geometry()->type() );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugent this should be constGeometry() to avoid the detach. Also in 144. 424, 431

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QgsSelectedFeature (in contrast to QgsFeature) does not have a constGeometry(). But yes, for 424,431 and the two other occurences below, it is better to use constGeometry().

to avoid the detach

afaik detach is only called when a non-const operation is really called on that object.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugent - ahh, i missed that QgsSelectedFeature reference, sorry. The situation with geometry/constGeometry is a little messy, however. QgsFeature::geometry() will ALWAYS trigger a detach, and constGeometry() will never detach. We had to go that way to maintain API - since QgsFeature::geometry() returns a non const pointer, it can be used to modify the feature's geometry and we have no way of knowing if this will be done so have to detach. In 3.0 this will be cleaned up so that geometry() will always return a QgsGeometry object (not reference of pointer, since QgsGeometry is now implictly shared), and geometry modification will be done via QgsFeature::setGeometry. But in the meantime we should avoid all use of QgsFeature::geometry() (it's effectively deprecated) and rely on constGeometry/setGeometry instead. There's some notes to this effect in the docs for QgsFeature.

@nyalldawson
Copy link
Collaborator

@mhugent Also note that all the new classes/methods need doxygen documentation - that's what's tripped up the PyQgsDocCoverage test and caused Travis to reject this PR

@@ -63,10 +69,10 @@ bool QgsVectorLayerEditUtils::moveVertex( double x, double y, QgsFeatureId atFea
if ( !L->getFeatures( QgsFeatureRequest().setFilterFid( atFeatureId ).setSubsetOfAttributes( QgsAttributeList() ) ).nextFeature( f ) || !f.constGeometry() )
return false; // geometry not found

geometry = *f.constGeometry();
geometry = *f.geometry();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugent looks like this one slipped through :)

@3nids
Copy link
Member

3nids commented Jul 23, 2015

it seems the detection of the node is not working properly.
https://youtu.be/E3E8YgBeVdQ

mhugent added a commit that referenced this pull request Jul 24, 2015
Add node editor widget and change node tool to click-click mode
@mhugent mhugent merged commit 740e3ab into qgis:master Jul 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants