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.

Sign up for GitHub

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

Jump to bottom

Teach the renderer to keep thread alive if engine requests it #9091

Merged
merged 4 commits into from
Feb 10, 2021
Merged

Teach the renderer to keep thread alive if engine requests it #9091

merged 4 commits into from
Feb 10, 2021

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented Feb 10, 2021 •
edited by DHowett
Loading

Teaches renderer base to keep thread alive if engine requests it.
DxEngine now requests it if shaders are on.

  • The render engine interface now has a true/false to return whether the
    specific renderer wants another frame to immediately follow up. The
    renderer base will ask for this information as it ends the paint on
    any particular engine (which is the time where invalid regions are
    typically cleaned up) and just poke the render thread the same as if
    an invalidation request came in from outside of render-land. That will
    trigger the render thread to just keep moving in the same way as any
    other invalidation.

Validation Steps Performed

  • Actually built it
  • Actually try it

I promised this in #8994

@miniksa
Make renderer base tell the thread to tick again if an engine reports…
ae9353c 
… that it wants it so continuous draw shaders can keep drawing.
@miniksa miniksa added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. zPreview-Service-Consider labels Feb 10, 2021
@miniksa miniksa self-assigned this Feb 10, 2021
zadjii-msft
zadjii-msft approved these changes Feb 10, 2021
View reviewed changes
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is GREAT. I don't think there's a way to be sure if a shader is actually using the Time variable or not - we'd somehow need to inspect the compiled shader to see if the variable was ever referenced? There's no way that's possible.

Lets :shipit:

src/renderer/base/renderer.cpp
// If the engine tells us it really wants to redraw immediately,
// tell the thread so it doesn't go to sleep and ticks again
// at the next opportunity.
if (pEngine->RequiresContinuousRedraw())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my gosh this is so clever, I love it

@miniksa
Copy link
Member Author

miniksa commented Feb 10, 2021

TODO

  • See if we can turn this off if a shader doesn't use time variable or at least for the in-built retro one?
    • The answer to this one is that we can turn it off for the in-built one, but I cannot see a way to detect if the shader is actually using the time parameter. Perhaps in the future if this becomes an extension point instead of just a set-path-to-HLSL-file, we can have someone declare they need the timer tick to differentiate. Or someone will have another idea of how the HLSL can return that info in the future.

@miniksa miniksa marked this pull request as ready for review February 10, 2021 18:56
@DHowett
Copy link
Member

DHowett commented Feb 10, 2021

nit: reword title to be imperative; pretend it completes the sentence "this commit will"

@DHowett
Copy link
Member

DHowett commented Feb 10, 2021

Moved from commit body

  • samples/PixelShaders/Animate_breathe.hlsl breathe
  • samples/PixelShaders/Animate_scan.hlsl scanline

@DHowett DHowett changed the title Teaches renderer base to keep thread alive if engine requests it Teach the renderer to keep thread alive if engine requests it Feb 10, 2021
@DHowett
Copy link
Member

DHowett commented Feb 10, 2021

Clever as.

@DHowett DHowett merged commit 3b24781 into main Feb 10, 2021
@DHowett DHowett deleted the dev/miniksa/tick branch February 10, 2021 19:24
DHowett pushed a commit that referenced this pull request Feb 10, 2021
@miniksa @DHowett
Teach the renderer to keep thread alive if engine requests it (#9091)
48543bb 
Teaches renderer base to keep thread alive if engine requests it.
`DxEngine` now requests it if shaders are on.

- The render engine interface now has a true/false to return whether the
  specific renderer wants another frame to immediately follow up. The
  renderer base will ask for this information as it ends the paint on
  any particular engine (which is the time where invalid regions are
  typically cleaned up) and just poke the render thread the same as if
  an invalidation request came in from outside of render-land. That will
  trigger the render thread to just keep moving in the same way as any
  other invalidation.

## Validation Steps Performed
- [x] Actually built it
- [x] Actually try it

I promised this in #8994

(cherry picked from commit 3b24781)
DHowett added a commit that referenced this pull request Feb 11, 2021
@DHowett
Merged PR 5676764: Migrate OSS up to 16d00a6
38da2ff 
Dustin L. Howett (3)
* Move CharToKeyEvents (and friends) into InteractivityBase (GH-9106)
* Update Cascadia Code to 2102.03 (GH-9088)
* verison: bump to 1.7 on main

Josh Soref (1)
* ci: update to Spell check to 0.0.17a (CC-9014)

Leonard Hecker (3)
* Fixed GH-5205: Ctrl+Alt+2 doesn't send ^[^@ (CC-5272)
* Fix issues in tests.xml and OpenConsole.psm1 (CC-9011)
* Fix GH-8458: Handle all Ctrl-key combinations (CC-8870)

Mike Griese (1)
* Add support for running a commandline in another WT window (GH-8898)

Michael Niksa (1)
* Teach the renderer to keep thread alive if engine requests it (GH-9091)

Lachlan Picking (1)
* Fix shader time input (CC-8994)

PankajBhojwani (1)
* Separate runtime TerminalSettings from profile-TerminalSettings (CC-8602)

Chester Liu (2)
* Add support for paste filtering and bracketed paste mode (CC-9034)
* Add support for chaining OSC 10-12 (CC-8999)

Related work items: MSFT-31692939
@ghost
Copy link

ghost commented Feb 11, 2021

🎉Windows Terminal Preview v1.6.10412.0 has been released which incorporates this pull request.:tada:

Handy links:

@Nacimota Nacimota mentioned this pull request Feb 11, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

Assignees

@miniksa miniksa

Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@miniksa @DHowett @zadjii-msft