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

Courier New bold fonts mask underscore characters on Windows #270

Closed
tksoh opened this issue Jan 12, 2021 · 24 comments
Closed

Courier New bold fonts mask underscore characters on Windows #270

tksoh opened this issue Jan 12, 2021 · 24 comments

Comments

@tksoh
Copy link
Contributor

tksoh commented Jan 12, 2021

[nedit-ng version 2020.1-155-g4be3c279]

If you compare the highlighted line 1 and 2 in the screenshot taken on Windows below, you can see on line 1 (using bold font) the underscore characters are not visible, while line 2 that's using normal font is fine. A quick round of investigation seemed to point Courier New, bold, size 9. Other sizes or font appears to be okay.

2021-01-12 13_11_33-Window

Notepad has no issue using Courier New, bold, size 9:

2021-01-12 16_24_09-Window

@eteran
Copy link
Owner

eteran commented Jan 12, 2021

Hmm, So I think I know what the problem is. I bet that because the underscore is below the baseline of the font, it is getting overwritten by the background drawing of the next line. Can you test to see if it happens for other font sizes? It's possible that this is an issue with height being rounded down.

Based on this guess, I think that this is basically either

a) a bug where the lines of text aren't properly accounting for the full height of the font, possibly due to rounding down of the height
b) a font quirk that needs be worked around

In either case, the solution would appear to be spacing the lines out vertically one more pixel.

@eteran
Copy link
Owner

eteran commented Jan 12, 2021

Amusingly, I just reproduced this on my windows machine and it ONLY happens with a font-size of 9, LOL.

@eteran
Copy link
Owner

eteran commented Jan 12, 2021

Indeed, adding a + 1 to this line apepars to fix the issue:

fixedFontHeight_ = fm.ascent() + fm.descent();

The question is, is that the "right" solution? Or is there is a way to detect when a font will need just a little bit of spacing. I'd rather not unnecessarily space things out more than they need to be.

@anjohnson
Copy link
Contributor

From here:

The ascent of a font is the distance from the baseline to the highest position characters extend to. In practice, some font designers break this rule, e.g. when they put more than one accent on top of a character, or to accommodate an unusual character in an exotic language, so it is possible (though rare) that this value will be too small.

and

The descent is the distance from the base line to the lowest point characters extend to. In practice, some font designers break this rule, e.g. to accommodate an unusual character in an exotic language, so it is possible (though rare) that this value will be too small.

It looks like QFontMetrics::height() would return the same result, but since these both seem to return a distance instead of a pixel count maybe this is a fencepost error so the +1 is required anyway? Don't know, just thinking aloud here...

@eteran
Copy link
Owner

eteran commented Jan 12, 2021

@anjohnson possibly. I'm going to sneak a peek at some other Qt editors to see if they have any special sauce for calculating font height :-)

@tksoh
Copy link
Contributor Author

tksoh commented Jan 13, 2021

I did a side-by-side comparison between notepad++ (left), nedit-ng (center) & Notepad (right), to see how they handle regular vs bold fonts (all three are using Courier New size 9), with same text area height.

Based on the screenshots below we can see:

  • with normal font:
    • Notepad++ only shows 38 lines
    • Both nedit-ng & Notepad show 40 lines
    • no issue with underscore characters
  • with BOLD font:
    • Notepad++ continue to show 38 lines
    • Notepad matches Notepad++ with 38 lines
    • nedit-ng continue to show 40 lines, but with hidden underscore characters

Looks like Notepad is a little more 'adaptive' to the font settings.

The other thing I don't quite like is that, even with normal font, NG actually displays 2-3 lines less than nedit 5 (on Linux, obviously). This is probably the price to pay for using TTF fonts. I wished there's a way to choose non-TTF on NG.

Screenshot with normal font, Windows:

2021-01-13 10_13_12-Window

Screenshot with bold font, Windows:

2021-01-13 11_19_11-Window

Screenshot with normal font, Linux:

2021-01-13 11_43_32-Window

@eteran
Copy link
Owner

eteran commented Jan 13, 2021

Yea I think that TTF's just aren't going to be quite as dense as bitmap fonts. That smoothness kinda has to take up at least a little more space, especially on the smaller point sizes.

But I think I just realized what the issue is here. I think...

NG calculates the font height based on the normal version of it. But what I really want is whatever height is the biggest between normal, bold and italic variants of the font.

If that's the issue, it's a super easy fix 👍.

@eteran
Copy link
Owner

eteran commented Jan 13, 2021

As a note about NEdit 5 being more dense.

I can say that to my eye, (not measuring), the bitmap font looks every so slightly smaller to me. Have you considered just using a point size of 8 in NG? Is that a viable option, or is it way too small?

eteran added a commit that referenced this issue Jan 13, 2021
@eteran
Copy link
Owner

eteran commented Jan 13, 2021

@tksoh #272 appears to address this in my local tests. Please confirm :-)

@tksoh
Copy link
Contributor Author

tksoh commented Jan 13, 2021

Indeed size 8 is a tad small for my taste, but that's actually not the main issue. The problem is TTF fonts start to get 'hazy' below size 10, and 'a' and 's' begin to look alike at size 8, especially with bold face. So size 9 is about the lowest. Looks like I shall miss the bitmap fonts dearly. Unless I can find a better TTF font.

Will try out your change asap.

@tksoh
Copy link
Contributor Author

tksoh commented Jan 13, 2021

I tested out #272 briefly, and it does appear to fix the hidden underscore issue. However, it also reduces the number of lines displayed from 40 to 37, which one line less than notepad++.

The other thing I just noticed is the large blank space below line 37, which is where line 38 should have been, but probably due to NG refusing to print it because there's isn't enough space to show the line in full. This could lead to the confusion that there's no more line after 37 at first glance. We can of course check the scrollbar to confirm, but that's less than intuitive. Notepad++ and Notepad choose to show the last line partially obscured, so we can tell there might be more lines hidden below. IMO, this seems like a right thing to do. This is certainly up for debate.

Nedit 5 has a special way of incrementing in step size per the line height as we drag horizontal edges of the window, so the window is always 'fitted' to show the full line without excessive blank space below.

@eteran
Copy link
Owner

eteran commented Jan 13, 2021

Yea, accounting for the bold height kinda has to be ever so slightly more space per line, meaning fewer lines would be shown.

I might be able to tighten up the spacing SLIGHTLY, but I'm honestly not sure.

As for the final line not being shown if only partially visible, I'll see if I can improve that 🙂.

For now I'll merge the font fix though since it does meaningfully address functionality.

eteran added a commit that referenced this issue Jan 13, 2021
@tksoh
Copy link
Contributor Author

tksoh commented Jan 13, 2021

I might be able to tighten up the spacing SLIGHTLY, but I'm honestly not sure.

As for the final line not being shown if only partially visible, I'll see if I can improve that 🙂.

That'd be great :-)

For now I'll merge the font fix though since it does meaningfully address functionality.

Absolutely.

@eteran
Copy link
Owner

eteran commented Jan 15, 2021

@tksoh Just pushed to master a fix (only 4 chars!) that will let NG render the last line even when partially displayed!

Closing this issue because I think all aspects of it have now been addressed :-).

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

tksoh commented Jan 20, 2021

@eteran Just found something probably related to this issue...

Using commit 4558173 on Windows. Tested with a couple of different font types.

This document actually has 59 lines. But you can see now only first 58 lines are displayed, while line 59 is hidden. Line 59 stays hidden even after I moved the cursor to line 59 with down arrow.

2021-01-20 14_44_53-Untitled (modified)

Line 59 would gradually show up as I drag the bottom edge of the window down.

2021-01-20 14_46_17-Untitled (modified)

However, when I drag pass the full line of line 59, NG would adjust the text to show only the first 58 lines again.

2021-01-20 14_46_37-Window

@tksoh
Copy link
Contributor Author

tksoh commented Jan 20, 2021

Just to be clear, the text area is only showing about 40 lines in total initially, so it was showing something like lines19 - 58 when I scrolled to the bottom of the text area

@eteran
Copy link
Owner

eteran commented Jan 20, 2021

Hmm, OK I think I know what the issue is here.

I'm pretty sure that the fix to make it render a partial last line has a side effect of NEdit basically always thinking that there may be one extra line. So when the cursor goes to that "one past the end" line, it doesn't think it needs to scroll to show it yet.

I'll need to improve the methodology to make sure it only allows the "extra" line if that line would be partially visible at all.

@eteran eteran reopened this Jan 20, 2021
eteran added a commit that referenced this issue Jan 22, 2021
Now we only add one to the line count if a partial line would have been visible
@eteran
Copy link
Owner

eteran commented Jan 22, 2021

I think this should be fixed in master. Please re-open if the issue is still present :-).

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

tksoh commented Jan 23, 2021

@eteran Unfortunately I am still seeing pretty much the same problem.

BTW, what is your policy in term of displaying the partially obscured line when we move the cursor to that line? Right now, the line remains obscured.

@eteran eteran reopened this Jan 23, 2021
@eteran
Copy link
Owner

eteran commented Jan 23, 2021

No "policy", since we're still trying to just get the behavior right.

I imagine it should be to scroll the partial line into full view it something similar.

@eteran
Copy link
Owner

eteran commented Jan 23, 2021

@tksoh is this on windows? I'm finally back at a windows machine and didn't actually see it occur on my linux/KDE setup

@tksoh
Copy link
Contributor Author

tksoh commented Jan 23, 2021

I saw this on Windows. I can check Linux later

@eteran
Copy link
Owner

eteran commented Jan 23, 2021

@tksoh OK, another question.

When you did see it, was even a single pixel of the cursor visible when the window has focus? I can seemingly only reproduce this bug under basically the circumstance of at least one pixel of the next line is visible.

It is definitely made more obvious as a problem when the window doesn't have focus since the cursor is replaced with a small caret near the bottom of the line instead of a bar (making it fully invisible).

If that's the circumstance where you are seeing this, this will be tricky to "get right" but the "proper" solution is likely to scroll just a bit when a line that is partially visible gets the cursor, so move it into full view.

@eteran
Copy link
Owner

eteran commented Jan 23, 2021

@tksoh After evaluating this issue, I am going to (for now) roll back the commit that makes NG render partially visible lines.

239c360#diff-7f2c5374dd1cefa04a9de579b5008794637109cbbcbb31acc7d221d53648032c

The NEdit5 rendering algorithm wasn't really designed to do that, and while it handles it pretty well to just tell it to "render one more"... it is proving hard to get the little details right.

I consider a bug where the cursor can be on a non-visible line to be much worse than partially visible lines not being drawn since the user can still scroll and reveal the lines.

I'm going to close this bug, but open a new one to track the partially visible line issue since "fixing it right" may prove to be complex.

@eteran eteran closed this as completed Jan 23, 2021
1div0 pushed a commit to 1div0/nedit-ng that referenced this issue Mar 24, 2021
1div0 pushed a commit to 1div0/nedit-ng that referenced this issue Mar 24, 2021
Now we only add one to the line count if a partial line would have been visible
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

3 participants