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

Combining paths in the glyph φ doesn't work. #187

Open
matj1 opened this issue Jan 29, 2024 · 11 comments
Open

Combining paths in the glyph φ doesn't work. #187

matj1 opened this issue Jan 29, 2024 · 11 comments
Labels
bug Something isn't working user reported Higher Priority
Milestone

Comments

@matj1
Copy link

matj1 commented Jan 29, 2024

Describe the bug & what the expected behavior should be:

I try to combine two shapes (the ring and the stem) into one in the glyph φ, but it says “Combine shapes error: No overlapping shapes.” instead. I expect that it would combine them into one path as the outline of the letter and two smaller paths as the holes.

Steps to reproduce the bug:

  1. Load my font project without the “.json” in the name.
  2. Select the glyph for φ (or θ or maybe something else where it happens)
  3. Click the button “Combine all paths” shown as a square overlapping a circle.

Other notes & screenshots:

This is how it looks:

How it looks

@matj1 matj1 added the bug Something isn't working label Jan 29, 2024
@matj1
Copy link
Author

matj1 commented Jan 29, 2024

I tried it with a new glyph in a new project, and it was inconsistent; I mean that the combining happened, but it deleted the part of the stem in the hole, which too is incorrect:

Snímek obrazovky (6)

And the result:

Snímek obrazovky (7)

There should be two holes.

@mattlag mattlag added the user reported Higher Priority label Jan 29, 2024
@mattlag
Copy link
Member

mattlag commented Jan 29, 2024

This is a good bug - thank you for submitting it. By "good bug" I mean it may be very tricky to fix 🙂.

If I remember correctly, the combine algorithm fist sorts shapes by winding, and combines them. Actually, I think the algorithm assumes shapes with different winding will never need to be combined. In order for this combine to be successful, the black shapes will have to influence the transparent shapes.

I will think about this - but, there may be no short-term fix. For the short-term, I would recommend exporting the SVG for this glyph and working with it in another graphics program (like Illustrator or Inkscape) then re-importing the merged shapes.

@mattlag mattlag added this to the v2.beyond milestone Jan 29, 2024
@matj1
Copy link
Author

matj1 commented Jan 29, 2024

One other issue with combining is in the cases with just one shape intersecting itself. I expect that Combine all paths would transform it to a path for the outline and a path for the hole (and maybe other paths for other holes). But the button is not available with just one path. An example of that is β in my font.

@mattlag
Copy link
Member

mattlag commented Jan 29, 2024

Interesting - this is a case I had not considered. I will think about that as well.

As a work-around, you could draw a shape that 'covers' the self-overlap. Then you have two shapes to select, which enables the combine shapes action - and the 'covering' shape will eliminate the self-overlap region:

Before:
image

After:
image

@matj1
Copy link
Author

matj1 commented Jan 29, 2024

I mean rather something like this:

My self-overlapping shape

There is also an other shape covering the intersection. The result of combining is very strange. I displayed layers especially to see which shapes are there after the combining.

My self-overlapping shape after combining.

@matj1
Copy link
Author

matj1 commented Jan 29, 2024

Also, I tried something similar to what you showed, and it said “Combining shapes error: No overlapping shapes found with the same winding.”.

“Combining shapes error: No overlapping shapes found with the same winding.”

@matj1
Copy link
Author

matj1 commented Jan 29, 2024

Also note that I can't make a shape covering the whole self-overlap and not adding anything to the shape. That is because, in my cases, the intersections are on curves, and calculating the exact coordinates of the intersections would be very difficult.

@mattlag
Copy link
Member

mattlag commented Jan 30, 2024

I will try to think about these two issues, and how to update the Combine Shapes algorithm. You have outlined good test cases, so hopefully I can make changes that meet these two cases.

@mattlag
Copy link
Member

mattlag commented Feb 7, 2024

Hello!

So... I went off the deep end fixing this bug. I basically gave up on our internal Bezier Curve overlap algorithm, and just stuck Paper.js into the project to do all our Shape Combine actions. v2.1.4 just shipped, and not only does it have better "Unite" multi-shape action, but there is also now Subtract, Divide, Exclude, and Union.

That's the fix. Unfortunately, I was thinking more about the PHI problem. Two circles and a vertical rectangle will never boolean combine in a way that you want. It's impossible. If you try this same thing in Illustrator, you'll get the same result.

The process for PHI will be something like:

  1. Make two copies of your vertical rectangle
  2. Select one of the vertical rectangles, make sure it's above the small interior circle, and use the rectangle to Subtract from the small interior circle. This will give you two "D" shapes.
  3. With your second vertical rectangle and your large circle, select both and Unite them

There is no way to select all three shapes, do some combine action, and get the result you want. It has to be split into two separate combine actions.

Thank you for starting me down this trail. The result isn't perfect, but we have new Combine features that are more dependable than before.

@mattlag mattlag closed this as completed Feb 7, 2024
@mattlag mattlag modified the milestones: v2.beyond, v2.1 Feb 7, 2024
@matj1
Copy link
Author

matj1 commented Feb 8, 2024

Unfortunately, I was thinking more about the PHI problem. Two circles and a vertical rectangle will never boolean combine in a way that you want. It's impossible. If you try this same thing in Illustrator, you'll get the same result.

I tried to combine it in Inkscape and in FontForge, and it behaved how I wanted in these cases.

@mattlag
Copy link
Member

mattlag commented Feb 8, 2024

You are correct! Although I have to be careful, Illustrator and Inkscape require an additional step of creating a "Compound Shape" in order to create the cut-out. There is no such concept in fonts.

But, FontForge does offer a "Remove Overlaps" action, which works as intended. It must somehow take into consideration the winding of non-intersecting shapes. Currently as I have it implemented, I just use the out-of-the-box Paper.js "Unite" function.

I'm going to re-open this, because there will have to be some additional work to move from "Unite" to "Remove Overlaps".

@mattlag mattlag reopened this Feb 8, 2024
@mattlag mattlag modified the milestones: v2.1, v2.beyond Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working user reported Higher Priority
Projects
None yet
Development

No branches or pull requests

2 participants