[morx] Failing test MORX-35: Insertion with DontAdvance

[brawer]

When handling morx insertion, the DontAdvance flag isn’t handled as expected. See
test case MORX-35. CoreText is passing this test.

[behdad]

Just wondering, why do we not see two "E"'s in the output?

[behdad]

Ie. "A" becomes "AC" while cursor still at A, then becomes "ABC" with cursor at "B". Then two "E"s should be inserted. No?

[behdad]

(Typo in MORX-36 "theww" instead of "three".)

[brawer]

First, my assumption had been the following: When the machine advances its cursor, it skips past the freshly inserted glyphs. For example, X̳AY ⟹ XA̳Y ⟹ XA̳CY ⟹ XABCY̳. So the implementation would count how many glyphs have been inserted since the last cursor change (not the last state change), and add this to the cursor position when advancing.

But, I made some other font to confirm this assumption, and it seems to be wrong. Adding more test cases.

[brawer]

I’ve now added X and Y to the test font. For XAY, CoreText gives XABCEY which surprises me.

Ie. "A" becomes "AC" while cursor still at A, then becomes "ABC" with cursor at "B". Then two "E"s should be inserted. No?

Indeed, that’s what fontkit does; but CoreText (the “Expected” row) disagrees...

[behdad]

Since CoreText's behavior is inconsistent, guess we just pick one and go ahead with it. I personally find the "skip everything inserted since last advance" the worse option just by Occam's Razor.

[brawer]

Yup, agree. Can you make a patch to HarfBuzz that implements this? Then, I’ll make HarfBuzz’s output the expected result.

[behdad]

Yup, agree. Can you make a patch to HarfBuzz that implements this? Then, I’ll make HarfBuzz’s output the expected result.

So, basically doing what fontkit is doing? "ABECE"? I agree that's what I would have expected HarfBuzz to generate. I'll fix that and let you know.

[brawer]

I’ve changed MORX-35 to expect what fontkit currently generates.

[behdad]

We should also test where things get inserted if mark is not reset and multiple insertions happen.

…

On Sun, Oct 7, 2018, 3:05 PM Sascha Brawer ***@***.*** wrote: I’ve changed MORX-35 to expect what fontkit currently generates. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1224 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIuK6B0aYF8wISvb9tGB7gOFgWV7k70ks5uilCAgaJpZM4XIbGc> .

[behdad]

Umm. That now seems to break MORX-33.

[behdad]

Maybe skipping over just-inserted glyphs is better indeed.

[behdad]

Yeah, I think I like to keep MORX-33 working, and make the non-DontAdvance case jump over the just-inserted glyphs. So, XAX will produce XABCEX, which is actually what CoreText does now. But A will also produce ABCE.