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

force alpha to 1 when using background color as inverted foreground color #2560

Merged
merged 8 commits into from
Nov 25, 2019

Conversation

ivanwonder
Copy link
Contributor

@ivanwonder ivanwonder commented Nov 11, 2019

This problem has been described there #1898.

Fixes #1898

src/browser/renderer/atlas/DynamicCharAtlas.ts Outdated Show resolved Hide resolved
src/browser/renderer/atlas/DynamicCharAtlas.ts Outdated Show resolved Hide resolved
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a bunch of changes to do the same in webgl and cover extra cases for canvas and non-black backgrounds. Wasn't as straightforward as I was thinking.

@Tyriar Tyriar added this to the 4.3.0 milestone Nov 25, 2019
@Tyriar Tyriar self-assigned this Nov 25, 2019
@Tyriar Tyriar merged commit 8b5c48f into xtermjs:master Nov 25, 2019
@Eugeny
Copy link
Member

Eugeny commented Nov 25, 2019

Works with the canvas renderer, but not WebGL. I've poked around a bit but could find why right away

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny do you mean transparency in general or this specific issue (\x1b[7m█\x1b[0m should be #ff0000 when the background color is #ff000080 and allowTransparency is true?

@Eugeny
Copy link
Member

Eugeny commented Nov 26, 2019

I meant #1898

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny is my test case correct? That was working on all 3 renderers when I tested.

@Eugeny
Copy link
Member

Eugeny commented Nov 26, 2019

With a white-on-transparent theme,

Canvas:
image

WebGL:
image

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny what's the background set to exactly?

@Eugeny
Copy link
Member

Eugeny commented Nov 26, 2019

Literally "transparent" (as in CSS color)

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny oh, that's the problem. I was assuming you were using #rgba or #rrggbbaa, it should work if you change that to #00000000. I hadn't even considered using transparent here, that's also why you don't get the scrollbar problem in #2252 (comment)

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny what's your proposal to fix that? It feels like that's sort of undefined behavior and we shouldn't allow using transparent for this reason, using 0 for the alpha channel instead lets you specify the opaque inverted version.

@Eugeny
Copy link
Member

Eugeny commented Nov 26, 2019

What do you use to convert named colors to RGBA? transparent should be equivalent to rgba(0, 0, 0, 0) per CSS spec

@Tyriar
Copy link
Member

Tyriar commented Nov 26, 2019

@Eugeny 👍 opening issue again to add this case:

const bg = this._config.colors.background.css;
if (bg.length === 9) {
// Remove bg alpha channel if present
return bg.substr(0, 7);
}
return bg;

@ivanwonder
Copy link
Contributor Author

@Tyriar Because of the transparent, so I remove the bg alpha channel by using the RGBA value and then convert it to CSS string.
https://github.com/ivanwonder/xterm.js/blob/1ff1809ab75be9a9aeee134e6dd5c040f68efda0/src/browser/Color.ts#L59

@Tyriar
Copy link
Member

Tyriar commented Nov 27, 2019

@ivanwonder the fix I did for webgl didn't cover when background=transparent though, I'll fix that before we do a release.

@ivanwonder
Copy link
Contributor Author

@Tyriar https://html.spec.whatwg.org/multipage/canvas.html#serialisation-of-a-color
context.fillStyle will convert transparent to rgba(0, 0, 0, 0).
if it has alpha equal to 1.0, then the string is a lowercase six-digit hex value, prefixed with a "#" character, Otherwise, the color value has alpha less than 1.0, and the string is the color value in the CSS rgba()
maybe it will help

@Tyriar
Copy link
Member

Tyriar commented Nov 30, 2019

@ivanwonder thanks, good info

@ivanwonder
Copy link
Contributor Author

I just try the idea that using context.fillStyle. Looks work fine.
https://github.com/ivanwonder/xterm.js/tree/reverse-transparency

Tyriar added a commit to microsoft/vscode that referenced this pull request Dec 31, 2019
xterm-addon-webgl@0.5.0-beta.7

Diff: xtermjs/xterm.js@8341c35...2a9e16b

- Include  in word separators xtermjs/xterm.js#2583
- Remove unused imports/functions xtermjs/xterm.js#2585
- force alpha to 1 when using background color as inverted foreground color xtermjs/xterm.js#2560
- Fix minimumContrastRatio on dom/truecolor xtermjs/xterm.js#2602
- v4.3.0 xtermjs/xterm.js#2605
- Avoid roundtrip to browser when double-disposing. xtermjs/xterm.js#2616
- Allow the thickness of the bar cursor to be configured xtermjs/xterm.js#2590
- update version of node-pty xtermjs/xterm.js#2621
- Implement hidden in DOM and WebGL renderers xtermjs/xterm.js#2625
- Expose texture atlas as API and use in demo xtermjs/xterm.js#2626
- Webgl v0.4.1 xtermjs/xterm.js#2628
- Add Linode to real world uses xtermjs/xterm.js#2636
- Added Gus to list of xterm real-world users xtermjs/xterm.js#2631
- Remove a large portion of InputHandler's dependency on Terminal xtermjs/xterm.js#2637
- Move back to reseting parser only on RIS xtermjs/xterm.js#2640
- Set glyph fg color based on original bg, not selection xtermjs/xterm.js#2650
- format color value to style '#rrggbbaa' xtermjs/xterm.js#2629
- Use register over add for APIs returning disposables xtermjs/xterm.js#2651
- Standardize how colors helper lib is structured xtermjs/xterm.js#2653
- Fullwidth handling in buffer writes xtermjs/xterm.js#2644
- Target es5 in attach addon xtermjs/xterm.js#2654

Fixes #86194
Fixes #87918
Part of #87456
lemanschik pushed a commit to code-oss-dev/code that referenced this pull request Nov 25, 2022
xterm-addon-webgl@0.5.0-beta.7

Diff: xtermjs/xterm.js@8341c35...2a9e16b

- Include  in word separators xtermjs/xterm.js#2583
- Remove unused imports/functions xtermjs/xterm.js#2585
- force alpha to 1 when using background color as inverted foreground color xtermjs/xterm.js#2560
- Fix minimumContrastRatio on dom/truecolor xtermjs/xterm.js#2602
- v4.3.0 xtermjs/xterm.js#2605
- Avoid roundtrip to browser when double-disposing. xtermjs/xterm.js#2616
- Allow the thickness of the bar cursor to be configured xtermjs/xterm.js#2590
- update version of node-pty xtermjs/xterm.js#2621
- Implement hidden in DOM and WebGL renderers xtermjs/xterm.js#2625
- Expose texture atlas as API and use in demo xtermjs/xterm.js#2626
- Webgl v0.4.1 xtermjs/xterm.js#2628
- Add Linode to real world uses xtermjs/xterm.js#2636
- Added Gus to list of xterm real-world users xtermjs/xterm.js#2631
- Remove a large portion of InputHandler's dependency on Terminal xtermjs/xterm.js#2637
- Move back to reseting parser only on RIS xtermjs/xterm.js#2640
- Set glyph fg color based on original bg, not selection xtermjs/xterm.js#2650
- format color value to style '#rrggbbaa' xtermjs/xterm.js#2629
- Use register over add for APIs returning disposables xtermjs/xterm.js#2651
- Standardize how colors helper lib is structured xtermjs/xterm.js#2653
- Fullwidth handling in buffer writes xtermjs/xterm.js#2644
- Target es5 in attach addon xtermjs/xterm.js#2654

Fixes microsoft#86194
Fixes microsoft#87918
Part of microsoft#87456
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling transparent BG color when used as FG
3 participants