masm11 / emacs

Mirror of GNU Emacs
http://www.gnu.org/software/emacs/
GNU General Public License v3.0
198 stars 14 forks source link

Font lock not applied to second line using proportional fonts #29

Closed fejfighter closed 4 years ago

fejfighter commented 4 years ago

Hi, @fejfighter,

Here is a screenshot of pgtk emacs running a mail viewer:

screenshot-20200709-005635

The last line of Subject is not colored, but its face is the same as the colored first and second lines.

That seems not to occur when only ascii characters. Of cource, it doesn't occur on X emacs.

I'll debug it.

Originally posted by @masm11 in https://github.com/masm11/emacs/pull/23#issuecomment-655615904

fejfighter commented 4 years ago

@fejfighter

Save this file: test.txt

Rename it to test.el.

Open it in pgtk emacs.

Shrink window width.

Then, the first line is colored but the second line is not colored.

screenshot-20200709-194720

EDIT:

The characters in the file are Japanese Hiragana characters. If you don't have Japanese fonts, please download NotoSansCJK-Regular.ttc, and use the Noto Sans Mono CJK JP font.

fejfighter commented 4 years ago

I suspect the reflow symbol in white is setting the cairo colour and it's not always being reset corectly for the second line but I can't confirm that is the case

if I mark text on line 1 going through to the wrapped line 2 it will cause the remainder of the line to be colour correctly, so it's something in that code path

Screenshot from 2020-07-10 19-29-16 Screenshot from 2020-07-10 19-28-43

fejfighter commented 4 years ago

@masm11 found the regression change

I'm looking for when the issue was introduced.

At the start of this year, pgtk emacs didn't have that issue.

commit f2d12841cbe8a7c7153d1428134077f7fccda983
Author: Yuuki Harano <masm+github@masm11.me>
Date:   Wed May 6 19:04:23 2020 +0900

    * pgtkmenu.c (set_frame_menubar): fix empty menu.

When that, there was not the problem.

commit 2a6abead8019300d18ec0a09fe9c3322c31739e6
Author: Yuuki Harano <masm+github@masm11.me>
Date:   Wed May 13 00:03:43 2020 +0900

    * src/pgtkterm.h: remove pgtk_lisp_to_color declaration

When that, there was.

Between those commits, I changed color name handling. Those changes seem to affect.

masm11 commented 4 years ago

Solved!

https://github.com/masm11/emacs/blob/54ef44216cef3480d69b4da2f46b8db5b0249d64/src/pgtkterm.c#L6721

In pgtk_parse_color: color->pixel is 0xffRRGGBB. It is returned as unsigned long.

https://github.com/masm11/emacs/blob/54ef44216cef3480d69b4da2f46b8db5b0249d64/src/pgtkterm.c#L6696

It is called in pgtk_defined_color, and it returns ffRRGGBB as unsigned long.

https://github.com/masm11/emacs/blob/54ef44216cef3480d69b4da2f46b8db5b0249d64/src/pgtkfns.c#L959

It is called in x_decode_color, and it returns ffRRGGBB as int, becase its return type is int.

https://github.com/masm11/emacs/blob/54ef44216cef3480d69b4da2f46b8db5b0249d64/src/pgtkfns.c#L959 It is called in x_set_babckground_color, and returned value is assigned to bg, which is unsigned long again, so the value is 0xffffffffffRRGGBB.

At the next line, FRAME_BACKGROUND_PIXEL(f) is 0xffffffffffRRGGBB.

Color names are converted to color values sometimes via x_decode_color and sometimes via pgtk_defined_color. If x_decode_color is used, returned value is 0xffffffffffRRGGBB. If pgtk_defined_color is used, returned value is 0xffRRGGBB.

When color value comparison is performed, these values are different. I'm not sure which comparison hits the problem.

The solution is removing alpha bits. https://github.com/masm11/emacs/commit/3c89107ff75c638dcf372e3e5483c3987614e3ad

fejfighter commented 4 years ago

Interesting,

had to be something subtle like the type changes you discovered.

I wonder if we managed to avoid (to some degree) it in the past by using the "Emacs_Col" type

I'll pull that in tonight

masm11 commented 4 years ago

I'll pull that in tonight

thanks.