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

Add support for the "blink" graphic rendition attribute #7388

Closed
j4james opened this issue Aug 25, 2020 · 10 comments · Fixed by #7490
Closed

Add support for the "blink" graphic rendition attribute #7388

j4james opened this issue Aug 25, 2020 · 10 comments · Fixed by #7490
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@j4james
Copy link
Collaborator

j4james commented Aug 25, 2020

Description of the new feature/enhancement

We already parse and store the SGR 5 blink attribute, but we don't actually "render" it. This is the only remaining DEC attribute that we don't yet support, and it's widely implemented by other terminal emulators. It's probably not essential, but it can occasionally be useful, and is fun for creating simple "animations".

Proposed technical implementation details (optional)

I've been experimenting with this on the conhost side of things, and I found it could quite easily be implemented by hooking into the existing timer routine in the CursorBlinker class, triggering a redraw of the render target every cycle or two. But since we don't want that overhead if nothing is actually blinking, I have a flag that is enabled whenever a blink attribute is encountered in the renderer, and only trigger the redraw if that flag has been set. The flag is cleared before every redraw, so once the viewport is free of any blinking attributes, the redraws will stop.

As for how the blinking is actually rendered, there are two main approaches, demonstrated in the images below. On the left is what I call the "PC style", where the text actually disappears. On the right is the "DEC style" (the way I believe most DEC terminals worked), where the text is just dimmed. Looking at the two screen shots you can see the pros and cons of each approach. In the "Christmas Card", the blinking lights on the tree clearly look better when they just dim, instead of disappearing. The BBS advert, on the other hand, is completely dependent on the PC style to produce the changing text effect.

blinktest

Having grown up with BBSes, I'm kind of partial to the PC style, but looking at things practically, I do think the DEC style is better. If you actually wanted to use blink to draw attention to some text, it's way better if you can still actually read that text. That's not so easy if it's constantly turning invisible, unless you also have a high blink rate, but that can be really annoying. For general text, a slow, subtle blink effect does seem more appropriate.

That said, I don't know of any other terminal emulators that actually implement the DEC style, so if we think it's important being compatible with other terminals emulators (rather than the actual terminals), then that's a factor to consider as well.

Ideally we'd one day have different compatibility modes, so we could actually support both options, but for now I do think we need to pick one to start with. I can't make up my mind which I prefer, so I'm happy to go with whatever the general consensus is here.

@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Aug 25, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 25, 2020
@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Aug 25, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 25, 2020
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Aug 25, 2020
@KalleOlaviNiemitalo
Copy link

This should perhaps check SystemParametersInfo SPI_GETCLIENTAREAANIMATION, whose description specifically mentions blinking. The blinking frequency in the sample GIF is only one change per second, though.

@j4james
Copy link
Collaborator Author

j4james commented Aug 25, 2020

The current implementation is using the existing timer from the CursorBlinker class, and that's linked to the system cursor blink rate (see GetCaretBlinkTime). That seemed to me like a reasonable proxy for a user's blink rate preference. I wouldn't be opposed to adding a SPI_GETCLIENTAREAANIMATION check on top of that though if there's any concern that this might still be problematic for some users.

I should also mention that my current implementation only blinks at half the cursor rate (which is more or less what a DEC terminal would do), so in the time it takes the cursor to blink on and off, the blinking attribute would only have rendered half a cycle (i.e. it would either be on the whole time, or off the whole time). I think that works nicely for the DEC style of blinking, but may be too slow for a PC style blink.

@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 26, 2020
@WSLUser
Copy link
Contributor

WSLUser commented Aug 27, 2020

For accessibility purposes, we will need to inherit the OS cursor blink rate configured in "Keyboard Properties" as blinking too fast can cause visual impairment but most users will not want it going too slow either. We can define the blink rate ourselves but I think OS inheritance will be better.

@j4james
Copy link
Collaborator Author

j4james commented Aug 27, 2020

For accessibility purposes, we will need to inherit the OS cursor blink rate configured in "Keyboard Properties"

I just said the current implementation is using the system cursor blink rate (GetCaretBlinkTime). Is that not the same thing?

@WSLUser
Copy link
Contributor

WSLUser commented Aug 27, 2020

I honestly have no clue. But I don't believe conhost is responsible for the keyboard settings and properties so I just wanted to put that out there. Since only the Terminal team have access to Windows source code, they can tell if it calls the same code. I do think we should document how to access the setting when this feature is implemented.

@zadjii-msft
Copy link
Member

Without even looking at OS code, this doc would seem to attest that @j4james's assertion is correct.

@jdebp
Copy link

jdebp commented Aug 29, 2020

Personally I like the DEC style, even though most of the home computers and personal computers that I've used have (in text mode) used the IBM MDA style of just AND-gating the foreground pixels, and even though there's the argument that Windows Terminal should stick with its PC heritage here. Note however that the EGA in 16-colour graphics mode blinked pixels by switching between two sets of 8 palette entries, and could implement DEC-style blinking, so the PC heritage is not quite as one-sided as one might think.

Also note the existence of DECBBSM, which is DEC private mode 116. If off, only the foreground pixels change intensity. If on, both foreground and background pixels change intensity.

The far more important configuration option that you'll have to set up is the ability to turn blinking off. Observe that the ability to disable the blink graphic rendition for text is a selectable option in terminal emulators such as (among others) PowerTerm WBT, Reflection ZFE, Foxit KoalaTerm, Terminal.app in MacOS (Apple doco), iTerm2, Konsole, tilix, and PuTTY.

@j4james
Copy link
Collaborator Author

j4james commented Aug 29, 2020

Personally I like the DEC style,

Thanks for the feedback. I'm leaning that way too at the moment.

Also note the existence of DECBBSM, which is DEC private mode 116. If off, only the foreground pixels change intensity. If on, both foreground and background pixels change intensity.

Yeah thanks, I am aware of that, but I'm not looking to do anything that fancy yet. I'm just trying to get us to the level of a basic VT100 terminal for now.

The far more important configuration option that you'll have to set up is the ability to turn blinking off.

In the current implementation you can at least turn it off via the cursor blink settings, and if it's easy I might also add in the animation option that KalleOlaviNiemitalo was suggesting. If anyone still has complaints after that, we can always add more options, but I suspect the reality is that most people will never encounter a blinking attribute unless they're actively looking for one. Do you have any examples of software using blinking that some people might find annoying?

@ghost ghost added the In-PR This issue has a related PR label Aug 31, 2020
@Qix-
Copy link

Qix- commented Sep 10, 2020

Would SGR 6 (fast blink) be supported here too? Perhaps at a perfect 2x rate? Both are turned off by SGR 25 if I understand correctly. I'm not sure the origin of SGR 6, but it's something that has always been listed in the various references I've seen.

EDIT: I see https://github.com/microsoft/terminal/pull/7490/files#diff-f30650c558f3ea3f151ac2c2bea3d9ebR135 already detects it but it's not being handled. This would be a really neat feature to have, given the emergence of "ansi dashboards" nowadays. It'd also set the windows terminal ahead of many (most) other emulators, too.

@ghost ghost closed this as completed in #7490 Sep 21, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Sep 21, 2020
ghost pushed a commit that referenced this issue Sep 21, 2020
This PR adds support for the _blink_ graphic rendition attribute. When a
character is output with this attribute set, it "blinks" at a regular
interval, by cycling its color between the normal rendition and a dimmer
shade of that color.

The majority of the blinking mechanism is encapsulated in a new
`BlinkingState` class, which is shared between the Terminal and Conhost
implementations. This class keeps track of the position in the blinking
cycle, which determines whether characters are rendered as normal or
faint. 

In Windows Terminal, the state is stored in the `Terminal` class, and in
Conhost it's stored in the `CONSOLE_INFORMATION` class. In both cases,
the `IsBlinkingFaint` method is used to determine the current blinking
rendition, and that is passed on as a parameter to the
`TextAttribute::CalculateRgbColors` method when these classes are
looking up attribute colors.

Prior to calculating the colors, the current attribute is also passed to
the `RecordBlinkingUsage` method, which keeps track of whether there are
actually any blink attributes in use. This is used to determine whether
the screen needs to be refreshed when the blinking cycle toggles between
the normal and faint renditions.

The refresh itself is handled by the `ToggleBlinkingRendition` method,
which is triggered by a timer. In Conhost this is just piggybacking on
the existing cursor blink timer, but in Windows Terminal it needs to
have its own separate timer, since the cursor timer is reset whenever a
key is pressed, which is not something we want for attribute blinking.

Although the `ToggleBlinkingRendition` is called at the same rate as the
cursor blinking, we actually only want the cells to blink at half that
frequency. We thus have a counter that cycles through four phases, and
blinking is rendered as faint for two of those four. Then every two
cycles - when the state changes - a redraw is triggered, but only if
there are actually blinking attributes in use (as previously recorded).

As mentioned earlier, the blinking frequency is based on the cursor
blink rate, so that means it'll automatically be disabled if a user has
set their cursor blink rate to none. It can also be disabled by turning
off the _Show animations in Windows_ option. In Conhost these settings
take effect immediately, but in Windows Terminal they only apply when a
new tab is opened.

This PR also adds partial support for the `SGR 6` _rapid blink_
attribute. This is not used by DEC terminals, but was defined in the
ECMA/ANSI standards. It's not widely supported, but many terminals just
it implement it as an alias for the regular `SGR 5` blink attribute, so
that's what I've done here too.

## Validation Steps Performed

I've checked the _Graphic rendition test pattern_ in Vttest, and
compared our representation of the blink attribute to that of an actual
DEC VT220 terminal as seen on [YouTube]. With the right color scheme
it's a reasonably close match.

[YouTube]: https://www.youtube.com/watch?v=03Pz5AmxbE4&t=1m55s

Closes #7388
DHowett pushed a commit that referenced this issue Sep 21, 2020
This PR adds support for the _blink_ graphic rendition attribute. When a
character is output with this attribute set, it "blinks" at a regular
interval, by cycling its color between the normal rendition and a dimmer
shade of that color.

The majority of the blinking mechanism is encapsulated in a new
`BlinkingState` class, which is shared between the Terminal and Conhost
implementations. This class keeps track of the position in the blinking
cycle, which determines whether characters are rendered as normal or
faint.

In Windows Terminal, the state is stored in the `Terminal` class, and in
Conhost it's stored in the `CONSOLE_INFORMATION` class. In both cases,
the `IsBlinkingFaint` method is used to determine the current blinking
rendition, and that is passed on as a parameter to the
`TextAttribute::CalculateRgbColors` method when these classes are
looking up attribute colors.

Prior to calculating the colors, the current attribute is also passed to
the `RecordBlinkingUsage` method, which keeps track of whether there are
actually any blink attributes in use. This is used to determine whether
the screen needs to be refreshed when the blinking cycle toggles between
the normal and faint renditions.

The refresh itself is handled by the `ToggleBlinkingRendition` method,
which is triggered by a timer. In Conhost this is just piggybacking on
the existing cursor blink timer, but in Windows Terminal it needs to
have its own separate timer, since the cursor timer is reset whenever a
key is pressed, which is not something we want for attribute blinking.

Although the `ToggleBlinkingRendition` is called at the same rate as the
cursor blinking, we actually only want the cells to blink at half that
frequency. We thus have a counter that cycles through four phases, and
blinking is rendered as faint for two of those four. Then every two
cycles - when the state changes - a redraw is triggered, but only if
there are actually blinking attributes in use (as previously recorded).

As mentioned earlier, the blinking frequency is based on the cursor
blink rate, so that means it'll automatically be disabled if a user has
set their cursor blink rate to none. It can also be disabled by turning
off the _Show animations in Windows_ option. In Conhost these settings
take effect immediately, but in Windows Terminal they only apply when a
new tab is opened.

This PR also adds partial support for the `SGR 6` _rapid blink_
attribute. This is not used by DEC terminals, but was defined in the
ECMA/ANSI standards. It's not widely supported, but many terminals just
it implement it as an alias for the regular `SGR 5` blink attribute, so
that's what I've done here too.

## Validation Steps Performed

I've checked the _Graphic rendition test pattern_ in Vttest, and
compared our representation of the blink attribute to that of an actual
DEC VT220 terminal as seen on [YouTube]. With the right color scheme
it's a reasonably close match.

[YouTube]: https://www.youtube.com/watch?v=03Pz5AmxbE4&t=1m55s

Closes #7388

(cherry picked from commit d1671a0)
@ghost
Copy link

ghost commented Sep 22, 2020

🎉This issue was addressed in #7490, which has now been successfully released as Windows Terminal Preview v1.4.2652.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants