After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682692 - off-by-one drawing bug
off-by-one drawing bug
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
[fixed-next]
Depends on:
Blocks: 756010
 
 
Reported: 2012-08-25 19:53 UTC by Christian Persch
Modified: 2018-03-21 20:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
typescript (5.16 KB, text/plain)
2012-08-25 19:53 UTC, Christian Persch
  Details
screenshot (71.54 KB, image/png)
2012-08-25 19:56 UTC, Christian Persch
  Details
drawing: Don't overdraw the cell on bold characters (765 bytes, patch)
2012-08-25 20:38 UTC, Christian Persch
rejected Details | Review
drawing: Don't overdraw the cell on bold characters (1.23 KB, patch)
2012-08-25 20:41 UTC, Christian Persch
none Details | Review
Don't extend the background on faux bold (2.04 KB, patch)
2018-03-21 13:05 UTC, Egmont Koblinger
committed Details | Review

Description Christian Persch 2012-08-25 19:53:51 UTC
Created attachment 222430 [details]
typescript

Steps: start vteapp and cat the attached typescript

Results: the QR code has a black 1px line at the right hand that's not there in the data; all the lines end in a U+2588 FULL BLOCK character.
Comment 1 Christian Persch 2012-08-25 19:56:29 UTC
Created attachment 222431 [details]
screenshot
Comment 2 Christian Persch 2012-08-25 20:38:06 UTC
Created attachment 222433 [details] [review]
drawing: Don't overdraw the cell on bold characters
Comment 3 Christian Persch 2012-08-25 20:41:46 UTC
Created attachment 222434 [details] [review]
drawing: Don't overdraw the cell on bold characters

This was added in comment dcb7fd974bea5fe49d4f5b0344ebdaffc6d7bae1 and apparently
used for pseudo-bold, which we don't do anymore.
Comment 4 Christian Persch 2012-08-25 21:07:38 UTC
Fixed on 0-34 and next.
Comment 5 Egmont Koblinger 2017-12-19 09:08:19 UTC
This doesn't look fully fixed to me. In case of faux bold, the background is still extended by 1px in ::draw_cells()

gint bold_offset = _vte_draw_has_bold(...);
_vte_draw_fill_rectangle(..., w + bold_offset, ...);

and once again in ::draw_rows.

How about we entirely drop this idea of extending the background color, just let the faux bold overflow from its background (as it already happens for every glyph that's just wider than the standard English ones) (and make _vte_draw_has_bold private/static to vtedraw)?

(... or drop faux bold?)
Comment 6 Christian Persch 2017-12-19 20:46:37 UTC
(In reply to Egmont Koblinger from comment #5)
> How about we entirely drop this idea of extending the background color, just
> let the faux bold overflow from its background (as it already happens for
> every glyph that's just wider than the standard English ones)

Good idea.
Comment 7 Egmont Koblinger 2018-03-21 13:05:45 UTC
Created attachment 369955 [details] [review]
Don't extend the background on faux bold

How to test:

Pick a font that doesn't have bold counterpart, maybe because it's already bold:
  ./src/app/vte-2.91 -f 'Monospace Bold 10'

Produce some bold and non-bold text with background whose right column should be aligned, e.g.
  ./perf/sgr-test.sh

Notice that the right edge of the purple background didn't align prior to this patch, but aligns now.

This is the first of two patches, the second one goes into bug 756010.
Comment 8 Christian Persch 2018-03-21 18:48:47 UTC
Comment on attachment 369955 [details] [review]
Don't extend the background on faux bold

Master only, please. Thanks for the patch!
Comment 9 Egmont Koblinger 2018-03-21 20:27:06 UTC
> Master only, please.

Obviously! :)

Submitted.