Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search matchBackground isn't working in dom renderer #4642

Closed
Tyriar opened this issue Aug 4, 2023 · 5 comments · Fixed by #4651
Closed

Search matchBackground isn't working in dom renderer #4642

Tyriar opened this issue Aug 4, 2023 · 5 comments · Fixed by #4651
Assignees
Labels
area/renderer-dom type/bug Something is misbehaving
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Aug 4, 2023

Repro:

Set matchBackground in client.ts to #ff0000 and use the search addon to find something with multiple matches.

Webgl:

Screenshot 2023-08-04 at 8 29 12 am

DOM:

Screenshot 2023-08-04 at 8 30 00 am
@Tyriar Tyriar added type/bug Something is misbehaving area/renderer-dom labels Aug 4, 2023
@Tyriar Tyriar added this to the 5.3.0 milestone Aug 4, 2023
@Tyriar Tyriar self-assigned this Aug 4, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Aug 4, 2023

@jerch this was due to the fast DOM renderer change, see microsoft/vscode#189600 for more info on the bug.

Pretty sure it's caused by merging cells before the decoration color is resolved as the bg and fg (not border) are rendered as part of the regular grid:

// Apply any decoration foreground/background overrides, this must happen after inverse has
// been applied
let bgOverride: IColor | undefined;
let fgOverride: IColor | undefined;
let isTop = false;
this._decorationService.forEachDecorationAtCell(x, row, undefined, d => {
if (d.options.layer !== 'top' && isTop) {
return;
}
if (d.backgroundColorRGB) {
bgColorMode = Attributes.CM_RGB;
bg = d.backgroundColorRGB.rgba >> 8 & 0xFFFFFF;
bgOverride = d.backgroundColorRGB;
}
if (d.foregroundColorRGB) {
fgColorMode = Attributes.CM_RGB;
fg = d.foregroundColorRGB.rgba >> 8 & 0xFFFFFF;
fgOverride = d.foregroundColorRGB;
}
isTop = d.options.layer === 'top';
});

@Tyriar
Copy link
Member Author

Tyriar commented Aug 4, 2023

Another result of this:

Webgl:

Screenshot 2023-08-04 at 9 04 23 am

DOM:

Screenshot 2023-08-04 at 9 05 20 am

@jerch
Copy link
Member

jerch commented Aug 5, 2023

Yeah, seems this needs an exclusion from the merger.

@jerch
Copy link
Member

jerch commented Aug 6, 2023

@Tyriar Is there an easy way to test/repro this? Whats the desired behavior here?

@Tyriar
Copy link
Member Author

Tyriar commented Aug 7, 2023

@jerch setting matchBackground to something obvious in client.ts and then use the find previous textbox/button. Was pretty easy to repro for me. Only the search term should be highlighted, in this one only the t cells should have red backgrounds:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/renderer-dom type/bug Something is misbehaving
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants