eclipse.platform.swt icon indicating copy to clipboard operation
eclipse.platform.swt copied to clipboard

Support fixed line height for `StyledText`

Open SyntevoAlex opened this issue 2 years ago • 12 comments

This is a continuation of https://bugs.eclipse.org/bugs/show_bug.cgi?id=527917

Currently, StyledText automatically increases line height when it encounters a character that is taller than current line height.

Here's an example of what happens when it starts with latin text, but then encounters a tall unicode character while scrolling: LineHeight17-38

This is bad for two reasons:

  1. It wastes a lot of screen space in text editor
  2. When two parallel text are needed (for example, to show git diffs), the panes easily desynchronize when scrolling them.

Possible solutions, in order of my preference, include:

  1. Somehow render tall characters without increasing line height
  2. Have variable line height in StyledText
  3. Don't render tall unicode characters - that's obviously bad because if character is there, it's there for a reason and should be shown

In my testing, I found that solution 1 is good - "forcing" tall characters into smaller lines still looks reasonable. Here's a screenshot using my not-yet-finished patch on Ubuntu: PatchAtWork

as it can be seen, even the tallest characters still look reasonable without increasing default line height.

SyntevoAlex avatar Dec 05 '23 16:12 SyntevoAlex

Code Mining can handle some variable line height and render lines with different height when a character is replaced by a StyleRange with different height. Could this strategy apply to taller unicode character as well, or does it have to be more "native"?

mickaelistria avatar Dec 05 '23 18:12 mickaelistria

Sorry but I failed to grasp the idea you're describing. What is "Code Mining" ? What exactly does it do when a line contains a tall unicode character?

SyntevoAlex avatar Dec 05 '23 18:12 SyntevoAlex

To clarify, StyledText already handles tall characters by making ALL lines tall enough to contain it.

SyntevoAlex avatar Dec 05 '23 18:12 SyntevoAlex

Code Minings are what is described in https://eclipse.dev/eclipse/news/4.8/platform_isv.php#codemining-sourceviewer-support . Under the hood, what they do is that they enlarge some lines in the StyledText to display addition information. Enlarging lines is simply achieved by putting the right GlyphMetrics on a StyleRange. Snippet212 does show a simpler case for it, with lines of variable height according to their content. Screenshot from 2023-12-05 23-14-33

mickaelistria avatar Dec 05 '23 22:12 mickaelistria

Thanks, that looks interesting! I'll investigate this snippet.

SyntevoAlex avatar Dec 05 '23 22:12 SyntevoAlex

Unfortunately this approach does not help to make lines lower (so that tall characters take same space as latin characters).

I made quick&dirty experimental patches for Windows and Linux.

Windows In TextLayout.shape() I made if (style.metrics != null) behave as if metrics was always set. The result is that lines now obey requested height. However only part of character is rendered - if character is twice taller than line, then only top half of character is painted. I understand that this produces unreadable text. You can try obscuring bottom half of this text's line and see if it's readable.

Linux In TextLayout.shape() I made if (metrics != null) behave as if metrics was always set. The result is that line can not become lower, only taller. Further, StyledText does not expect this and still assumes that lines have same height, which throws off hit test calculations.

To summarize, setting StyleRange is not a good way to make taller lines have same height as latin lines.

SyntevoAlex avatar Dec 10 '23 15:12 SyntevoAlex

  1. Have variable line height in StyledText

I guess that this is more complicated to implement, but does everybody agree that it would be the most correct solution?

revolter avatar Dec 27 '23 19:12 revolter

Variable line height is already implemented in StyledText. But when two parallel StyledTexts need to be in sync (to show diffs in git), variable line height becomes a problem of its own.

SyntevoAlex avatar Dec 27 '23 19:12 SyntevoAlex

But when two parallel StyledTexts need to be in sync (to show diffs in git), variable line height becomes a problem of its own.

Can you please clarify what particular problems you're referring to?

mickaelistria avatar Jan 08 '24 11:01 mickaelistria

Can you please clarify what particular problems you're referring to?

  1. Aligning (that is, scrolling synchronously) two parallel StyledTexts becomes a bigger problem with variable line height
  2. It's uglier. In Linux, gedit uses variable line height, and I never liked it. It's somewhat similar to why we programmers like to use mono fonts.
  3. Why waste screen space when hieroglyphs fit rather well into standard latin line heights?

SyntevoAlex avatar Jan 24 '24 22:01 SyntevoAlex

Aligning (that is, scrolling synchronously) two parallel StyledTexts becomes a bigger problem with variable line height

I'm optimistic that this particular issue can be fixed relatively easily. The effort of designing what the behavior should be, then coding it shouldn't be hard.

Why waste screen space when hieroglyphs fit rather well into standard latin line heights?

Isn't it a bit hard to know in advance whether all characters fit "rather well"?

mickaelistria avatar Jan 25 '24 00:01 mickaelistria

I'm optimistic that this particular issue can be fixed relatively easily. The effort of designing what the behavior should be, then coding it shouldn't be hard.

Maybe. Or maybe not. Either way, it's an additional problem. Also, there's a beauty issue when line of the left has different height from matching line on the right (because one of these has a hieroglyph and the other doesn't). Yes, it's not a very big issue, yet still, this patch solves it.

Isn't it a bit hard to know in advance whether all characters fit "rather well"?

In the attached testing snippet, I took my time to test all common scripts (Japanese, Chinese, Hebrew, etc etc) on all 3 platforms. It looks good. You don't have to take my word on it, check for yourself.

SyntevoAlex avatar Jan 28 '24 23:01 SyntevoAlex