Skip to content

Commit

Permalink
CAD dock: fix a typo -> enable new snapping option
Browse files Browse the repository at this point in the history 
When X or Y is locked and user has mouse on top of a segment, CAD dock widget
will snap at the intersection of the segment and the axis.
  • Loading branch information
@wonder-sk @3nids
wonder-sk authored and 3nids committed Sep 12, 2017
1 parent 407baf0 commit 9c4d1da
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/gui/qgsadvanceddigitizingdockwidget.cpp
View file
Expand Up @@ -578,7 +578,7 @@ bool QgsAdvancedDigitizingDockWidget::applyConstraints( QgsMapMouseEvent* e )
{
point.setX( previousPt.x() + mXConstraint->value() );
}
if ( !mSnappedSegment.isEmpty() && !mXConstraint->isLocked() )
if ( !mSnappedSegment.isEmpty() && !mYConstraint->isLocked() )
{
// intersect with snapped segment line at X ccordinate
const double dx = mSnappedSegment.at( 1 ).x() - mSnappedSegment.at( 0 ).x();
Expand All @@ -605,7 +605,7 @@ bool QgsAdvancedDigitizingDockWidget::applyConstraints( QgsMapMouseEvent* e )
{
point.setY( previousPt.y() + mYConstraint->value() );
}
if ( !mSnappedSegment.isEmpty() && !mYConstraint->isLocked() )
if ( !mSnappedSegment.isEmpty() && !mXConstraint->isLocked() )
{
// intersect with snapped segment line at Y ccordinate
const double dy = mSnappedSegment.at( 1 ).y() - mSnappedSegment.at( 0 ).y();
Expand Down

8 comments on commit 9c4d1da

@luipir
Copy link
Contributor

@luipir luipir commented on 9c4d1da Sep 12, 2017

Choose a reason for hiding this comment

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

2.18 and 2.14 travis is broken an this PR has been merged against a failig travis [1]... should we merge every panding PR or we want to fix the origin of the travis fail?
@wonder-sk @3nids @m-kuhn any opinion?

[1] https://travis-ci.org/qgis/QGIS/builds/274468649

@3nids
Copy link
Member

@3nids 3nids commented on 9c4d1da Sep 12, 2017 •
edited

Choose a reason for hiding this comment

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

@luipir original commit has no failing test (it's only the docker build time out, Travis is green)
failing on 2.18 branch began with PR  #5167 ( ping @alexbruy )

@alexbruy
Copy link
Contributor

Choose a reason for hiding this comment

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

Travis failure in #5167 unrelated to pull-request changes

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 9c4d1da Sep 12, 2017

Choose a reason for hiding this comment

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

@alexbruy are you sure that it's unrelated?
It looks like ever since it's failing with the same error while before it was unstable but that seems to have been for real reasons (build warnings) and unstable tests (QgsActionManager which should be disabled).

If it's not related to your changes, it could possibly also be an update of the environment (travis or package repository).

@luipir if a pull request looks fine and really only has the same test failing as the main branch, I think it can be merged. Of course fixing the CI is preferable but it requires someone to take the time to look into it. This is done completely and 100% on a volunteer basis up to now.

@alexbruy
Copy link
Contributor

Choose a reason for hiding this comment

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

@m-kuhn yes. It did not touched vector (contains ogrLayerName function) and ToolsTest (containts failing test for ogrLayerName function) files.

@luipir
Copy link
Contributor

@luipir luipir commented on 9c4d1da Sep 12, 2017

Choose a reason for hiding this comment

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

@m-kuhn I know, My complain is that if we leave travis in constant failing state we are creating a fast track among committers and no-committers. I can ask to boundless core committers to merge, but hes' not save for a nect LTR release and, most important, is not fair!
QGIS is financing bugfix for 2.18 and 3.0, would make sense to finance travis stability. IMHO the error could be more related with " it could possibly also be an update of the environment (travis or package repository)"

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 9c4d1da Sep 12, 2017

Choose a reason for hiding this comment

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

The failing test points out behavior of GDAL when opening files in a directory which are referenced by a layerid (integer). Apparently right now the two files in the directory are resolved in a different order than originally (a.csv -> layerid=1, b.csv -> layerid=0), I'm not sure about the rules of resolving file names to indexes or if there's any promise by GDAL to produce stable results here. This probably depends on system specific things like the filesystem locale.
@strk you introduced this test originally, any insights from your side? Do you think it's ok to disable it?

@luipir
Copy link
Contributor

@luipir luipir commented on 9c4d1da Sep 12, 2017

Choose a reason for hiding this comment

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

probably @alexbruy fixed the failing teswt with #5176

Please sign in to comment.