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

Path directions for text are unpredictable #512

Open
bgmort opened this issue Sep 17, 2021 · 8 comments
Open

Path directions for text are unpredictable #512

bgmort opened this issue Sep 17, 2021 · 8 comments
Labels

Comments

@bgmort
Copy link

bgmort commented Sep 17, 2021

I'm using maker.js to generate designs for laser cutting. I've noticed an issue with way text gets rendered to SVG, and it appears to me to be a bug, though I'm hoping there's a workaround, and it might be that there is something that I'm doing wrong.

I'm loading fonts using opentype.js and rendering the text as curves, and exporting to SVG. I'm finding that the directions of paths are not preserved from the glyphs and are unpredictable, resulting in inner paths that are not always the correct direction for the non-zero fill mode, regardless of which fillRule option is used when exporting to SVG. This is a problem because some laser software (Glowforge) doesn't honor fill rules, and always treats curves as non-zero while engraving (though it honors the fill rule when previewing in the interface, so the issue is not seen until the engrave goes wrong).

When opentype parses the font, the directionality of each curve is preserved. But after creating a makerjs.model.Text model from the font, the path directions are inconsistent. This can even be seen in the examples in the playground that use text.

image

I believe this is why directions are unpredictable when the models get exported to SVG paths. So I have two questions:

  1. How difficult would it be to fix this in the text model so path directions are maintained when the model is created?
  2. Is there a document best workaround with the current version to fix path directions? I started writing my own fixChains function using makerjs.model.findChains(model, { contain: { alternateDirection: true } }), but it's not functional yet, and it seems like this is something that should already be somewhere in the code base already.
@danmarshall
Copy link
Contributor

I think it may be here already:
https://github.com/microsoft/maker.js/blob/master/packages/maker.js/src/core/svg.ts#L280

So when you call exporter.toSVG({options}), set options.fillRule to 'nonzero'.

@bgmort
Copy link
Author

bgmort commented Sep 17, 2021

@danmarshall I tried that already, and still got unpredictable results.

I did manage to write something using makerjs.model.findChains(model, { contain: { alternateDirection: true } }) that manages to correct the path directions, and is fairly performant. But upon exporting to SVG, it loses those path directions again. I would have thought that with the paths corrected, using either fill rule would work for exporting. Suprisingly, exporting with the even-odd rule, some of the paths get reversed again. I guess it's probably having to re-do the work of detecting chains again, but doing it without trying to alternate directions this time.

If I fix the paths myself, then export using options.fillRule = 'nonzero', it does appear to come out correctly (I need to test this some more), though it takes quite a long time. It would be nice like to be able to just export, and have the paths retain the direction they were originally drawn in. But the more I dig into how this works, the more I understand why it's trickier than it would seem.

@danmarshall
Copy link
Contributor

Let's have a quick recap of some fundamentals. See https://maker.js.org/docs/advanced-drawing/#Bezier%20curves
Fonts are imported as Bezier curves, which are then converted to arcs. That is the final representation within any Model object. Arc Path objects are always counter-clockwise (mathematic polar notation). Line Paths are start to end point. So in a Model these directions will never change.

When you find a chain, this is where those directions may be reversed, according to how they're "found".
https://maker.js.org/docs/working-with-chains/#Chain%20links
This is the only time that directionality may be coerced, in a chain link.

Perhaps you can share a code sample which shows a working case and an error case?

@bgmort
Copy link
Author

bgmort commented Sep 18, 2021

If a model never stores directions for arcs, that explains why my attempted fix didn't work. After finding chains, I turn them back into a model, and was thinking they would retain the order and direction of the chains.

https://codesandbox.io/s/charming-sunset-kuz0b

Have a look at this. It's bare bones, but illustrates what's happening in my project. Notice that it's somewhat unpredictable – fixChains changes but doesn't necessarily improve the output, and it's different for different fonts. fillRule: 'nonzero' in the export options gets most letters right but misses a few, and seems a bit random. It even sometimes seems nondeterministic, where I'll get a different result for the same text, after changing it from something else.

@danmarshall
Copy link
Contributor

I'm pretty sure this is a bug. I was able to get a good repro using just the 8 character. Then I discovered that the largest chain was determined to be clockwise, which it is not:

var makerjs = require('makerjs');

function demo(closed) {

  var points = [
    [
        33.83999999349655,
        36.359999976331224
    ],
    [
        32.51553859349655,
        43.102752976331224
    ],
    [
        25.038070255410236,
        50.60347114926677
    ],
    [
        18.428084798496567,
        52.05466148457484
    ],
    [
        18.21600001654018,
        52.05599989895457
    ],
    [
        9.754690685560373,
        49.7137779108071
    ],
    [
        4.021964985560373,
        43.064295510807106
    ],
    [
        2.7359999846781022,
        36.35999995768579
    ],
    [
        4.71186345278274,
        28.974491642269125
    ],
    [
        7.631999971368683,
        25.775999988380065
    ],
    [
        4.260708644874638,
        21.973511236554454
    ],
    [
        2.883804963037365,
        17.564678597550007
    ],
    [
        2.760553652401171,
        16.18957706607498
    ],
    [
        2.7360000562149267,
        15.191999970025062
    ],
    [
        3.7862078136895114,
        8.815864343286332
    ],
    [
        8.85892297105782,
        2.178653768003498
    ],
    [
        16.76861867105782,
        -0.508671931996501
    ],
    [
        17.20004890899928,
        -0.5393299536777469
    ],
    [
        18.432000016923727,
        -0.5760000708312347
    ],
    [
        27.75452233040596,
        2.083482472792573
    ],
    [
        32.34145428125059,
        7.508505474096058
    ],
    [
        33.20965178441748,
        9.841507879618216
    ],
    [
        33.91199992073882,
        15.191999962241002
    ],
    [
        33.03895118382448,
        20.46218918285239
    ],
    [
        29.264762818105154,
        25.533091263621593
    ],
    [
        28.94400003795979,
        25.776000026405626
    ],
    [
        32.55973478668082,
        30.19718342671717
    ],
    [
        33.829996186680816,
        35.76556162671717
    ]
];
  
  console.log(makerjs.measure.isPointArrayClockwise(points));
  
  this.models = {
    example: new makerjs.models.ConnectTheDots(closed, points)
  };

}

module.exports = demo;

So this is the next place for me to look.

@bgmort
Copy link
Author

bgmort commented Sep 21, 2021

Here's a straightforward function that runs in O(n) instead of O(n log n) and appears to avoid edge cases. I cloned the repo and built it locally, but the local build didn't include my changes, and I haven't yet figured out why. But it runs nicely in isolation. Should I write up some tests and submit a PR?

/**
 * Check for array of points being clockwise or not.
 *
 * @param points The array of points to test.
 * @param out_result Optional output object, if provided, will be populated with key points and area results
 * @returns Boolean true if points flow clockwise.
 */
export function isPointArrayClockwise(points: IPoint[], out_result?: { keyPoints?: IPoint[], area?: number }) {
    let area = 0;

    for (let i = 0; i < points.length; i++) {
        let point = points[i];
        let x1 = point[0];
        let y1 = point[1];
        let nextPoint = points[(i + 1) % points.length];
        let x2 = nextPoint[0];
        let y2 = nextPoint[1];

        area += (x1 * y2 - x2 * y1);
    }

    if (out_result) {
        out_result.keyPoints = points;
        out_result.area = area / 2;
    }

    return area < 0;
}

@danmarshall
Copy link
Contributor

One thing you can also do is just say makerjs.measure.isPointArrayClockwise = isPointArrayClockwise to try it out in your codebase. Can you try and see if that fixes your issue?

@bgmort
Copy link
Author

bgmort commented Sep 21, 2021

Monkey patching that function doesn't work since isChainClockwise references it locally, but monkey patching isChainClockwise with a call to the new isPointArrayClockwise does work.

That does look like an improvement! At first it appeared to solve my problems, but there are still a few instances where it doesn't. See the Quicksand font in the codesandbox. I think we've found one bug, but there's still another one out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants