-
Notifications
You must be signed in to change notification settings - Fork 143
Added segment snapping to the ShapeEditor #132
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,22 +77,32 @@ bool CMapView::SnappingIsOn(bool shift) | |
| // ************************************************************ | ||
| bool CMapView::HandleOnMouseMoveShapeEditor(int x, int y, long nFlags) | ||
| { | ||
|
|
||
| tkLayerSelection behavior; | ||
| _shapeEditor->get_SnapBehavior(&behavior); | ||
|
|
||
| double projX, projY; | ||
| VARIANT_BOOL snapped = FindSnapPointCore(x, y, &projX, &projY); | ||
| if (!snapped) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to earlier, FindSnapPointCore returns a VARIANT_BOOL, and you're setting it into a bool variable. Either 'snapped' should be changed to VARIANT_BOOL, OR perhaps instead, we should change FindSnapPointCore to return a bool, since it is an internal function and is more easily handled with the standard bool data type. |
||
| PixelToProjection(x, y, projX, projY); | ||
| GetEditorBase()->ClearSnapPoint(); | ||
| } | ||
| else { | ||
| double sX, sY; | ||
| ProjToPixel(projX, projY, &sX, &sY); | ||
| GetEditorBase()->SetSnapPoint(sX, sY, true); | ||
| } | ||
|
|
||
|
|
||
| if ((_dragging.Operation == DragMoveVertex || | ||
| _dragging.Operation == DragMoveShape || | ||
| _dragging.Operation == DragMovePart)) // && (nFlags & MK_LBUTTON) && _leftButtonDown | ||
| { | ||
| _dragging.Snapped = false; | ||
| tkLayerSelection behavior; | ||
| _shapeEditor->get_SnapBehavior(&behavior); | ||
| if (behavior == lsAllLayers && _dragging.Operation == DragMoveVertex) | ||
| { | ||
| double xFound, yFound; | ||
| if (this->FindSnapPointCore(x, y, &xFound, &yFound)) { | ||
| double xNew, yNew; | ||
| ProjToPixel(xFound, yFound, &xNew, &yNew); | ||
| _dragging.SetSnapped(xFound, yFound); | ||
| } | ||
| } | ||
| if (behavior == lsAllLayers && _dragging.Operation == DragMoveVertex && snapped) | ||
| _dragging.SetSnapped(projX, projY); | ||
| else | ||
| GetEditorBase()->ClearSnapPoint(); | ||
| _dragging.HasMoved = true; | ||
|
|
||
| // in case of vertex moving underlying data is changed in the process (to update displayed length); | ||
|
|
@@ -122,12 +132,8 @@ bool CMapView::HandleOnMouseMoveShapeEditor(int x, int y, long nFlags) | |
| } | ||
| else | ||
| { | ||
| tkEditorBehavior behavior; | ||
| _shapeEditor->get_EditorBehavior(&behavior); | ||
| EditorBase* base = GetEditorBase(); | ||
| bool handled = false; | ||
| double projX, projY; | ||
| this->PixelToProjection(x, y, projX, projY); | ||
|
|
||
| // highlighting of vertices | ||
| if (behavior == ebVertexEditor) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have to look further into this - 'behavior' was originally defined (within this block of code) as tkEditorBehavior, but you have redefined it (above) as tkLayerSelection, and assigned the result of get_SnapBehavior. It seems that this test (behavior == ebVertexEditor) may not be giving back the expected result. |
||
|
|
@@ -146,7 +152,7 @@ bool CMapView::HandleOnMouseMoveShapeEditor(int x, int y, long nFlags) | |
| } | ||
| else { | ||
| if (base->ClearHighlightedVertex()) { | ||
| _canUseMainBuffer = false; | ||
| _canUseMainBuffer = false; | ||
| return true; | ||
| } | ||
| } | ||
|
|
@@ -155,6 +161,7 @@ bool CMapView::HandleOnMouseMoveShapeEditor(int x, int y, long nFlags) | |
| // highlighting parts | ||
| if (behavior == ebPartEditor) //if (nFlags & MK_CONTROL) { | ||
| { | ||
|
|
||
| if (base->ClearHighlightedVertex()) | ||
| _canUseMainBuffer = false; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best not to treat a VARIANT_BOOL as a bool. Since VARIANT_TRUE is a short integer (-1), and bool is an intrinsic type, you may not always get the expected result. It would be better to say something like
if (visible == VARIANT_FALSE) continue;
This may seem a trivial point, but we've had issues in the past when mixing these types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty new to C and C++, most of the changes were made by looking at other code and copying what I need, thanks for the explanation. Can you point me to a good reference for this in particular?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a pretty good explanation, noting particularly to beware of comparing the various 'true' values:
BOOL vs. VARIANT_BOOL vs. BOOLEAN vs. bool
I should add that being a COM interface, our API needs to use VARIANT_BOOL. However, internally, it is often beneficial to use bool since it is more concise and straightforward. Only convert back to VARIANT_BOOL as necessary.