Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text with bold font truncated with some font types #274

Closed
tksoh opened this issue Jan 14, 2021 · 40 comments
Closed

text with bold font truncated with some font types #274

tksoh opened this issue Jan 14, 2021 · 40 comments

Comments

@tksoh
Copy link
Contributor

tksoh commented Jan 14, 2021

Two nedit windows plus a notepad window in the screenshot below, all three with same editor content and font type (Lucida Console, size 9), the left is highlighted while the middle one is in plain text. Notepad is set to bold face.

You can see on line 1 the word "_required" is truncated to "_requi" when highlighted in bold face in the left window. Text shows up correctly in Notepad. Several fonts/sizes are having similar issue. This is just one example.

Using nedit-ng version 2020.1-160-g333aa16a on Windows 10

2021-01-14 13_37_12-Font

@tksoh
Copy link
Contributor Author

tksoh commented Jan 14, 2021

I also saw several variable width fonts, include some unicode ones (for Chinese and Japanese language) listed. They probably should not be there, but I'm not sure.

2021-01-14 14_08_26-CMakeLists txt - C__Users_teeka_OneDrive_Documents_projects_nedit-ng_src_

@eteran
Copy link
Owner

eteran commented Jan 14, 2021

I'm not sure the fonts in the list are variable width (the example text looks pretty uniform in width to me). It's not something I have much control over regardless since I simply ask the system "what are your fixed-width fonts". If it happens to be wrong... I doubt any API it'll offer will tell me differently.

But I think I have a good idea of what the problem is. I think it's literally the same issue as #270, just on the other axis.

#270 basically was a mistake in assuming that a given font is the same height regardless of boldness... This one appears to an issue of incorrectly assuming that a given font is the same width regardless of boldness.

The reason why we see it basically where a bold text meets non-bold text, is because we render adjacent things with the same style as a single text render command. So for example, we render cmake_minimum_required all at once, but then we miscalculate width of what we just rendered since we (incorrectly) assume that it'll be the non-bold font character width times the number of characters.

So the fix, if we want things to remain truly fixed width, is to, like we did for the font-height, calculate the font metrics in terms of the widest of bold/non-bold.

I'll see about a fix ASAP.

@eteran
Copy link
Owner

eteran commented Jan 14, 2021

@tksoh After further investigation, this is a difficult problem, windows font rendering is... weird.

The difference between the bold and non-bold widths is small, but it adds up across a line. Rendering character individually yields different widths per character than rendering them as part of a multi-character string. (This can be confirmed by simply choosing this font and "rolling a selection slowly across a bold word).

The effect is hard to describe, but basically, the letters seem to shift around the highlight boundary (because that's where we have to switch to single character rendering).

And if I just say "render all characters with an assumed width of the bold variant... well all hell breaks loose. Because the non-bold words will come up short of that having the exact opposite of the truncation problem (and visibly MUCH worse).

I can certainly detect when the bold/plain fonts have different widths, but It'll be REALLY hard to deal with it well as even if I basically dynamically handle the width correctly, it'll break things that rely on nice vertical alignment (like block selections).

I'll need to think on this one ...

@anjohnson
Copy link
Contributor

Hi @eteran, I hate to butt in again, but... I have seen this same issue on Linux too, where the bold font doesn't seem to be quite the same width as the regular one. Some italic or oblique fonts also seem to show strange things, as you might expect with them hanging over the ends of the baseline. I believe I'm using the Ubuntu mono font on Linux – I can check that if you need me to.

@eteran
Copy link
Owner

eteran commented Jan 14, 2021

@anjohnson yea, handling mixed formatting fonts is hard. Honestly, the previous issue, (line-height), I'd consider a font-bug that we just worked around. Since any two fonts with the same point size, should be EXACTLY the same height... but alas, they aren't.

Width is a REAL challenge. I can be hacky and literally render one character at a time in order to guarantee that each character occupies exactly one "cell", but that is pretty much gonna be a mess and end up being super slow. Hopefully, there is some way to force the behavior we need.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

@tksoh @anjohnson

So the easiest solution, and one that is guaranteed to work, is to simply only present the user with fonts where the bold width is equal to the non-bold width. That will "just work".

I have mixed feelings about it because it highly limits the number of "supported fonts". For example on windows, with such a restriction in place, only the "Cascadia", "Consolas", and "Courier" families would qualify. (I can say that "Source Code Pro" and "Hack" also do, but they need to be installed by the user and don't come with the system by default).

On one side, NEdit5 historically, by default, has a VERY limited font selection since most systems don't have many if any bitmap fonts installed.

On the other side, if a user has a font installed, they almost certainly expect it to be available.

I can say that this "solution" will "fix" the problem... but only by avoiding it. Actually addressing it on a rendering level will be a BIG challenge unless there proves to be a way to simply ask Qt to force the bold and non-bold variants to be the same sizes. So far, I don't think such a mechanism exists because as far as Qt is concerned, they are just different fonts.

@anjohnson
Copy link
Contributor

anjohnson commented Jan 15, 2021

Does Qt provide a way to scale a specific font for rendering? You are dealing with TTF or similar in most cases, so I wonder if you could take the user's chosen font and scale it up or down slightly to fit into the bounding box that encloses both the regular and bold versions.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

There's another option that seems to work (that I mentioned) above.

Basically, we can render a single character at a time. It will certainly be slower, but maybe not slow enough to be an issue. I'll have to test some things.

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

I just took a look at Qt Creator, and it's able to handle the variable width fonts (a very long list), and I don't feel much lag, if any, as I drag the vertical scroll bar around continuously. I presume it's build using Qt too. So, perhaps it's worth to find out how they handle it.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

Yea, I've been looking at Qt creator. I can say that it does appear that they allow vertical columns to be slightly misaligned due to font stylings. So they may just be punting on the issue.

For example, if they use the Lucida font and I do a rectangular select in a region that includes both italics and bold in Creator, it ends up looking like this:

image

Personally, I find that very much not ideal, because I use rectangular selects all the time and make heavy use of vertical alignment in my code...

But I have a branch for you to try...

#275

Basically, it renders one character at a time, and seems to work well overall.

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

I tried #275. I like how it handles the rectangular selection. even though it's not a feature I use often. Unfortunately, the scrolling is quite laggy (even when using the mouse wheel), and so is making selection -- something I don't personally feel too comfortable with. So this is going to be a struggle. I don't know if the slowdown is caused by the need to support rectangular selection, or just to render the text correctly.

On the font availability... to be honest, I don't understand why people would use variable font for coding, but this is something very personally, so I won't argue. Perhaps we can consider going Qt Creator's way, but put up a big warning sign when users choose a variable font?

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

Another thought, in the interest of striking the balance between performance impact and 'correctness', is it possible to employ the new rendering and selection mechanism on variable fonts, but leave the true monospaced fonts with the original mechanism?

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

So the interesting thing is that these fonts aren't quite "variable width". They are just different between bold and normal. Individually, the two are fixed width, they are just fixed at different values.

(Essentially we can think of Courier and Courier Bold as independent yet related fonts).

But...

I LOVE your idea of using the fast renderer when I can get away with it. That makes a lot of sense.

Regarding lag, just to be sure, if you are doing a debug build it WILL be slow and should be expected tk be slow. A release build is for "real use"

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

Oh and the slowdown isn't directly regarding rectangular selection, it is just that it is trying REALLY hard to be correct.

I have some ideas for speed up though!

Keep an eye on this branch 😉

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

So the interesting thing is that these fonts aren't quite "variable width". They are just different between bold and normal. Individually, the two are fixed width, they are just fixed at different values.

My apology, I was speaking generally. Qt Creator and Notepad++ actually allow true variable width font such Arial.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

@tksoh Ah, gotcha. Yea, so I actually gutted variable width font support from NG. I know that it is technically a breaking change, but it's something that made the job of porting significantly easier. Like you, I see little to no benefit for a variable-width font in a code editor, so while I may lose some potential users over that decision, I generally feel it was the correct one. Especially since NEdit5's proportional font support was not the default (you have to check a box to even see the non-fixed width fonts).

On another note, I just did a push which I expect to be significantly faster while still fixing this issue.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

It is interesting, my fix is basically running on the assumption (that is seemingly correct) that the difference between bold/normal widths is NEARLY nothing. So close to nothing that we can squeeze any individual bold character into the same space as a non-bold character without clipping.

So far that seems true with all the fonts I've tried. And yet the font designers chose to say that the bold variants are things like 1 pixel (or less!) wider than the regular versions. If it fits, that seems unnecessary.

PS. Personally, I highly recommend "Source Code Pro" (https://fonts.google.com/specimen/Source+Code+Pro). It's a very nice fixed font designed to be easily readable for code (and is also the same width regardless of boldness!)

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

@tksoh OK, now that PR will also always use the fast path for "friendly" fonts :-)

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

PS. Personally, I highly recommend "Source Code Pro" (https://fonts.google.com/specimen/Source+Code+Pro). It's a very nice fixed font designed to be easily readable for code (and is also the same width regardless of boldness!)

I have "Source Code Pro" on Windows, and quite like it too. Unfortunately it's not readily available on Linux Mint, and I am not in the mood to go through the trouble to install it there yet, so I settle with Consolas just to stay consistent across the two platform, and it's quite close to "Source Code Pro".

@tksoh OK, now that PR will also always use the fast path for "friendly" fonts :-)

Looks great. I think we are good to go. Nice work!

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

Merging!

@eteran eteran closed this as completed Jan 15, 2021
@eteran
Copy link
Owner

eteran commented Jan 15, 2021

@tksoh BTW, you totally inspired the solution when you said this:

is it possible to employ the new rendering and selection mechanism on variable fonts, but leave the true monospaced fonts with the original mechanism?

So thanks for your help on this!

@tksoh
Copy link
Contributor Author

tksoh commented Jan 15, 2021

@eteran My pleasure ☺️

@anjohnson
Copy link
Contributor

Not sure that this looks too good:

image

This is 12 point, and it doesn't matter what font I pick (this is "PT Mono"), the character before the equals sign gets similarly cut off in all the fonts I tried including Courier and Courier New, and this is using the regular font, not bold.

Continuing the study in 12 point, the = char is from a Pass-1 pattern ^([^=]+)\s*=\s*(.*)$ and the name and value parts are Coloring for the two sub-expressions. Note that the longer the item name is the more pixels get dropped, and once it gets to about 34 characters long the last character in the name is completely invisible. If I go to 13 point though the opposite happens and I see a gap to the left of the = sign which gets larger the longer the name is:

12 point:
image

13 point:
image

Note also above that the horizontal position of the cursor also seems to be calculated incorrectly, by about column 25 it's drawn in the middle of the character it's supposed to be following. Selected characters seem to get displayed slightly less wide than unselected ones so as I select more of the name starting from the left margin more of that last character comes into view:
image

Hope this is enough to get you started, I can provide more evidence if you need it.

@anjohnson
Copy link
Contributor

I seem to have quite a few copies of this warning on my terminal, maybe from every time I switched fonts (not sure):

Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"
Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"
Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"

@eteran
Copy link
Owner

eteran commented Jan 18, 2021

@anjohnson what's particularly confusing is that the current code will literally draw one character at a time if is drawing bold characters AND the bold font is a different width from the non-bold font.

What this means is that I'd expect that if bold characters get truncated, that they ALWAYS get truncated. Maybe on macOS the system kinda lies about the width of bold fonts?

@eteran
Copy link
Owner

eteran commented Jan 18, 2021

@anjohnson can you try this and see if it helps? In TextArea.cpp, line 3042.

Change:

fastPath = !widerBold_ || (!styleRec->isBold && !styleRec->isItalic);

to just:

fastPath =  (!styleRec->isBold && !styleRec->isItalic);

If that looks better, then macOS is not reporting font widths correctly, and we'll have to work around that.

@eteran
Copy link
Owner

eteran commented Jan 18, 2021

@anjohnson regarding that warning, it comes from Qt itself. So, perhaps it's an issue with some fonts installed on your system?

See: https://gitlab.kitware.com/paraview/paraview/-/issues/18143

@eteran eteran reopened this Jan 18, 2021
eteran added a commit that referenced this issue Jan 18, 2021
…w places

a they just internally disable the warning when they use it. So it seems
reasonable that maybe we should do similar.

`QFont::ForceIntegerMetrics` appears to have no effect on Linux and Windows
that I can detect.. but at least one mac user mentioned a pretty bad rendering
issue. And

(see #274)

And possibly a second one (waiting to hear back on environment details):

(see #287)
@grege2
Copy link

grege2 commented Jan 18, 2021

Hi all, I had the same warnings (below) on Mac as @anjohnson a few days ago. Same font. It was definitely provoked by opening the Text Fonts dialog and just opening the list of font choices. I didn't report it, there was already a storm of activity going on with fonts :=) But, it has disappeared on Big Sur. I went back through all my builds in the last month or so, and all is good.

Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"
Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"
Warning: Failed to compute left/right minimum bearings for "GB18030 Bitmap"

@anjohnson
Copy link
Contributor

@eteran Makes no difference, and yes I an running the modified (-dirty) version here. The d in both instances of isBold is clipped:
image

Did you note that I said none of the characters in my previous example are being shown in bold at all, they are all in the plain font, as are the ones shown above. It seems like it's doing calculations on text widths for things like finding a matching close-parenthesis and coming up with a different answer than the code that is actually rendering the text, did you miss some code that's using a different width calculation when you made these changes?

@eteran
Copy link
Owner

eteran commented Jan 18, 2021

Ughh... That is so strange. Especially since if there isn't any bold then it should be basically doing what it used to be doing.

Have you tried the latest in master, this morning I reverted a small change that I had made because of a similar big report.

@anjohnson
Copy link
Contributor

Ahh, yes as of 2020.1-174-ge189f50d the problem seems to have cleared up, thanks!

@eteran eteran closed this as completed Jan 19, 2021
@tksoh
Copy link
Contributor Author

tksoh commented Feb 1, 2021

Sorry to keep coming back to this...

Using commit 783cd9a on Ubuntu, Noto Color Emoji font somehow still showing text truncated at end of lines. The same font seems to be working okay on Qt Creator.

2021-02-01 16_27_13-toshiba (tksoh's X desktop (tksoh-Satellite-L645_1)) - VNC Viewer

@tksoh
Copy link
Contributor Author

tksoh commented Feb 1, 2021

Line spacings are also a lot smaller than usual.

@eteran
Copy link
Owner

eteran commented Feb 1, 2021

Are there any warnings about variable width fonts?

@eteran
Copy link
Owner

eteran commented Feb 1, 2021

There seems to be REALLY weird spacing good jg on with that font.

Just looking at the line numbers alone (which are rendered by Qts ordinary methods) there is something pretty goofy with that font.

@eteran
Copy link
Owner

eteran commented Feb 1, 2021

OK, So I installed this font on my Gentoo box...

There are no warnings, but I don't see how this font can reasonably be called "fixed-width".

Numbers and special characters are rendered MUCH wider than alpha characters:

image

Just take a look at the "64" in quint64". That weird spacing is coming directly from Qt's rendering as far as I can tell.

Similarly, # characters are rendered extra-wide, and so are * characters.

I don't know what Qt Creator does to support this... but I'm not sure the juice is worth the squeeze on this one.

@eteran
Copy link
Owner

eteran commented Feb 1, 2021

I looked at Qt creator's support.. and yes, it does handle it better. But it does so at the cost of things not really being fixed width.

Spaces, numbers and all special characters are rendered almost double wide.

image

As far as I can see, these special characters are almost seperate from the font in a way. That is, they don't respond to coloring or styling, almost like they are pre-rendered (See how the 1 in QLatin1String is styled wrong in Qt Creator?

I think the best I can do is try to detect fonts like this and just say "no", lol

@tksoh
Copy link
Contributor Author

tksoh commented Feb 2, 2021

Googling "noto color emoji' brought me to a page full of emoji, so I am not even sure it should be qualified as 'font'. LOL

https://www.google.com/get/noto/help/emoji/

It seems to me someone mixed up the font and use the emoji for numbers. The question is how to reliably detect these kind of broken fonts and ignore them.

@eteran
Copy link
Owner

eteran commented Feb 2, 2021

Besides a blacklist, not sure. I'll have to see what properties I can inspect to see if anything stands out.

@eteran
Copy link
Owner

eteran commented Feb 2, 2021

To explain what's going on, and why things get cut off...

To maximize speed, NG makes a few fixed-width assumptions, most notably that char_width * char_count == string_length_in_pixels. I could use Qt's built-in mechanisms to determine the pixel size of rendered strings, but for some versions of Qt, this proved to be unusably slow :-(.

So... what happens is that something like #include <iostream> is calculated to be 19 * char_width wide. Let's just assume that char_width is 10. So the string would be calculated as 190 pixels wide... But the # is actually like 15 pixels wide! So the REAL string width is more like 195 pixels wide. This means when NG is calculated where to render what follows that string (this can be whitespace!), it starts 5 pixels too soon and cuts it off.

Qt creator gets around this by being tolerant of variable-width fonts... but as previously discussed, this makes some select things look terrible. For me, a code editor should be like a terminal. That is, a fixed grid of characters that live in fixed-sized cells.

I'll have to look at Qt-creators strategy for calculating string sizes efficiently. If it's practical, I may eventually revisit the idea of variable width font support (with warnings politely suggesting to the user that it's a bad idea).

1div0 pushed a commit to 1div0/nedit-ng that referenced this issue Mar 24, 2021
…w places

a they just internally disable the warning when they use it. So it seems
reasonable that maybe we should do similar.

`QFont::ForceIntegerMetrics` appears to have no effect on Linux and Windows
that I can detect.. but at least one mac user mentioned a pretty bad rendering
issue. And

(see eteran#274)

And possibly a second one (waiting to hear back on environment details):

(see eteran#287)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants