oak-team/oak-gpui
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

gpui: Fix blending of colors in HighlightStyle::highlight (#37666)

Browse Source 
Whilst looking into adding support for RainbowBrackes, we stumbled upon
this: Whereas for all properties during this blending, we take the value
of `other` if it is set, for the color we actually take `self.color`
instead of `other.color` if `self.color` is at full opacity.
`Hsla::blend` returns the latter color if it is at full opacity, which
seems wrong for this case. Hence, this PR swaps these.

Will not merge before the next release, to ensure that we don't break
something somewhere unexpected.

Release Notes:

- N/A
This commit is contained in:
Finn Evers
2025-09-10 18:56:49 +02:00
committed by GitHub
parent 611b96627b
commit 6441099a67
6 changed files with 180 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/editor/src/display_map.rs
+48 -53
View File
@@ -703,9 +703,8 @@ impl<'a> HighlightedChunk<'a> {
}),
..Default::default()
};
let invisible_style = if let Some(mut style) = style {
style.highlight(invisible_highlight);
style
let invisible_style = if let Some(style) = style {
style.highlight(invisible_highlight)
} else {
invisible_highlight
};
@@ -726,9 +725,8 @@ impl<'a> HighlightedChunk<'a> {
}),
..Default::default()
};
let invisible_style = if let Some(mut style) = style {
style.highlight(invisible_highlight);
style
let invisible_style = if let Some(style) = style {
style.highlight(invisible_highlight)
} else {
invisible_highlight
};
@@ -962,62 +960,59 @@ impl DisplaySnapshot {
},
)
.flat_map(|chunk| {
let mut highlight_style = chunk
let highlight_style = chunk
.syntax_highlight_id
.and_then(|id| id.style(&editor_style.syntax));
if let Some(chunk_highlight) = chunk.highlight_style {
// For color inlays, blend the color with the editor background
let mut processed_highlight = chunk_highlight;
if chunk.is_inlay
&& let Some(inlay_color) = chunk_highlight.color
{
// Only blend if the color has transparency (alpha < 1.0)
if inlay_color.a < 1.0 {
let blended_color = editor_style.background.blend(inlay_color);
processed_highlight.color = Some(blended_color);
}
let chunk_highlight = chunk.highlight_style.map(|chunk_highlight| {
HighlightStyle {
// For color inlays, blend the color with the editor background
// if the color has transparency (alpha < 1.0)
color: chunk_highlight.color.map(|color| {
if chunk.is_inlay && !color.is_opaque() {
editor_style.background.blend(color)
} else {
color
}
}),
..chunk_highlight
}
});
if let Some(highlight_style) = highlight_style.as_mut() {
highlight_style.highlight(processed_highlight);
} else {
highlight_style = Some(processed_highlight);
}
}
let diagnostic_highlight = chunk
.diagnostic_severity
.filter(|severity| {
self.diagnostics_max_severity
.into_lsp()
.is_some_and(|max_severity| severity <= &max_severity)
})
.map(|severity| HighlightStyle {
fade_out: chunk
.is_unnecessary
.then_some(editor_style.unnecessary_code_fade),
underline: (chunk.underline
&& editor_style.show_underlines
&& !(chunk.is_unnecessary && severity > lsp::DiagnosticSeverity::WARNING))
.then(|| {
let diagnostic_color =
super::diagnostic_style(severity, &editor_style.status);
UnderlineStyle {
color: Some(diagnostic_color),
thickness: 1.0.into(),
wavy: true,
}
}),
..Default::default()
});
let mut diagnostic_highlight = HighlightStyle::default();
if let Some(severity) = chunk.diagnostic_severity.filter(|severity| {
self.diagnostics_max_severity
.into_lsp()
.is_some_and(|max_severity| severity <= &max_severity)
}) {
if chunk.is_unnecessary {
diagnostic_highlight.fade_out = Some(editor_style.unnecessary_code_fade);
}
if chunk.underline
&& editor_style.show_underlines
&& !(chunk.is_unnecessary && severity > lsp::DiagnosticSeverity::WARNING)
{
let diagnostic_color = super::diagnostic_style(severity, &editor_style.status);
diagnostic_highlight.underline = Some(UnderlineStyle {
color: Some(diagnostic_color),
thickness: 1.0.into(),
wavy: true,
});
}
}
if let Some(highlight_style) = highlight_style.as_mut() {
highlight_style.highlight(diagnostic_highlight);
} else {
highlight_style = Some(diagnostic_highlight);
}
let style = [highlight_style, chunk_highlight, diagnostic_highlight]
.into_iter()
.flatten()
.reduce(|acc, highlight| acc.highlight(highlight));
HighlightedChunk {
text: chunk.text,
style: highlight_style,
style,
is_tab: chunk.is_tab,
is_inlay: chunk.is_inlay,
replacement: chunk.renderer.map(ChunkReplacement::Renderer),
crates/editor/src/display_map/custom_highlights.rs
+5 -5
View File
@@ -150,11 +150,11 @@ impl<'a> Iterator for CustomHighlightsChunks<'a> {
..chunk.clone()
};
if !self.active_highlights.is_empty() {
let mut highlight_style = HighlightStyle::default();
for active_highlight in self.active_highlights.values() {
highlight_style.highlight(*active_highlight);
}
prefix.highlight_style = Some(highlight_style);
prefix.highlight_style = self
.active_highlights
.values()
.copied()
.reduce(|acc, active_highlight| acc.highlight(active_highlight));
}
Some(prefix)
}
crates/editor/src/display_map/inlay_map.rs
+3 -3
View File
@@ -385,9 +385,9 @@ impl<'a> Iterator for InlayChunks<'a> {
next_inlay_highlight_endpoint = usize::MAX;
} else {
next_inlay_highlight_endpoint = range.end - offset_in_inlay.0;
highlight_style
.get_or_insert_with(Default::default)
.highlight(*style);
highlight_style = highlight_style
.map(|highlight| highlight.highlight(*style))
.or_else(|| Some(*style));
}
} else {
next_inlay_highlight_endpoint = usize::MAX;
crates/editor/src/editor.rs
+1 -2
View File
@@ -23817,8 +23817,7 @@ pub fn styled_runs_for_code_label<'a>(
} else {
return Default::default();
};
let mut muted_style = style;
muted_style.highlight(fade_out);
let muted_style = style.highlight(fade_out);
let mut runs = SmallVec::<[(Range<usize>, HighlightStyle); 3]>::new();
if range.start >= label.filter_range.end {
crates/gpui/src/style.rs
+119 -44
View File
@@ -886,43 +886,32 @@ impl HighlightStyle {
}
/// Blend this highlight style with another.
/// Non-continuous properties, like font_weight and font_style, are overwritten.
pub fn highlight(&mut self, other: HighlightStyle) {
match (self.color, other.color) {
(Some(self_color), Some(other_color)) => {
self.color = Some(Hsla::blend(other_color, self_color));
}
(None, Some(other_color)) => {
self.color = Some(other_color);
}
_ => {}
}
if other.font_weight.is_some() {
self.font_weight = other.font_weight;
}
if other.font_style.is_some() {
self.font_style = other.font_style;
}
if other.background_color.is_some() {
self.background_color = other.background_color;
}
if other.underline.is_some() {
self.underline = other.underline;
}
if other.strikethrough.is_some() {
self.strikethrough = other.strikethrough;
}
match (other.fade_out, self.fade_out) {
(Some(source_fade), None) => self.fade_out = Some(source_fade),
(Some(source_fade), Some(dest_fade)) => {
self.fade_out = Some((dest_fade * (1. + source_fade)).clamp(0., 1.));
}
_ => {}
#[must_use]
pub fn highlight(self, other: HighlightStyle) -> Self {
Self {
color: other
.color
.map(|other_color| {
if let Some(color) = self.color {
color.blend(other_color)
} else {
other_color
}
})
.or(self.color),
font_weight: other.font_weight.or(self.font_weight),
font_style: other.font_style.or(self.font_style),
background_color: other.background_color.or(self.background_color),
underline: other.underline.or(self.underline),
strikethrough: other.strikethrough.or(self.strikethrough),
fade_out: other
.fade_out
.map(|source_fade| {
self.fade_out
.map(|dest_fade| (dest_fade * (1. + source_fade)).clamp(0., 1.))
.unwrap_or(source_fade)
})
.or(self.fade_out),
}
}
}
@@ -987,10 +976,11 @@ pub fn combine_highlights(
while let Some((endpoint_ix, highlight_id, is_start)) = endpoints.peek() {
let prev_index = mem::replace(&mut ix, *endpoint_ix);
if ix > prev_index && !active_styles.is_empty() {
let mut current_style = HighlightStyle::default();
for highlight_id in &active_styles {
current_style.highlight(highlights[*highlight_id]);
}
let current_style = active_styles
.iter()
.fold(HighlightStyle::default(), |acc, highlight_id| {
acc.highlight(highlights[*highlight_id])
});
return Some((prev_index..ix, current_style));
}
@@ -1306,10 +1296,95 @@ impl From<Position> for taffy::style::Position {
#[cfg(test)]
mod tests {
use crate::{blue, green, red, yellow};
use crate::{blue, green, px, red, yellow};
use super::*;
#[test]
fn test_basic_highlight_style_combination() {
let style_a = HighlightStyle::default();
let style_b = HighlightStyle::default();
let style_a = style_a.highlight(style_b);
assert_eq!(
style_a,
HighlightStyle::default(),
"Combining empty styles should not produce a non-empty style."
);
let mut style_b = HighlightStyle {
color: Some(red()),
strikethrough: Some(StrikethroughStyle {
thickness: px(2.),
color: Some(blue()),
}),
fade_out: Some(0.),
font_style: Some(FontStyle::Italic),
font_weight: Some(FontWeight(300.)),
background_color: Some(yellow()),
underline: Some(UnderlineStyle {
thickness: px(2.),
color: Some(red()),
wavy: true,
}),
};
let expected_style = style_b;
let style_a = style_a.highlight(style_b);
assert_eq!(
style_a, expected_style,
"Blending an empty style with another style should return the other style"
);
let style_b = style_b.highlight(Default::default());
assert_eq!(
style_b, expected_style,
"Blending a style with an empty style should not change the style."
);
let mut style_c = expected_style;
let style_d = HighlightStyle {
color: Some(blue().alpha(0.7)),
strikethrough: Some(StrikethroughStyle {
thickness: px(4.),
color: Some(crate::red()),
}),
fade_out: Some(0.),
font_style: Some(FontStyle::Oblique),
font_weight: Some(FontWeight(800.)),
background_color: Some(green()),
underline: Some(UnderlineStyle {
thickness: px(4.),
color: None,
wavy: false,
}),
};
let expected_style = HighlightStyle {
color: Some(red().blend(blue().alpha(0.7))),
strikethrough: Some(StrikethroughStyle {
thickness: px(4.),
color: Some(red()),
}),
// TODO this does not seem right
fade_out: Some(0.),
font_style: Some(FontStyle::Oblique),
font_weight: Some(FontWeight(800.)),
background_color: Some(green()),
underline: Some(UnderlineStyle {
thickness: px(4.),
color: None,
wavy: false,
}),
};
let style_c = style_c.highlight(style_d);
assert_eq!(
style_c, expected_style,
"Blending styles should blend properties where possible and override all others"
);
}
#[test]
fn test_combine_highlights() {
assert_eq!(
@@ -1337,14 +1412,14 @@ mod tests {
(
1..2,
HighlightStyle {
color: Some(green()),
color: Some(blue()),
..Default::default()
}
),
(
2..3,
HighlightStyle {
color: Some(green()),
color: Some(blue()),
font_style: Some(FontStyle::Italic),
..Default::default()
}
crates/language/src/buffer.rs
+4 -4
View File
@@ -635,13 +635,13 @@ impl HighlightedTextBuilder {
self.text.push_str(chunk.text);
let end = self.text.len();
if let Some(mut highlight_style) = chunk
if let Some(highlight_style) = chunk
.syntax_highlight_id
.and_then(|id| id.style(syntax_theme))
{
if let Some(override_style) = override_style {
highlight_style.highlight(override_style);
}
let highlight_style = override_style.map_or(highlight_style, |override_style| {
highlight_style.highlight(override_style)
});
self.highlights.push((start..end, highlight_style));
} else if let Some(override_style) = override_style {
self.highlights.push((start..end, override_style));