Bug 56464

Created attachment 20848 [details]
screenshot

Created attachment 20849 [details]
screenshot2

*** Issue 75371 has been marked as a duplicate of this issue. ***

*** Issue 92254 has been marked as a duplicate of this issue. ***

When I run with 1.6, I am seeing problems with both wavy underlines and 
regular underlines on Windows. I think the issue is that the following methods 
return different values in 1.6 vs. 1.5--

fmcInfo.getUnderlineOffset(graphics) (with size 13 Monospaced font on Windows 
XP, .8 with 1.5 JVM and 3.1 with 1.6 JVM)

fmcInfo.getUnderlineThickness(graphics) (1.2 with 1.5 JVM, .55 with 1.6 JVM).

Thus the following code in DrawGraphics's flush method draws the lines lower 
than before:

graphics.fillRect(startX, startY + (int)(fmcInfo.getUnderlineOffset(graphics) 
+ lineAscent + 1.5), x - startX, (int)(fmcInfo.getUnderlineThickness(graphics) 
+ 0.5));

I found a Sun bug report that may explain the font metric differences:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4334839 

Why was 1.5 added initially?  Should these values be changed with the 1.6 JVM?

I have removed the extra 1.5 already from the trunk. It's in

1.25	2006-09-06 09:14:08 +0000 mmetelka
Removed extra shift for underline.

I'll check the current state.

The problem is still there. The wavy underline affects three vertical pixels and
it appears that the lowest one is part of the next line already.
If the lower line needs to be repainted it clears the lowest pixels of the
underline.
There are several solutions possible:
1) Make the line just two pixels high.
2) Move the line one pixel higher. It may be dangerous if the font would be too
small then it might collide with the baseline of the font and it would affect
readability.
3) When rendering a line check if the above line has a wavy underline and render
it. This would be the most difficult solution affecting performance as well.

To get around the wavy-underline problem, I tried option #1 (make line 2 
pixels high).  We actually like it better here.  From DrawGraphics,

int[] wf = {0, 0, -1, -1};  // CLA changed to only be 2 pixels, see NetBeans 
56464

Note that you also have a problem for small fonts with 1.6 where the solid 
underline is not shown at all.  This is because  (int)
(fmcInfo.getUnderlineThickness(graphics) + 0.5) always does the floor, and 
with 1.6 fmcInfo.getUnderlineThickness(graphics) can return < .5 for small 
fonts.  Thus I have also made the following change:

(int)Math.round(fmcInfo.getUnderlineThickness(graphics) + 0.5)  // CLA added 
Math.round or else it is zero for small fonts

The solid line underline problem was reported as another IZ# and I recollect
fixing it. So, it should be working fine in dev builds now. Or are you saying
that the solid underline gets ereased by the caret row highlight as well?

BTW, for the future, it is better to attach a diff with your adjustments,
something like 'cvs diff -uN ...' or use the Versioning->Export Patch for...
functionality in Netbeans. That way we can simply apply your patch instead of
trying to guess what and where exactly you changed. Thank you.

Sorry, I should have mentioned that we are using the 5.0 version of NetBeans.  
I did check that the 5.5 version was the same (we are in the process of 
upgrading), but I did not check the source code under development.

I'll try to make my proposed code changes more useful, but it is challenging 
because we are always at least one release behind.  Thus I can't verify the 
fixes that I propose in the version of NetBeans under development (we heavily 
edit the NetBeans source code, so it takes us about a month to change versions 
of NetBeans, and we can't invest that sort of time for releases under 
development).

No, no, don't wait for upgrade and don't upgrade just to be able to send diffs.
It's perfectly ok to send a diff from 5.0, 5.5 or whatever version you're
currently using, just tell us what it is. The diff is likely to work in 6.0 too
and if your changes conflict with ours we will sort it out somehow. Thanks.

I should mention that the Math.round I proposed on Mar 21 actually causes a 
problem on the Mac (10.4.9 with the newest 1.5 JVM) with font size 12, 
monospaced.  The was the problem I was trying to address:

-------------------------------------------
Note that you also have a problem for small fonts with 1.6 where the solid 
underline is not shown at all.  This is because  (int)
(fmcInfo.getUnderlineThickness(graphics) + 0.5) always does the floor, and 
with 1.6 fmcInfo.getUnderlineThickness(graphics) can return < .5 for small 
fonts.  Thus I have also made the following change:

(int)Math.round(fmcInfo.getUnderlineThickness(graphics) + 0.5)  // CLA added 
Math.round or else it is zero for small fonts
--------------------------------------------

On the Mac with that particular font, the underline ends up being 2 pixels 
thick and it leaves behind some repaint crud because it is too low.  Thus I 
have changed our code to use Math.max instead:

(int)Math.max(1, fmcInfo.getUnderlineThickness(graphics) + 0.5)  // CLA added 
Math.max or else it is zero for small fonts

Hope you guys are at JavaOne and having a good time!

*** Issue 112174 has been marked as a duplicate of this issue. ***

Both problems should be fixed now. This last revision only changes the waveunderline height, the strikethrough and
underline thickness was fixed in some other issue using 1pt as the minimum thickness. Thanks for the patches.

Checking in DrawGraphics.java;
/cvs/editor/libsrc/org/netbeans/editor/DrawGraphics.java,v  <--  DrawGraphics.java
new revision: 1.32; previous revision: 1.31
done

Moving to the ui category.

The same problem in Windows, on current version of Netbeans 6.5.

Underline disappear when moving caret from current line to the following one (see the following screenshot).
Underline is painted 1 pixel lower then required.

The problem appears with most fonts (include standard Monospaced 12), except 'Lucida Console', 'Arial' and some other.

The problem is not appear in Linux or MAC.

Created attachment 81623 [details]
Screenshot

Does changing the font size help? Can somebody with Win box please confirm the problem? Thanks

I can confirm this, it is reproducible with latest trunk build

Created attachment 88404 [details]
the issue under Linux illustration

Under Linux the issue also takes place (png is attached) for current NB trunk. DejaVu Sans Mono font is un use.

Another problem related to current-row highlighting - right part of italized symbols is truncated. I'll attach an example.

Again, it is current NB trunk, Linux, DejaVu Sans Mono font are in use. I have tried another monospaced fonts with the
same "effects" also.

Any hope?

Created attachment 88809 [details]
another current-row issue illustartion

There is always hope :-).

The problem is likely that font's underline offset plus our 1.5 pixel shift constant (some fonts return underline offset to be the same as the lower edge of 
the letters) is greater than font's descent.
I have retested the relation between '_' and underline line for the default font sizes on Linux and Mac (mentioned in the patch) and IMHO the extra shift 
could be 0.5 instead of 1.5 so I've changed that.
To jiriprox: Jirko, could you please test on Windows and with that "DejaVu Sans Mono" font? Thanks.

As a last resort (if we would have to get back to 1.5) I could check that the (underlineOffset + 1.5) < fontDescent and if not then force y = fontDescent - 1 
and height = 1.

http://hg.netbeans.org/jet-main/rev/defcbe769cdb

Integrated into 'main-golden', will be available in build *200910091401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/defcbe769cdb
User: Miloslav Metelka <mmetelka@netbeans.org>
Log: #56464 - Highlighting row overrides error underline - underline line shift constant changed from 1.5 to 0.5

For me the issue still exists (I have reported here examples for Linux). As for undelining decoration - all is fine now,
thanks! But:

1. as for underline char '_' - it isn't shadowed by following current line only with spacing set to ~1.3 and above (i.e.
very sparse text flow). With spacing, say, 1.2 '_' is shadowed. Saying about spacing I mean an appropriate element in
the userdir/config/Editors/Preferences/org-netbeans-modules-editor-settings-CustomPreferences.xml file.

2. current row highlighting still eats right part of wide italized symbols - I have attached example png earlier.

BTW, with spacing 1.2 such symbols like 'g', 'p','y' are also affected by next highlighted row - their bottom part is
slightly truncated.

The "appropriate element" means "line-height-correction"?
The problem is that we can hardly find a solution that would work with all the fonts available. If I do no correction then the LineMetrics.getUnderlineOffset() 
would do what I've mentioned above. With the current correction factor it does not work for your particular font.
I will fix the italics problem.

Yes, I'm saying about "line-height-correction".

Is it possible to list fonts intended to work? DejaVu, I (probably wrong) think, is the most spreaded font under Linux
among sans mono ones.

(if we forget about technical details and try to "feel with eyes" only, it seems (for my eyes) current row highlighting
must be shifted down *in a whole* wrt the current row)

The culprit of the italics rendering problem is the graphics.fillRect() at DrawGraphics:744 which clears the background
beneath the rendered fragment of text. The italicised font returns the same measurements (at least on linux for e.g.
monospaced:18) but e.g. the "w" char is rendered so that it spans the space which should normally be occupied by the
adjacent character. There's a Font.getItalicAngle() but anyway it would be rather difficult to fix it with the present
DrawEngine etc. infrastructure but I will definitely consider this usecase when implementing issue 121357.

Miloslav, at any case all these bugs... microbes are not show stoppers. 
Probably it may be handy just to close this issue at all, as far as they
are already taken in your mind wrt issue 121357 :-)

Ok marking as fixed.

> (if we forget about technical details and try to "feel with eyes" only, it seems (for my eyes) current row highlighting must be shifted down *in a whole* wrt 
the current row)

I understand your concern but it's according to the rendering guidelines - we add a font's ascent to the base y coordinate to get the font's baseline and (ascent 
+ descent) gives the line height. In terms of issue 121357 I could possibly add another hidden tuning parameter that would allow to shift the baseline slightly 
up so that the underline is not at the last pixel of the line's background highlighting.