Modify background color for highlighted text when fg == bg #458

Manually merged
dnkl merged 1 commits from modify-bg-color-when-highlighted-text-has-same-fg-bg into master 9 months ago
  1. 4
      CHANGELOG.md
  2. 5
      render.c

4
CHANGELOG.md

@ -33,6 +33,10 @@
* `generate-alt-random-writes.py --sixel`: width and height of emitted
sixels has been adjusted.
* _Concealed_ text (`\E[8m`) is now revealed when highlighted.
* The background color of highlighted text is now adjusted, when the
foreground and background colors are the same, making the
highlighted text legible
(https://codeberg.org/dnkl/foot/issues/455).
### Deprecated

5
render.c

@ -443,6 +443,11 @@ render_cell(struct terminal *term, pixman_image_t *pix,
}
}
if (unlikely(is_selected && _fg == _bg)) {
Review

FYI, unlikely has not actually been useful since the 80's.

FYI, unlikely has not actually been useful since the 80's.
Review

I'm intrigued to know why. Could you elaborate?

I'm intrigued to know why. Could you elaborate?
Review

The compiler doesn't do anything with it anymore, it's a no-op. The branch predictor is a black box, there is no way for the programmer to inform it in advance on x86_64 or most other modern architectures.

The compiler doesn't do anything with it anymore, it's a no-op. The branch predictor is a black box, there is no way for the programmer to inform it in advance on x86_64 or most other modern architectures.
Review

The compiler doesn't do anything with it anymore

It doesn't?

https://godbolt.org/z/sx6n9xjjn

> The compiler doesn't do anything with it anymore It doesn't? https://godbolt.org/z/sx6n9xjjn
Review

Hm, I think this is gcc interpreting it as part of its internal branch prediction optimizer, but not giving any hints to the hardware. In any case, it does seem to do something, so I guess it works.

Hm, I think this is gcc interpreting it as part of its internal branch prediction optimizer, but not giving any hints to the hardware. In any case, it does seem to do something, so I guess it works.
Review

I think this is gcc interpreting it as part of its internal branch prediction optimizer

It's GCC re-ordering the code so that the "hot" parts are contiguous, in order to lessen i-cache fragmentation. Without the likely annotation, the dynamic branch predictor will still take care of predictable branches, but it can't re-order the code to make it more cache efficient. You can only do that at compile-time.

Using __builtin_expect() in the right places can have a significant impact on performance and power efficiency.

https://easyperf.net/blog/2018/07/09/Improving-performance-by-better-code-locality#why-latter-case-is-better

(this article is a bit of a typo-fest; but illustrates the point quite well)

> I think this is gcc interpreting it as part of its internal branch prediction optimizer It's GCC re-ordering the code so that the "hot" parts are contiguous, in order to lessen i-cache fragmentation. Without the `likely` annotation, the dynamic branch predictor will still take care of predictable branches, but it can't re-order the code to make it more cache efficient. You can only do that at compile-time. Using `__builtin_expect()` in the right places can have a significant impact on performance and power efficiency. https://easyperf.net/blog/2018/07/09/Improving-performance-by-better-code-locality#why-latter-case-is-better (this article is a bit of a typo-fest; but illustrates the point quite well)
/* Invert bg when selected/highlighted text has same fg/bg */
_bg = ~_bg;
}
if (cell->attrs.dim)
_fg = color_dim(_fg);
if (term->conf->bold_in_bright.enabled && cell->attrs.bold)

Loading…
Cancel
Save