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

Bug Report Flickering background if background image is set #1103

Closed
Summon528 opened this issue Jun 1, 2019 · 9 comments
Closed

Bug Report Flickering background if background image is set #1103

Summon528 opened this issue Jun 1, 2019 · 9 comments
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Milestone

Comments

@Summon528
Copy link
Contributor

Environment

Windows build number: Microsoft Windows [Version 10.0.18362.145]
Windows Terminal version (if applicable): 880272c74

Any other software?

Steps to reproduce

  1. Set terminal to light theme
  2. Set a dark background image in a profile
  3. Open a new tab with that profile

Expected behavior

Actual behavior

Screen flickered
Imgur

Related to #853, probably because dxrender layer overwrites background image for a little bit amount of time

@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 Jun 1, 2019
@DHowett
Copy link
Member

DHowett commented Jun 1, 2019

This could also be #1082?

@Summon528
Copy link
Contributor Author

Actually I believe not, that one is happening when acrylic is on and background image is off. This one is the opposite. Also the draft fix (#1092) does not fix this one

@DHowett-MSFT
Copy link
Contributor

Perhaps the real issue in both #1082 and this issue is that the control is added to the UI tree before the background change is propagated into the renderer.

void TermControl::_BackgroundColorChanged(const uint32_t color)
{
_root.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, color]() {

This function dispatches the background update into the settings/down into the renderer only after the root control's dispatcher has a free cycle, which will almost certainly be after it's been installed into the TermApp's content grid.

Good catch, @Summon528.

@d-bingham If you don't mind terribly, would you consider this for #1092 as well? Let's try to knock out all the flashing bugs at once.

@d-bingham
Copy link
Contributor

Easy enough to add to #1092, but in fiddling with it for a few minutes I don't have much of a reproduction -- @Summon528 can you provide a profiles.json that shows it?

@DHowett-MSFT
Copy link
Contributor

Actually, yeah, I'm trying with this and I can't repro it:

        {
            "guid": "{dec46506-2b06-458e-975d-5f04d0b9927f}",
            "name": "Repro",
            "colorScheme": "URXVT",
	    "background": "#ff0000",
	    "backgroundImage": "C:\\windows\\system32\\FeatureToastBulldogImg.png",
            "historySize": 9001,
            "snapOnInput": true,
            "cursorColor": "#de00de",
            "cursorShape": "filledBox",
            "commandline": "cmd.exe",
            "fontFace": "Consolas",
            "fontSize": 12,
            "acrylicOpacity": 0.85,
            "useAcrylic": false,
            "closeOnExit": true,
            "padding": "8"
        },

maybe it needs to be a large image/something slow to load/something from a remote server? I see some very minor flickering when I use an image directly from the web.

@d-bingham
Copy link
Contributor

I haven't been able to get the flicker on a new tab to happen reliably (with using an image over the network to introduce some sort of delay), but can get it to happen reliably by opening a tab and then closing it (causing the flicker to happen after the tab close but before the original tab is re-displayed).

From doing some testing the flicker is the default background brush (which will be white-ish in the Windows light theme and black-ish int he Windows dark theme) being drawn while it waits for the image to be loaded into the brush -- so if you've got a dark image in the dark theme it's not particularly noticeable, but it is definitely so with a dark image and a light theme.

Working on getting the async handling of image loading working a bit better by setting up a solid color brush on the background (using the profile's default background color) while waiting for the image to load, and keeping the image itself in the profile instead of each individual tab, so it can be reused effectively.

@Summon528
Copy link
Contributor Author

Summon528 commented Jun 2, 2019

maybe it needs to be a large image/something slow to load/something from a remote server? I see some very minor flickering when I use an image directly from the web.

I guess this is the key, cause I was loading a gif from the web 😜. I can also repro reliably even if the gif is in my local machine

        {
            "startingDirectory": "%USERPROFILE%",
            "guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
            "name": "PowerShell",
            "background": "#012456",
            "colorScheme": "Campbell",
            "historySize": 9001,
            "snapOnInput": true,
            "cursorColor": "#FFFFFF",
            "cursorShape": "bar",
            "commandline": "powershell.exe",
            "fontFace": "Fira Code",
            "fontSize": 12,
            "acrylicOpacity": 0.5,
            "useAcrylic": false,
            "closeOnExit": true,
            "padding": "0, 0, 0, 0",
            "icon": "ms-appx:///ProfileIcons/{61c54bbd-c2c6-5271-96e7-009a87ff44bf}.png",
            "backgroundImage": "https://i.imgur.com/Yaf1Axq.gif",
            "backgroundImageOpacity": 1
        }

@DHowett-MSFT DHowett-MSFT added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 3, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 3, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Jun 19, 2019
@zadjii-msft zadjii-msft added the Priority-3 A description (P3) label Jan 22, 2020
@zadjii-msft zadjii-msft modified the milestones: Terminal v2.0, 22H2 Jan 4, 2022
@zadjii-msft
Copy link
Member

FWIW, I can still repro this on 1.14, but I'm not sure there's much else that can be done here. We now at least open the control with the configured background color, but the image still takes a few frames to load. I don't really think it's a good idea to hold off creation of the control until the image is loaded. I don't think there's a way to get some sort of placeholder BG color from the image before it loads, cause obviously we need to load the image first.

@Summon528 you mind if we close this one out now?

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 8, 2022
@Summon528
Copy link
Contributor Author

Yeah sure, it's way less annoying now

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 9, 2022
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

6 participants