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

When squaring a very circular / spiky object it is deleted #10641

Open
danieldegroot2 opened this issue Dec 31, 2024 · 5 comments
Open

When squaring a very circular / spiky object it is deleted #10641

danieldegroot2 opened this issue Dec 31, 2024 · 5 comments
Labels
bug A bug - let's fix this!

Comments

@danieldegroot2
Copy link
Contributor

danieldegroot2 commented Dec 31, 2024

URL

https://www.openstreetmap.org/edit#map=18/52.527151/-7.578765

How to reproduce the issue?

https://www.openstreetmap.org/way/1018870625
Go to 'Edit' => Right click on this object => Square
Squaring is possible while this object is very circular and has many nodes
This results in the object being deleted, likely due to incorrect calculation / geometry

Do not allow the user to delete the object by squaring/circularizing, or disable squaring (not circularizing) if it is too circular and/or resolve the calculatioin for squaring such a circle.

Similarly, a node on the circle outline can be squared. However(, of course), nothing happens

In Rapid, nothing happens when squaring the circle itself. Something probably happens, but there are zero changes -object is not deleted either- and the square option stays available afterwards.
Could not find an associated issue ticket, so this might be a more recent bug than when rapid started.

Screenshot(s) or anything else?

image

image

Also happens with very spiky object (when node is moved in far enough); this can be easily reproduced

image

Also here, but only highlighted node is deleted

image

Here it is also possible to square but nothing happens, no nodes are visibly moved or deleted.

image

Which deployed environments do you see the issue in?

Released version at openstreetmap.org/edit, Development version at ideditor.netlify.app, RapiD version at mapwith.ai/rapid

What version numbers does this issue effect?

2.30.4, 2.31.0-dev, perhaps not Rapid 2.5.0

Which browsers are you seeing this problem on?

Chrome

@tyrasd tyrasd added the bug A bug - let's fix this! label Jan 7, 2025
@danieldegroot2 danieldegroot2 changed the title When squaring a circular object it is deleted When squaring a very circular / spikey object it is deleted Jan 7, 2025
@danieldegroot2 danieldegroot2 changed the title When squaring a very circular / spikey object it is deleted When squaring a very circular / spiky object it is deleted Jan 7, 2025
@ChaitanyaKadu03
Copy link
Contributor

ChaitanyaKadu03 commented Jan 12, 2025

Hello @tyrasd @danieldegroot2 @k-yle,

I’ve been looking into Issue #10641 regarding circular/spiky objects being deleted when attempting to square them. After reviewing the code in modules/actions/orthogonalize.js, specifically the actionOrthogonalize() function, I've found where adjustments might help preserve shapes without deleting them(Ex. actionDeleteNode()).

Proposal

  • Investigate the threshold logic and how the function handles “extreme” node angles or very circular geometry.
  • Ensure that if an area is too spiky or too circular, it won't trigger a full deletion.
  • Add checks or fallback behavior that prevents unexpected deletion for edge cases.

Request for Assignment
Could you please assign this issue to me? I'm prepared to:

  1. Implement a fix or adjustments within actionOrthogonalize to handle extreme angles/circular shapes correctly.
  2. Test the change against real-world scenarios like the examples in the issue description.
  3. Open a Pull Request soon to get early feedback from contributors.

If you have any specific requirements or thoughts on approach, I’d love to hear them. Otherwise, I’ll proceed with this plan and submit a PR for review shortly.

Thanks for!

@ChaitanyaKadu03
Copy link
Contributor

Hey @tyrasd @danieldegroot2 @k-yle ! Quick question on the spiky/circular squaring issue:
Should we:

  1. Force a square by using the shape’s bounding box (side = max(width, height)), or
  2. Disable “Square” entirely for extreme shapes?

Let me know which approach you prefer, and I’ll proceed accordingly. Thanks!

@tordans
Copy link
Collaborator

tordans commented Jan 12, 2025

I would prefer the bbox as a fall back than nothing happening. For the circle case the bbox will be a good result. For the spike object not that ideal but also better than not changing it.

@tyrasd
Copy link
Member

tyrasd commented Jan 14, 2025

Disable “Square” entirely for extreme shapes?

This option would be more consistent with the existing behaviour, where the squaring is not available if the shape is not "squareable"

@ChaitanyaKadu03
Copy link
Contributor

@tyrasd Sorry for the delay! I've opened PR #10693 to address this issue. Please let me know if any further improvements are needed. I'll add two test cases as soon as you approve the approach. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this!
Projects
None yet
Development

No branches or pull requests

4 participants