Make text always align with font base line
ClosedPublic

Authored by xuetianweng on Fri, May 15, 11:38 PM.

Details

Summary

When font uses other fallback font, the base line may change to make all
text on the same baseline. But when we render the text, we should always
follow the baseline from FontMetrics to avoid text baseline jump up and
down depending on the text in the line.

Screenshot before and after.

Test Plan

Text manually with some mixed text.

Diff Detail

Repository
R39 KTextEditor
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26976
Build 26994: arc lint + arc unit
xuetianweng created this revision.Fri, May 15, 11:38 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptFri, May 15, 11:38 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
xuetianweng requested review of this revision.Fri, May 15, 11:38 PM
xuetianweng edited the summary of this revision. (Show Details)Fri, May 15, 11:41 PM

This is my another try as an alternative solution to D25339. Actually this works surprisingly good IMHO, at least for CJK users for most cases.

I like this patch. In fact, I observed over the past years again and again that sometimes, especially if chinese or similar letters are included, the baseline is wrong in Kate, leading to much more overpainting that needed.
If this patch fixes this, then I'm all for it. Or let's put it like this: The current implementation is wrong, this patch looks less wrong than the current state :-)

+1

I like this patch. In fact, I observed over the past years again and again that sometimes, especially if chinese or similar letters are included, the baseline is wrong in Kate, leading to much more overpainting that needed.
If this patch fixes this, then I'm all for it. Or let's put it like this: The current implementation is wrong, this patch looks less wrong than the current state :-)

+1

Actually if this patch is accepted, we may then want to add a configurable multiplier on line height instead of enable variable lineheight support to resolve the character clip issue.
Background painting to fill the gap to cover extra gap between line is probably easier and safer to implement.

Equal line height would look better anyway.

brauch added a subscriber: brauch.Sun, May 17, 5:34 PM

Hmm, consider though that a configuration option should be something that gives a choice to the user. It shouldn't be necessary to set a config option in order to make the program behave correctly.

cullmann accepted this revision.Mon, May 18, 1:51 PM

Hmm, looks better for me, too.
Let's go with this at the moment.
If it creates issues, we can still revert it again.
Thanks for taking care of this.

For the option: some option to allow people to adjust the spacing for e.g. better readability is fine (if it works), to have some option that people need to fiddle with to have correct rendering at all sounds bad.

This revision is now accepted and ready to land.Mon, May 18, 1:51 PM
cullmann closed this revision.Mon, May 18, 1:52 PM