GNOME Bugzilla – Bug 768679
[PATCH] Fix win32_render_layout_line to draw colors
Last modified: 2017-04-08 01:55:19 UTC
Created attachment 331218 [details] [review] proposed patchset The docs for win32_render_layout and win32_render_layout_line explicitly state that the DC they're passed is used for "uncolored" drawing. However, the code clearly attempts to support colors; it's just doing so incorrectly. This is a series of patches that fix drawing in pangowin32 so at least the most common cases of text color, background color, and underline color are supported. (Alpha transparency isn't.)
Review of attachment 331218 [details] [review]: Can you please provide one attachment per format patch? Thanks
Created attachment 331219 [details] [review] [1/6] enable rendering colored text
Created attachment 331220 [details] [review] [2/6] enable colored underline drawing
Created attachment 331221 [details] [review] [3/6] reset BkMode so ExtTextOutW doesn't draw its own background
Created attachment 331222 [details] [review] [4/6] draw background box in correct color
Created attachment 331223 [details] [review] [5/6] draw background box in correct position
Created attachment 331225 [details] [review] [6/6] update docs to remove "uncolored"
Created attachment 335254 [details] Side-by-side comparison of old and new behavior Added a side-by-side comparison screenshot to illustrate why these patches should be merged. The code drawing the window is the same in both cases; the only difference is the used pangowin32.dll. Left window is drawn to by vanilla pango 1.40.2. Ultimately it would likely be preferable to turn the win32 backend into a subclass of PangoRenderer to avoid having to implement all of its functionality separately. This is also something I've considered, but it seemed to be more reasonable for now to just fix the incorrect WinAPI calls.
The patches look good to me. I would like an opinion from LRN or Fan though.
attachment 331219 [details] [review] - looks ok attachment 331220 [details] [review] - looks ok attachment 331221 [details] [review] - looks ok attachment 331222 [details] [review] - looks ok attachment 331223 [details] [review] - looks ok attachment 331225 [details] [review] - looks ok I should note that my "looks ok" isn't worth much, as i never did use Windows GDI drawing myself. But commit messages in these patches are meaningful, and the changes seem to be matching them.
Hi Nicolas, I need to say that I am on the same stateof this as LRN is regarding GDI. Any chance that you have the source file that reflects the image you have in comment #8? With blessings, thank you!
Created attachment 335873 [details] Source producing attachment 335254 [details] Sure. I've built the test application with MSVC, but I don't think it's using any nonstandard extensions, so gcc should work fine too.
Do you require any additional information or code changes to move forward with this patch series?
This looks good to go in, if someone can commit please.
if you mark it as a-c-n, I'll pick it up in an occasional sweep
Review of attachment 331219 [details] [review]: lgtm
Review of attachment 331220 [details] [review]: lgtm
Review of attachment 331221 [details] [review]: lgtm
Review of attachment 331222 [details] [review]: lgtm
Review of attachment 331223 [details] [review]: lgtm
Review of attachment 331225 [details] [review]: lgtm