Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

Possible fix for text node truncation issue #462

Closed
wants to merge 1 commit into from
Closed

Possible fix for text node truncation issue #462

wants to merge 1 commit into from

Conversation

lxcid
Copy link
Contributor

@lxcid lxcid commented May 22, 2015

This fix is related to #261.

First, the issue seems to originate from ASTextNodeWordKerner which is set as the NSLayoutManager's delegate. If you prevent it from being the layout manager's delegate, rendering will work correctly.

Digging deeper, it seems like we are checking for the wrong glyph property.

Inside the code, we seems to be trying to search for whitespaces so that we can check its glyph property. The problem is that we are checking for NSGlyphPropertyControlCharacter.

In the documentation NSGlyphPropertyControlCharacter is described as follows:

Control character such as tab, attachment, and so on, that has associated special behavior.

It seems like there's a mismatch here. Whitespaces are usually described with NSGlyphPropertyElastic. This is checked against the documentation...

Glyphs with elastic glyph width behavior such as whitespace.

... and check against the original provided glyph property with something like this in the method...

    NSGlyphProperty glyphProperty = properties[arrayIndex];
    switch (glyphProperty) {
        case NSGlyphPropertyNull: {
            NSLog(@"NSGlyphPropertyNull");
        } break;
        case NSGlyphPropertyControlCharacter: {
            NSLog(@"NSGlyphPropertyControlCharacter");
        } break;
        case NSGlyphPropertyElastic: {
            NSLog(@"NSGlyphPropertyElastic");
        } break;
        case NSGlyphPropertyNonBaseCharacter: {
            NSLog(@"NSGlyphPropertyNonBaseCharacter");
        } break;
    }

I'm unsure under what circumstances will whitespaces not describe as NSGlyphPropertyElastic by the system, thats something to ponder too.

Hope this help!

…` instead of `NSGlyphPropertyControlCharacter`. The documentation does suggest so. This seems to fixed the truncation issue.
@lxcid
Copy link
Contributor Author

lxcid commented May 22, 2015

I could repro the bug with the following changes to Kittens.

screenshot 2015-05-23 03 17 15

@lxcid
Copy link
Contributor Author

lxcid commented May 22, 2015

Okay, after seeing the test failing. I can maybe understand the rationale of checking whitespaces for NSGlyphPropertyControlCharacter property. It help trigger the code path to allow ASTexTNodeWordKerner to provide word kerned space width.

See https://github.com/lxcid/AsyncDisplayKit/blob/bug/261-textnode-truncation-bug/AsyncDisplayKit/Details/ASTextNodeWordKerner.m#L113-L121

But this seems to cause problem for the truncation issue.

@appleguy
Copy link
Contributor

@lxcid thanks for researching this, and it was really great to meet you at the Pinterest WWDC party :).

It does look like this is a bit more complicated than this diff can address right now, so I'm going to keep the task open (and reference this diff) but close the PR. There is a possibility of integrating a slightly newer text layout system from ComponentKit that is likely to fix some corner cases like this.

@appleguy appleguy closed this Jun 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants