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

Terminal crashes when graphics cards timeout and graphics driver does soft reset #2265

Closed
miniksa opened this issue Aug 5, 2019 · 3 comments · Fixed by #2830
Closed

Terminal crashes when graphics cards timeout and graphics driver does soft reset #2265

miniksa opened this issue Aug 5, 2019 · 3 comments · Fixed by #2830
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone

Comments

@miniksa
Copy link
Member

miniksa commented Aug 5, 2019

When the graphics cards do a soft reset, the terminal crashes. I need to make sure the DX code is more robust.

Also lol at getting my own issue closed for an empty description.

@miniksa miniksa added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Severity-Crash Crashes are real bad news. Product-Terminal The new Windows Terminal. labels Aug 5, 2019
@miniksa miniksa self-assigned this Aug 5, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 5, 2019
@ghost ghost closed this as completed Aug 5, 2019
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 5, 2019
@ghost
Copy link

ghost commented Aug 5, 2019

Hi! Thanks for attempting to open an issue. Unfortunately, you didn't write anything in the body which makes it impossible to understand your concern. You are welcome to fix up the issue and try again by opening another issue with the body filled out.

@miniksa miniksa reopened this Aug 5, 2019
@miniksa miniksa removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 5, 2019
@richardszalay
Copy link
Contributor

richardszalay commented Aug 5, 2019

FWIW, this does not reproduce using CTRL+SHIFT+WIN+B (for me, at least)

@zadjii-msft zadjii-msft added the Priority-1 A description (P1) label Sep 13, 2019
DHowett-MSFT pushed a commit that referenced this issue Sep 20, 2019
If _PaintFrameForEngine returns E_PENDING, we'll give it another two
tries to get itself straight. If it continues to fail, we'll take down
the application.

We observed that the DX renderer was failing to present the swap chain
and failfast'ing when it did so; however, there are some errors from
which DXGI guidance suggests we try to recover. We'll now return
E_PENDING (and destroy our device resources) when we hit those errors.

Fixes #2265.
@ghost ghost added the In-PR This issue has a related PR label Sep 20, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Sep 23, 2019
DHowett-MSFT pushed a commit that referenced this issue Sep 23, 2019
If _PaintFrameForEngine returns E_PENDING, we'll give it another two
tries to get itself straight. If it continues to fail, we'll take down
the application.

We observed that the DX renderer was failing to present the swap chain
and failfast'ing when it did so; however, there are some errors from
which DXGI guidance suggests we try to recover. We'll now return
E_PENDING (and destroy our device resources) when we hit those errors.

Fixes #2265.
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements 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 23, 2019
@ghost
Copy link

ghost commented Sep 24, 2019

🎉This issue was addressed in #2830, which has now been successfully released as Windows Terminal Preview v0.5.2661.0.:tada:

Handy links:

DHowett-MSFT pushed a commit that referenced this issue Sep 26, 2019
If _PaintFrameForEngine returns E_PENDING, we'll give it another two
tries to get itself straight. If it continues to fail, we'll take down
the application.

We observed that the DX renderer was failing to present the swap chain
and failfast'ing when it did so; however, there are some errors from
which DXGI guidance suggests we try to recover. We'll now return
E_PENDING (and destroy our device resources) when we hit those errors.

Fixes #2265.

(cherry picked from commit 277acc3)
DHowett-MSFT pushed a commit that referenced this issue Sep 26, 2019
- Remember last-used string in the Find dialog in conhost (GH-2845)

  (cherry picked from commit bfb1484)

- Bugfix: TextBuffer Line Wrapping from VTRenderer (GH-2797)

  Change VT renderer to do erase line instead of a ton of erase chars

  (cherry picked from commit 8afc5b2)

- Add some retry support to Renderer::PaintFrame (GH-2830)

  If _PaintFrameForEngine returns E_PENDING, we'll give it another two
  tries to get itself straight. If it continues to fail, we'll take down
  the application.

  We observed that the DX renderer was failing to present the swap chain
  and failfast'ing when it did so; however, there are some errors from
  which DXGI guidance suggests we try to recover. We'll now return
  E_PENDING (and destroy our device resources) when we hit those errors.

  Fixes GH-2265.

  (cherry picked from commit 277acc3)

- Enable VT processing by default for ConPTY (GH-2824)

  This change enables VT processing by default for _all_ conpty clients. See
  GH-1965 for a discussion on why we believe this is a righteous change.

  Also mentioned in the issue was the idea of only checking the
  `VirtualTerminalLevel` reg key in the conpty startup. I don't think this would
  be a more difficult change, looks like all we'd need is a simple
  `reg.LoadGlobalsFromRegistry();` call instead of this change.

  **Validation Steps Performed**
  Manually launched a scratch app in both the terminal and the console. The
  console launch's output mode was 0x3, and the terminal's was 0x7. 0x4 is the `
  ENABLE_VIRTUAL_TERMINAL_PROCESSING` flag, which the client now had by default
  in the Terminal.

  Closes GH-1965

  (cherry picked from commit 1c412d4)

- Remove unwanted DECSTBM clipping (GH-2764)

  The `DECSTBM` margins are meant to define the range of lines within which
  certain vertical scrolling operations take place. However, we were applying
  these margin restrictions in the `ScrollRegion` function, which is also used in
  a number of places that shouldn't be affected by `DECSTBM`.

  This includes the `ICH` and `DCH` escape sequences (which are only affected by
  the horizontal margins, which we don't yet support), the
  `ScrollConsoleScreenBuffer` API (which is public Console API, not meant to be
  affected by the VT terminal emulation), and the `CSI 3 J` erase scrollback
  extension (which isn't really scrolling as such, but uses the `ScrollRegion`
  function to manipulate the scrollback buffer).

  This commit moves the margin clipping out of the `ScrollRegion` function, so it
  can be applied exclusively in the places that need it.

  While testing, I also noticed that one of the `ScrollRegion` calls in the
  `AdjustCursorPosition` function was not setting the horizontal range correctly
  - the scrolling should always affect the full buffer width rather than just the
    viewport width - so ...

Related work items: #23507749, #23563809, #23563819, #23563837, #23563841, #23563844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants