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

Objects sometimes translate when flipping #1495

Closed
ericwa opened this issue Oct 20, 2016 · 5 comments
Closed

Objects sometimes translate when flipping #1495

ericwa opened this issue Oct 20, 2016 · 5 comments
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Enhancement New features

Comments

@ericwa
Copy link
Collaborator

ericwa commented Oct 20, 2016

  1. Start with the default map, and add this brush:
    screen shot 2016-10-20 at 4 10 49 pm
  2. Select both brushes and press Cmd+F to flip them. This is the result:
    screen shot 2016-10-20 at 4 11 18 pm
    The brushes have been translated a bit on the X axis. Is TB grid-aligning the center point of the entire selection before flipping it?
    It might be better if things didn't shift like this when flipping.
@kduske
Copy link
Collaborator

kduske commented Oct 21, 2016

It is in fact grid aligning. Not sure if not doing that is better or worse. I did it to avoid floating point problems. I could however just snap the point to int to achieve the same.

@kduske kduske added this to the TrenchBroom 2.0.0 milestone Oct 21, 2016
@kduske kduske added Type:Enhancement New features Platform:All Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. labels Oct 21, 2016
@ericwa
Copy link
Collaborator Author

ericwa commented Oct 21, 2016

Hm.. I think the safest thing would be not rounding at all here. Any floating point errors will be tiny since it's all double precision.

Also, rounding the center point to integer before flipping could introduce 1 unit offsets, in the same way as my example does, which would be difficult to see.

@kduske
Copy link
Collaborator

kduske commented Oct 21, 2016

Well it might turn integer plane plane points into float, which I'd prefer to avoid.

@ericwa
Copy link
Collaborator Author

ericwa commented Oct 23, 2016

How about rounding the bounding box min/max points to integers before taking the center point?

This will give a center point that is either integer, or an integer +/- 0.5. Those should be safe floating-point values; 0.5 is not a repeating fraction in binary like 0.1 is.

I've got a PR for this I'm just polishing up.

@ericwa
Copy link
Collaborator Author

ericwa commented Dec 7, 2016

Fixed now that #1500 is merged

@ericwa ericwa closed this as completed Dec 7, 2016
ericwa added a commit that referenced this issue Nov 27, 2017
Not snapping the center point gives undesirable behaviour when
part of the selection bbox is off-grid, and the selection contains
on-grid vertices (which will be moved off grid).

This reverts the fix for #1495
ericwa added a commit that referenced this issue Nov 27, 2017
kduske pushed a commit that referenced this issue Nov 27, 2017
* 1889: Restore grid snapping of flip center.

Not snapping the center point gives undesirable behaviour when
part of the selection bbox is off-grid, and the selection contains
on-grid vertices (which will be moved off grid).

This reverts the fix for #1495

* 1889: snap flip center to 1/2 the grid size instead of the actual grid

This will avoid reintroducing #1495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Enhancement New features
Projects
None yet
Development

No branches or pull requests

2 participants