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

'precision' parameter doesn't work as described #44

Closed
colinmorris opened this issue Apr 17, 2018 · 9 comments
Closed

'precision' parameter doesn't work as described #44

colinmorris opened this issue Apr 17, 2018 · 9 comments

Comments

@colinmorris
Copy link
Contributor

The docstring says...

precision (float): up to which sum of all overlaps along both x and y
to iterate; may need to increase for complicated situations;

In practice, at each iteration, precision is compared with the sum of the q values returned by the repel_text* functions, and these functions seem to calculate q as the sum of the absolute x/y displacements of texts. This is definitely not the same as the sum of the overlaps.

For example, a text bbox may have an overlapping point (or text) left of its center that demands a dx of 1, and another overlapping point right of center that demands a dx of -1. Thus the net x displacement (q) of the text is 0, but the overlaps are not 0.

@Phlya
Copy link
Owner

Phlya commented Apr 17, 2018

Right, thanks, of course... This should be very easy to fix though, would you like to submit a PR?

@colinmorris
Copy link
Contributor Author

Hm, it actually seems non-trivial to me because I don't fully understand the code in the repel_* functions. Like, in repel_text here, I don't get why it's multiplying by two. If those are supposed to be the coords of the centre of the text bboxes, shouldn't it be dividing by 2?

Also digging more into repel_text_from_points, I think I found a bug (#49).

So I think I'll leave this one to you. I might take a shot at a PR for one or both of the other two earlier issues (#45 and #46), but I can't guarantee it'll be in a timely fashion, so if you want to go ahead and quickly scratch them off, feel free.

@Phlya
Copy link
Owner

Phlya commented Apr 17, 2018

Concerning multiplication by 2 - keep in mind that those are lists, not arrays, and this is simply used to create pairs of coordinates of corners, not centers of bboxes. The logic is that if neither rectangle contains any corners of the the other rectangle, these rectangles don't overlap. May not be the simplest way to test this...

@Phlya
Copy link
Owner

Phlya commented Apr 17, 2018

You actually got me thinking, and this is not strictly true in all cases - i.e. one vertical rectangle can overlap with a horizontal one with all corners being outside. But as far as all rectangles are the same height (which is typically true), this should hold... But probably this should be generalized just in case, and simply checking for left side being to the right of the right side of the other rectangle (and same for all other sides) should be very easy and work 100%.

@Phlya
Copy link
Owner

Phlya commented Apr 17, 2018

As a note to my future self, this looks interesting
https://github.com/Pithikos/python-rectangles

@Phlya
Copy link
Owner

Phlya commented Apr 17, 2018

Or, shapely, of course - maybe all calculation should migrate to using it? Then, perhaps, we can properly support non-rectangular objects?

@colinmorris
Copy link
Contributor Author

Ah, I get it, sorry I thought they were arrays when I first looked at it. That also explains the modulo!

That's a good point about the corners possibly not being contained. I'm not familiar with either of those libraries, but they look promising. Presumably you could also just use the bbox.intersection method for each pair, but maybe that would be too expensive.

@Phlya
Copy link
Owner

Phlya commented Apr 18, 2018

That's why I used this approach, but it might have been a premature optimization...

@Phlya
Copy link
Owner

Phlya commented Apr 22, 2018

I think this is fixed in the recent commit - the main issue reported here, that is, not the intersection algorithm.

@Phlya Phlya closed this as completed Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants