facebook / Rapid

The OpenStreetMap editor driven by open data, AI, and supercharged features
https://rapideditor.org
ISC License
515 stars 91 forks source link

Squaring building changes it but doesn't square it #722

Closed mvexel closed 1 year ago

mvexel commented 1 year ago

Description

Consider way https://www.openstreetmap.org/way/1122719902. Select the building, press Q to square, it changes the geometry but does not make it square. Upload the change, reload the area, try again, it will change the geometry once again but it's not any more (or less) square than it was before

Version

2.00-alpha.3.3

What browser are you seeing the problem on? What version are you running?

Firefox v107.0

Steps to reproduce

Consider way https://www.openstreetmap.org/way/1122719902. Select the building, press Q to square, it changes the geometry but does not make it square. Upload the change, reload the area, try again, it will change the geometry once again but it's not any more (or less) square than it was before

The browser URL at the time you encountered the bug

https://mapwith.ai/rapid-v2-alpha3#background=Bing&datasets=fbRoads,msBuildings&disable_features=boundaries&id=w1122719902&map=21.63/40.40828/-111.76312&photo=mapillary/288200786362789&photo_overlay=mapillary,kartaview,streetside

mvexel commented 1 year ago

This seems fixed now?

bhousel commented 1 year ago

My guess is that this vertex is just barely too angular that it gets skipped.
The orthogonalize command skips any that are > 13° off from 90° or 0°.

https://github.com/facebook/RapiD/blob/9c9cd5540e582f587b033c77d16160a3f6e64ed2/modules/actions/orthogonalize.js#L12

Screenshot 2023-01-16 at 5 13 48 PM
mvexel commented 1 year ago

I was wrong and this still behaves unpredictably.

https://user-images.githubusercontent.com/120452/213782087-b93b50de-1cb0-46d2-8a8b-0ee2a93ccea7.mov

(I pressed Q every time I (re) selected the building, a few times nothing happened at all and Rapid tells me that the way cannot be any more square)

bhousel commented 1 year ago

I was wrong and this still behaves unpredictably.

This looks correct to me? It's not going to try to square an angle that is too spiky - the algorithm assumes anything > 13° off is mapped that way on purpose.

mvexel commented 1 year ago

I just compared it to iD behavior and you're right, it does what it's supposed to. I think I was expecting some unspecified magic.

bhousel commented 1 year ago

I just compared it to iD behavior and you're right, it does what it's supposed to. I think I was expecting some unspecified magic.

cool - yeah I do understand that it might not work how users expect so I'm definitely open to changing it at some point..