microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.21k stars 8.27k forks source link

Cursor attributes cannot be set with ANSI VT Sequences before any user input #16749

Open rforzachamp821 opened 7 months ago

rforzachamp821 commented 7 months ago

Windows Terminal version

1.20.10303.0

Windows build number

10.0.22631.0

Other Software

N/A (the code in the repro steps should create the effect)

Steps to reproduce

  1. Compile something along the lines of the following C++ code:

    #include <iostream>
    int main() {
    std::cout << "\033[2 q";
    std::cin.get();
    std::cout << "\033[2 q";
    std::cin.ignore(99999999, '\n'); // you can use INT_MAX in place of the spammed 9s
        std::cin.get();
    
    return 0;
    }

    This code sets the new cursor attributes, waits for an ENTER key, sets the new cursor attributes again, and waits for an enter key again.

  2. Run it by clicking on the generated executable, NOT from the VS debug console (if using VS)

  3. The first time the program requests an ENTER key, there should be the default cursor attributes. Once the ENTER key is pressed, only then should the cursor attributes set.

https://github.com/microsoft/terminal/assets/66303587/93491fa7-daf8-4c87-aea6-e43df0ea6eaf

Expected Behavior

Cursor attributes should have been set on command before any user input.

Actual Behavior

Cursor attribute setting was ignored, until user inputted an ENTER character. Only then could any of the necessary cursor attribute VT sequences work (e.g "\033[2 q").

rforzachamp821 commented 7 months ago

Additionally, I'm not quite sure why the issue doesn't occur with the VS debug console. Does it set/reset something? I don't know for sure.

DHowett commented 7 months ago

Just a quick check before I dig in --

Are you enabling the ENABLE_VIRTUAL_TERMINAL_PROCESSING console mode somewhere?

rforzachamp821 commented 7 months ago

Ah. I have attempted to enable it before, yes. However, that hasn't really changed anything. To make things clear, I tested this code to no avail:

#include <iostream>
#include <Windows.h>

// A function that checks for VT Terminal Sequence support, enables them if support is there
// and disables them if support isn't there. Returns true/false respectively, too.
bool EnableVTMode()
{
    // Set output mode to handle virtual terminal sequences
    HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
    if (hOut == INVALID_HANDLE_VALUE)
    {
        return false;
    }

    DWORD dwMode = 0;
    if (!GetConsoleMode(hOut, &dwMode))
    {
        return false;
    }

    dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
    if (!SetConsoleMode(hOut, dwMode))
    {
        return false;
    }
    return true;
}

int main() {
    if (EnableVTMode()) {
        std::cout << "Success Enable VT Sequences\n";
    }
    else std::cout << "Failed Enable VT Sequences\n";

    std::cout << "\033[2 q";
    std::cin.get();
    std::cout << "\033[2 q";
    std::cin.ignore(INT_MAX, '\n');
    std::cin.get();

    return 0;
}

I think the Windows Terminal sets it automatically to default, though, as it accepts VT sequences without the flag manually set.

lhecker commented 7 months ago

Both in C and in C++ output is line-buffered by default (= only flushed to the terminal on newlines or when the buffer is full). When using std::cout you can use std::flush. In your case:

std::cout << "\033[2 q" << std::flush;
std::cin.get();
std::cout << "\033[2 q" << std::flush;

I'm pretty sure this will fix your issue. Feel free to close it if it works. 🙂

rforzachamp821 commented 7 months ago

Once again, after trying a couple of times, it still hasn't managed to work. Flushing the buffer after every output shouldn't be the cause of the issue, anyway, as i'm pretty sure it flushes automatically when the std::cin.get() line is hit.

lhecker commented 7 months ago

Oh I completely misunderstood the question anyways. 🤦

rforzachamp821 commented 7 months ago

Ah, all good. I was so confused as to how that would have been the fix 😂

rforzachamp821 commented 7 months ago

After a little bit of messing around, it seems like the issue does NOT occur when the executable is set as a profile on the terminal, and then executed from the 'V' drop-down menu next to the '+' icon in the tab row. This issue is getting more and more confusing day-by-day.

tusharsnx commented 7 months ago

In my testing, when running the exe from the explorer repeatedly, the cursor sometimes does change to block, but sometimes it doesn't. So, it's not even consistent.

WT Stable (v1.18): Cursor is not Steady Block in 5 of 10 runs. WT Preview (v1.20): Cursor is not Steady Block in 0 of 10 runs.

So, @rforzachamp821 if you don't mind, can you try running the exe by setting WT Preview as the default terminal?

image

rforzachamp821 commented 7 months ago

Hey, @tusharsnx , Just to confirm, I have been testing with WT Preview set to default. I have confirmed this before testing, before even creating this issue. I am testing using the latest build of WT Preview, as indicated by the version number listed in the issue description.

rforzachamp821 commented 7 months ago

After even more testing, it seems like the new AtlasEngine text renderer does not fail to change the cursor attributes before user input. No matter how many tries I have done to reproduce the bug with that text renderer, it works flawlessly. Therefore, the bug seems to be with the default text renderer (I'm not sure of its name). As a result, I will not close this issue yet because the bug still exists.

rforzachamp821 commented 7 months ago

Any updates?

j4james commented 7 months ago

As far as I can see, this is a concurrency issue in the terminal startup process, so it's likely to be timing dependent. The conpty connection thread is started before the terminal has fully initialized, so it can end up processing the DECSCUSR escape sequence before the terminal has had a chance to initialize the default cursor style.

Have a look at the TermControl::_InitializeTerminal method. It has a call to _core.Initialize (which starts the connection thread with _connection.Start()), and then several lines later we call _UpdateAppearanceFromUIThread, which sets the default cursor style.

If we receive a DECSCUSR escape sequence between the _core.Initialize call and the _UpdateAppearanceFromUIThread call, that style change is going to be overridden. And I suspect there are other escape sequences that could be affected in a similar way, e.g. setting the color palette.

rforzachamp821 commented 7 months ago

I see. I think it would be possible for the terminal to process all style-based ANSI escape sequences after the call to _UpdateAppearanceFromUIThread, right? For example, I can see some code before the function call that sets the caret blinking. Something like that, but after the function call?