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

Overlap removal improvements. #1485

Merged
merged 5 commits into from
Jul 8, 2014
Merged

Overlap removal improvements. #1485

merged 5 commits into from
Jul 8, 2014

Conversation

frank-trampe
Copy link
Contributor

Attempt a fix for the overlap removal problems by adding functionality for consolidating duplicate and near-duplicate intersections.

This addresses #402, #473, #488, #496, and #729.

Output is extremely verbose right now. I plan to disable most of that before merging.

Due to the large structural changes in the overlap removal functionality, I would strongly encourage anybody who uses it to test the branch overlap_exploration_2 extensively and to report back to me in the next week on any problems.

…y for consolidating duplicate and near-duplicate intersections.
@frank-trampe
Copy link
Contributor Author

And, yes, I know that there are still reported errors with needed versus unneeded segments, but the end product seems to be correct following this fix. I plan to deal with the unneeded segment issue once the main issue has been conclusively resolved.

@frank-trampe
Copy link
Contributor Author

Can somebody look at the diff for me? @jtanx?

@jtanx
Copy link
Contributor

jtanx commented Jul 7, 2014

I'm on holiday ;)

Just a quick one though -

  • Are you keeping track of all those manual defines that you're doing? e.g FF_RELATIONAL_GEOM. Will these be moved into the configure script at some point?

@adrientetar
Copy link
Member

BTW, why would one want to disable relational geom?

@frank-trampe
Copy link
Contributor Author

I'm always cautious when changing a data structure, particularly since Fontforge has a few that get casted to other subset or superset data structures. I also want to check before the final merge that the relational geometry actually fixes something. (I did a lot of things.)

@frank-trampe
Copy link
Contributor Author

My testing confirms that the overlap remover fails to find matching t-values in certain cases when the relational geometry is absent, so it does indeed fix something. I'm leaving the macro conditionals for now in case we run into possibly related problems (either geometric or memory-related).

@frank-trampe
Copy link
Contributor Author

And I'm inclined to leave the output verbosity flag out of the configure script since it's only particularly useful to somebody who is working on splineoverlap.c anyway.

@frank-trampe
Copy link
Contributor Author

By the way, I did check output from rmo-we.sfd, and, even with the errors about needed and unneeded segments and exits from intersections, the final paths are complete and closed.

@tshinnic
Copy link
Contributor

See new issue #1505 : "Remove overlap broke btw commits 704e0a4 and 7535262" (sorry I couldn't test before...)

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

Successfully merging this pull request may close these issues.

4 participants