linebender / piet

An abstraction for 2D graphics.
Apache License 2.0
1.24k stars 93 forks source link

Use absolute index to get whitespaceless width & don't sub x coord #444

Closed ForLoveOfCats closed 3 years ago

ForLoveOfCats commented 3 years ago

It seems that the index argument expected here is relative to the start of the text, not the start of the line. Prior to this change long lines w/trailing whitespace would not correctly affect the widest whitespaceless width as their whitespace start index would index past the first line and into part of the second line resulting in a significantly smaller value.

Addresses https://github.com/linebender/druid/issues/1753

ForLoveOfCats commented 3 years ago

Annnnnnd it's broken the Arabic text in snapshot 11

ForLoveOfCats commented 3 years ago

Tracked down the issue with the Arabic text. The hit test for the whitespace is always returning zero. I think it is https://gitlab.gnome.org/GNOME/pango/-/issues/544 striking again which is unfortunate.

ForLoveOfCats commented 3 years ago

Hesitant workaround for the RTL issue by just using the logical width for any RTL text. Additionally snapshot 11 is still broken but in a good way because the results are more correct. I wasn't even aware that snapshot 11's Arabic text had the wrong width to begin with but now it has a much more correct width.

ForLoveOfCats commented 3 years ago

The snapshot changes (just snapshot 11) is up at https://github.com/linebender/piet-snapshots/pull/25. This PR has been updated to point to that and CI is passing as of writing this comment