microsoft / terminal

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

Profile icons not displayed when initialPosition is set to a value, that opens terminal on a second monitor #14954

Open its-csc opened 1 year ago

its-csc commented 1 year ago

Windows Terminal version

1.16.10261.0

Windows build number

10.0.19044.2604

Other Software

No response

Steps to reproduce

You need two monitors. The monitor on the left ist the primary display, the right monitor is the second display.

icons-missing

Expected Behavior

I expected to see all profile icons, even if the terminal starts on a second screen.

icons-shown

In the settings window, all icons are always diplayed correctly.

Actual Behavior

icons-missing

zadjii-msft commented 1 year ago

oh my god you've totally figured out the root cause of #14339, haven't you?

@awakecoding you were seeing that too, right? Did you have two monitors?

awakecoding commented 1 year ago

oh my god you've totally figured out the root cause of #14339, haven't you?

@awakecoding you were seeing that too, right? Did you have two monitors?

I usually have 3 monitors with mixed DPI: my laptop as primary display in the middle with high dpi, and two external displays on the sides. This is guaranteed to trigger a lot of bugs in all software (you wouldn't believe how the Windows 11 Explorer still has issues with it)

its-csc commented 1 year ago

@zadjii-msft Oh sorry for duplicate issue. I was looking for one and couldn't find one. Hope this helps to fix it. 😃

To my scenario: First, internal laptop monitor: FullHD display, scale settings set to 125%. Second, external, monitor: WQHD display (2560x1440), scale settings set to 100%.

zadjii-msft commented 1 year ago

No worries! Honestly, I might close the other one in favor of this thread. The other thread is all "it no worky", while this thread actually has a root cause and concrete repro steps.

awakecoding commented 10 months ago

I've managed to find the conditions required to hit this bug 100% of the time, and it happens with the official Microsoft Windows Terminal zip package. You need mixed DPI displays, which is easy to get with a high DPI laptop display and external low DPI monitors like this:

mixed-dpi-displays

Download and extract the Windows Terminal zip, and have it ready in the file explorer. Move the file explorer window to either one of the low DPI displays in such a configuration and double-click WindowsTerminal.exe or wt.exe and you'll have missing icons. Move the file explorer to the high DPI display in the middle and do the same, and it'll always render those icons.

Here's an interesting twist: if I change my laptop display to 1920x1080 (low dpi) then the issue doesn't manifest itself. It really needs a primary high DPI display for the bug to occur only when the initial display is one of the non-primary DPI displays. Moving the file explorer window to the display you want to use appears sufficient to make that display the initial one when launching Windows Terminal from there.

awakecoding commented 10 months ago

I just noticed something interesting: in my case, only the icons defined using ms-appx:// URLs are affected:

image

Those icons are from the ProfileIcons directory alongside Windows Terminal. In that screenshot, Ubuntu 22.04 uses the generic Linux profile icon (ms-appx:///ProfileIcons/{9acb9455-ca41-5af7-950f-6bca1bc9722f}.png), while Ubuntu 20.04 uses an external icon with an HTTPS URL (https://assets.ubuntu.com/v1/49a1a858-favicon-32x32.png). This distinction may be important to find the root cause of the icon loading bug.

awakecoding commented 10 months ago

I figured that I could set an explicit initial position for Windows Terminal instead of letting Windows decide, making it much easier to reproduce the issue. The profile icon files are definitely loaded from disk, and we can see the call stack deep into Windows.UI.Xaml.dll from Process Monitor on those:

image (1)

I've managed to record a time travel trace of Windows Terminal and put breakpoints in that call stack, but I couldn't really figure out anything obvious. I'm thinking the images are loaded from the files but the issue would be something to do with UI threads and rendering. There's a lot of sketchy async stuff with comments about certain things needing to run on specific threads.

https://github.com/microsoft/terminal/blob/17867af5349a66fa8121d1c930dd9587e45f5117/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp#L189

    // Method Description:
    // - Attempt to convert something into another type. For the
    //   IconPathConverter, we support a variety of icons:
    //    * If the icon is a path to an image, we'll use that.
    //    * If it isn't, then we'll try and use the text as a FontIcon. If the
    //      character is in the range of symbols reserved for the Segoe MDL2
    //      Asserts, well treat it as such. Otherwise, we'll default to a Sego
    //      UI icon, so things like emoji will work.
    // - MUST BE CALLED ON THE UI THREAD.
    // Arguments:
    // - value: the input object to attempt to convert into an IconSource.
    // Return Value:
    // - Visible if the object was a string and wasn't the empty string.
    Foundation::IInspectable IconPathConverter::Convert(const Foundation::IInspectable& value,
                                                        const Windows::UI::Xaml::Interop::TypeName& /* targetType */,
                                                        const Foundation::IInspectable& /* parameter */,
                                                        const hstring& /* language */)
    {
        const auto& iconPath = winrt::unbox_value_or<winrt::hstring>(value, L"");
        return _getIconSource<Controls::IconSource>(iconPath, false);
    }

https://github.com/microsoft/terminal/blob/17867af5349a66fa8121d1c930dd9587e45f5117/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp#L167

        if (!iconSource)
        {
            // Set the default IconSource to a BitmapIconSource with a null source
            // (instead of just nullptr) because there's a really weird crash when swapping
            // data bound IconSourceElements in a ListViewTemplate (i.e. CommandPalette).
            // Swapping between nullptr IconSources and non-null IconSources causes a crash
            // to occur, but swapping between IconSources with a null source and non-null IconSources
            // work perfectly fine :shrug:.
            typename BitmapIconSource<TIconSource>::type icon;
            icon.UriSource(nullptr);
            iconSource = icon;
        }

Are we sure it's all running in the correct thread for the icon loading, and what would be the easiest way to tell if it's wrong? What about this weird hack to avoid crashing by setting non-null icon sources? Could this be the cause of the problem?

jboekesteijn commented 3 weeks ago

I just noticed something interesting: in my case, only the icons defined using ms-appx:// URLs are affected [...]

Great, I 'fixed' the issue by manually choosing icons from external files, thanks to your comment :nerd_face: