FlorisSteenkamp/MAT

Cannot read property 'pos' of undefined (find-3-prong-for-delta3s.js:166)

Closed this issue · 8 comments

Hi, I am facing a strange issue with calling findMats.

Works
I have a polygon with a hole, which represents the letter "B" (see 'B.svg' in gist).

If I call findMats with the svg-data of "B" everything works fine and I get the expected result.

Does not work
Two polygons, both are representing the letter "B" like above, same shape, just duplicated and moved. (see 'BB.svg' in gist)

I have a problem with calling findMats on the second polygon path, which is technically the same shape as before, just on another position.

Following error occurs:

TypeError: Cannot read property 'pos' of undefined
at Object.find3ProngForDelta3s (find-3-prong-for-delta3s.js:166)
at Object.find3Prong (find-3-prong.js:44)
at findAndAdd3Prong (find-and-add-3-prongs.js:113)
at findAndAdd3Prongs (find-and-add-3-prongs.js:96)
at Object.findAndAddAll3Prongs (find-and-add-3-prongs.js:23)
at findPartialMat (find-mats.js:88)
at Object.findMats (find-mats.js:45)

Hi sermunar.

Thanks for the bug report.

Yes, it certainly is a bug, but I couldn't reproduce it since I have made many improvements in the meantime, especially regarding robustness (via adaptive infinite precision floating point arithmetic) that is not yet published.

I will publish an update during the upcoming week that should solve your problem.

Will keep you posted.

(The first image below is where I kept the svgs seperate and in the second one I combined them
so there is only one path element.)

bb1

bb2

Hi @FlorisSteenkamp,

thanks for your reply.

I tried to combine my "BB" paths to check if it works, but it still leads to the same error.

Just in case, I tried two different shapes and combined them, which works like a charm.

GB

I am excited to check your update, thanks.

I tried some more tests with different shapes, first seperated and calling "findMats" for each path element and then the two shapes combined as single path element.

Following shapes are leading to a strange behaviour, maybe its a different bug, but I think it relates to this issue as well and maybe it helps to reproduce the core issue.

Shape "A" and "B" seperated (svg)
findMats for shape "A" works fine, but in findMats result for shape "B" I get unexpected curves at the end.

A and B

Shape "A" and "B" combined (svg)
findMats for the combined shape "AB" does not work and throws the same (reported) error.

A and B combined

Yes, the bugs may be related.

I am currently busy with publishing the dependent packages and then finally publishing the updated library. It should solve your issues.

Hi sermunar,

Ok, I have made the fixes and ran it through about 3000 font glyphs without issue.

Your svgs are as below:

ab1

ab2

Please check and see if your issues are resolved before I close.

Also, not sure if you are aware of it, but you can run the MATs through a Scale Axis Transform (SAT) to reduce the noise inherent in a MAT, e.g.

let mats = findMats(loops);
let S = 1.5 // or 1.1 or 5 or whatever
let sats = mats.map(mat => toScaleAxis(mat, S));

And thanks for the bug report!

Hey @FlorisSteenkamp,

thanks for your fast responses everytime and your update 1.0.1!
Yes, I know the toScaleAxis function and I am using it as well, I just didnt mention it on this bug report to keep it clean.

Sadly I cant run findMats anymore after 1.0.1. Not a single shape is running successfully.
Following error is thrown

TypeError: flo_gauss_quadrature_1.default is not a function
at Object.getLoopArea (get-loop-area.js:22)
at find-mats.js:70
at Array.filter ()
at Object.findMats (find-mats.js:49)

I realized the "flo_gauss_quadrature" is missing in the updated package.json, guess that leads to this issue.

And I have a compile error because of the changed definitions of CP-Node. Looks like the property "matCurveToNextVertex" is missing now.

image

I copied this code-part for traversing the edges from your README. Could you please suggest to me how can I get the bezier data now.

Thanks a lot!

Hi Serkan

Sorry, it seems I ran the tests and then published before building first 😯.
It is fixed now.

Regarding your question about matCurveToNextVertex. This has been changed to simplify the code and remove unnecessary state. It has been superseded by getCurveToNext:

//let bezier = cpNode.matCurveToNextVertex;  // <- old
let bezier = getCurveToNext(cpNode);         // <- new 

However, I put matCurveToNextVertex back to make the code backwards compatible and added a deprecation note.

I am currently updating the documentation to reflect the above and other changes; most notably, previously one called findMats(bezierLoops, x) where x is a number of additional points per bezier curve to use to improve accuracy; however, this has now been replaced by two seperate tolerance values for better adaptive behaviour.

But of course the parameters can also be left out and you can just call findMats(bezierLoops) so the default values are used.

There is also a new function that may come in useful called simplifyMat that will remove about 70% of MAT points whilst keeping the MAT to within a specifiable tolerance, e.g. call:

simplifyMat(toScaleAxis(mat, scale))

instead of

toScaleAxis(mat, scale)

I'm excited to see the results!

Hey @FlorisSteenkamp,

"Houston we have no problem" :)

Everything works like a charm now!

image

image

I changed it to "getCurveToNext" as suggested and the simplification is a big improvement as well, thanks a lot!