Modify

Opened 12 years ago

Closed 12 years ago

#3571 closed defect (fixed)

[PATCH] orthogonalizing tool do not work proper

Reported by: andre68@… Owned by: bastiK
Priority: major Milestone:
Component: Core Version: latest
Keywords: undo orthogonalize Cc:

Description

If a closed way has some extra nodes and the way is orthogonalized then undoing fails in at least the attached case. This is the only situation where I've seen this so I can't say if there are other cases.

Simply try the attached case and you will see what I mean.

Stable (r1981) does not have this problem since it refuses to orthogonalize such ways.

Attachments (4)

undo-error.osm (813 bytes) - added by andre68@… 12 years ago.
ortho-error.osm (891 bytes) - added by vsandre 12 years ago.
orth.patch (36.7 KB) - added by bastiK 12 years ago.
orth-2262.patch (37.7 KB) - added by bastiK 12 years ago.

Download all attachments as: .zip

Change History (19)

Changed 12 years ago by andre68@…

Attachment: undo-error.osm added

comment:1 Changed 12 years ago by vsandre

Priority: normalcritical
Summary: Undo fails after orthogonalizing shape with JOSM r2180orthogonalizing tool do not work proper

The problem occurs when the start/end-node of the rectangle is not in an edge but in the middle of two egdes.

I add an other case of a strange behaviour of this function. The left node is the start node of the rectangle.

  • open the file an select the way, now push q and you get a something what looks like a house. why? undo works ok.
  • now move the start/end node in the middle of the two other nodes, push q and the way will be deleted.

Changed 12 years ago by vsandre

Attachment: ortho-error.osm added

comment:2 Changed 12 years ago by bastiK

Owner: changed from team to bastiK
Priority: criticalmajor

comment:3 Changed 12 years ago by jttt

bastiK, are you working on this track? I looked into the code and it looks like in r2014 (#3205) the check for bad angles was removed but the edges with bad angles are still there and sometimes produce a way with nonsense values. At least for bug reported by vsandre, not sure about andre68.

Please let me know if you're working on a patch or if I should do that.

comment:4 Changed 12 years ago by bastiK

I'm currently at a rework of the orthogonalizing tool - it's almost done, so no need to fix this.

comment:5 Changed 12 years ago by bastiK

I suppose the rework won't make it into the next tested.

So if you think it's an easy fix - go ahead!

comment:6 Changed 12 years ago by anonymous

Ticket #3657 has been marked as a duplicate of this ticket.

Changed 12 years ago by bastiK

Attachment: orth.patch added

comment:7 Changed 12 years ago by bastiK

Summary: orthogonalizing tool do not work proper[PATCH] orthogonalizing tool do not work proper

Remake of the orthogonalize tool.

New:

  • No longer required, that the way is closed.
  • Multiple ways sharing mulitiple nodes are handled better.
  • The two (optional) reference nodes stay fixed.

As a side effect it is now possible to extend a way keeping the exact same direction. (However, it's not very straightforward.)

What it not does: Ignore angles of 45 degrees as requested by the user in #3657. There is a workaround, though: Add an auxilary way on top of the old, covering only the orthogonal parts. Orthogonalize and remove it.

comment:8 Changed 12 years ago by stoecker

Owner: changed from bastiK to andre68@…
Status: newneedinfo

Why do we need a specific menu point for undoing Ortho?

comment:9 Changed 12 years ago by stoecker

Owner: changed from andre68@… to bastiK
Status: needinfonew

comment:10 Changed 12 years ago by bastiK

What menu point? It shouldn't show up in the menu.

Sometimes the shape contains nodes that are already perfectly well placed.

This could be some imported data or something else. Then it is convenient to take back the orthonalization for certain nodes.

However, I'm not entirely happy with the way this feature is exposed to the user.

The only way to find out about it, is to read the usage dialog (requires, that the user has tried the tool on unsuited selection) or to read the (nonexistent) help in the wiki.

comment:11 Changed 12 years ago by stoecker

Well, ok I misread the patch. What do the changes in MainMenu.java do? You changed the type of the action variable and add another variable which is unused. Why?

Regarding the help --> you can add some info at http://josm.openstreetmap.de/wiki/Help/Action/Orthogonalize?action=edit

comment:12 Changed 12 years ago by bastiK

It's a new JosmAction with it's own shortcut. (Shift-Q, hope this doesn't conflict with plugins.)

I thought this is the place, where all Actions are initialized, so I put it there.

It can be placed in the constructor of OrthogonalizeAction as well, but then you have a "two in one" action.
Don't know, what's less confusing...

I see this would have needed some commenting. :)

comment:13 Changed 12 years ago by stoecker

Hmm, does not apply. Please update and comment the lines in MainMenu.

Changed 12 years ago by bastiK

Attachment: orth-2262.patch added

comment:14 Changed 12 years ago by bastiK

Ok.

comment:15 Changed 12 years ago by stoecker

Resolution: fixed
Status: newclosed

(In [2268]) fix #3571 - patch by bastiK - improve orthogonalization action

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain bastiK.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.