Click support breaks with word wrap

[lostfictions]

Trying this on Unity 5.1.2f1.

Steps to repro:

• Open up the Demo 9 scene
• Set Word Wrap on the text element to a value small enough to cause wrapping (I tried 7)
• Hit play and try clicking on a few glyphs. The debug messages report wrong values, even from the very first (unwrapped) word.

Let me know if you need me to fork and set up a ready-made repro case! It seems pretty easy-to-reproduce.

[lostfictions]

Finally had time to dig into this a bit more. Looks like the generated glyph bounds assume left-aligned text when word wrap is enabled, so center- or right-aligned text will cause GetGlyphIndexAtWorldPoint() to report bogus values. (Turning "Draw Glyph Bounds" on makes this a lot more obvious.)

EDIT: no, wait, there's something weirder going on -- even when text is left-aligned click support under word wrap reports weird values. @gnustoboz, @Chman, any idea on this?

[gnustoboz]

Hey @lostfictions, sorry, haven't looked at this project lately and didn't see this until you @ mentioned me, so thanks. I'll try to take a look at this soon, hopefully this weekend, and let you know what I find. It was originally developed under Unity 4.x so it wouldn't surprise me if something isn't behaving itself now.

[lostfictions]

Thanks! Took another look at it and it seems like there's several problems:

• Something near AddPlaceholderGlyphBounds() is causing off-by-one errors when it's called for whitespace and word wrap is enabled, causing clicks on glyphs after whitespace to have the wrong index. (It's also causing the last few glyphs to not get bounding boxes at all.) should be fixed by Remove spurious AddPlaceholderGlyphBounds call #12.
• Having text right- or center-aligned isn't taken into account for bounding box generation when word wrap is enabled -- bounding boxes are placed where they'd be if the text was left-aligned anyway.
• This is probably unrelated/should be a separate issue, but bounding box generation seems to be a bit off for certain kerning pairs. The box for 'j' in 'uj' only covers about half the glyph, for example. This was making some of the weird behaviour tricky to diagnose until I figured it out... should be fixed by Fix undersized glyph bounding boxes for click support #13.

[lostfictions]

Okay, making progress on this. The main thing that's broken now is using word wrap in tandem with center- or right-alignment. I can sort of see how the mesh offsetting works for this, but I'm not sure why the bounding boxes aren't being generated with the correct values.

[gnustoboz]

Ugh. There's a lot more refactoring in store here than I was hoping for. I've only looked at the last commit here and haven't seen your branch so forgive me if I cover any ground you've already sorted out. The root problem here is that the word wrap code came first, then I came along and added the click logic, and they're not cooperating because the glyph bounds are calculated when a character is rendered, but the word wrap logic is going back and modifying the verts afterwards.

When word wrap is disabled, RebuildMesh() gets the entire line length at once, then if the alignment is center or right it offsets the cursorX accordingly then renders the whole line using BlitString(). The glyph bounds get calculated and stored in a call from BlitString(), so everything lines up regardless of the alignment.

When word wrap is enabled, RebuildMesh() renders one word at a time as if it were left aligned, while tracking the current line width and a list of the current verts in vertexPointers[]. When the word wrap boundary is crossed, it calls OffsetStringPosition() to go back and offset all of the current verts based on the line width and the alignment, then it clears the current vert array and starts again on the next line. There's nothing that goes back and adjusts the glyph bounds at that point, which is why they're still in the left-alignment position.

Right now the bounds are only being stored in the main glyph bounds list, so the trick is to add a way to modify them on each wrap. It could keep track of which bounds need to be adjusted and then go back and adjust just those on each wrap, or it could pre-calculate each line's length so it knows the offset before it starts rendering and the alignment would work the same way it does with word wrap disabled, or it could have a local array like vertexPointers[] to track the bounds as they're generated, offset them as needed, then add them to the main collection.

So hopefully that helps, because I don't know how soon I'd be able to look at this again but I'll help however I can.

[gnustoboz]

Ha, wow. Looks like @invicticide actually has a pre-calculate solution in place that just isn't being used for rendering.

If you find this in RebuildMesh():

string[] lines = Regex.Split(Text, @"\n");

and replace it with this:

string displayText;
if (WordWrap <= 0)
    displayText = Text;
else
    displayText = GetWrappedText(Text);

string[] lines = Regex.Split(displayText, @"\n");

...then that pre-calculates the wrap points and adds line breaks into the displayed string itself, so the non-wrap render logic can be used. I just started testing it but it looks like you can then totally remove all of the word wrap logic in RebuildMesh and use the non-wrapping display logic for everything since it will honor the line breaks the way it should. The version I'm running right now has all of the word wrap logic removed except for GetWrappedText(), plus I removed the OffsetStringPosition() method entirely, and so far it seems to be working.

Edit: noticed and corrected who actually added this method.

[lostfictions]

Thanks so much for digging into this! I'd taken a look and come to a similar conclusion that a bunch of further refactoring would be needed.

Definitely hadn't thought of using GetWrappedText() instead of trying to disentangle all the existing logic though -- that's a great idea! I'll give it a go as soon as I have a moment to try it.