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

Various Differential Drawing fixes for #5345 #5427

Merged
merged 28 commits into from
Apr 20, 2020
Merged

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

This is everything I've found in my attempt to resurrect #5345.

There are a bunch of new bugs and edge cases I've added to #5345 (comment). Largely, things are much better.

TermControl.cpp

  • Use the font size in DIPs for pointer movement events, mouse scrolling events
  • When the settings are reloaded, if we change the font size, resize the buffer to fit the current window.
  • Actually whenever the font size changes, make sure to refresh the size of the buffer for the new font size in this window.
  • Use the engine's curent scale when dealing with a swapchain size change to avoid a bug where only part of the biewport would get redrawn
    • (this might actually not be needed anymore)
  • When the DPI changes, guess what, the font size might change too. Resize the buffer if it did.
  • When we're trying to get the pointer position inside the terminal, use til to make our lives easier.

TSFInputControl.cpp

  • Use til in this method to make our life easier
  • Properly account for things that are in DIPs and things that aren't anymore.

ScreenInfo.cpp

  • Make sure to update the size of the viewport with the VtEngine as soon as a resize happens, otherwise the rest of the invalidates that happen in this frame are going to error out and cause tearing.

point.h

  • add a point(float, float) ctor.

rectangle.h

  • add a winrt Rect cast.

size.h

  • add a size(int, ptrdiff_t) ctor.
  • add a size(ptrdiff_t, int) ctor.
  • add a size(float, float) ctor.

DxRenderer.cpp

  • When we incalidate all, just mark this as a _firstFrame, and don't bother with Present1. We know it won't work. So don't.
  • Don't use _invalidMap.all() in PaintBackground, because it mostly won't work.

…might be able to do it without reverting the double resize
@DHowett-MSFT
Copy link
Contributor

I was wondering if the DIP calculation should have gone the other way in term control: transform the cursor position into raw pixels (dpx) instead of the font size into dips. @miniksa mentioned something about rounding issues, if I remember, and being able to map the cursor position literally into the actual pixels it hit might be safer? IDK. It makes the math clearer at the points at which the cursor position is used because we can have one helper (cursor position to physical) instead of having to apply the font size scale?

@DHowett-MSFT
Copy link
Contributor

For the record the WPF control is in DPX. We eventually will converge the logic as much as possible between the two (there’s no reason for us to have multiple implementations of selection, which requires position math, for example) but that’s post 1.0 stuff

@miniksa
Copy link
Member

miniksa commented Apr 20, 2020

I was wondering if the DIP calculation should have gone the other way in term control: transform the cursor position into raw pixels (dpx) instead of the font size into dips. @miniksa Michael Niksa FTE mentioned something about rounding issues, if I remember, and being able to map the cursor position literally into the actual pixels it hit might be safer? IDK. It makes the math clearer at the points at which the cursor position is used because we can have one helper (cursor position to physical) instead of having to apply the font size scale?

I mentally thought of it the other way with moving everything from DPI to DPX and doing it in DPX. But I'm willing to take whatever works at this point and clean it up in post.

This process has revealed a bit of a rough edge with point, size, and rectangle that I think we could mitigate down the road by either having a different form of those structures for float/double calculations or to specify the DIP/DPX-ness and scale factor of them when they're created so they can figure out the right thing for situations like this.

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 20, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 20, 2020
(cherry picked from commit ba1a8a34292180cae10c135743ecd94b1da6f286)
(cherry picked from commit 98cdbeca349acb55c5b5aef1d96206a8fa7b62e4)
@zadjii-msft
Copy link
Member Author

@miniksa I think this should have resolutions for the little things people brought up by now.

@miniksa
Copy link
Member

miniksa commented Apr 20, 2020

@miniksa I think this should have resolutions for the little things people brought up by now.

@zadjii-msft, OK Thanks.

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

Good enough. I can finalize any cleanup in the other PR. Thank you @zadjii-msft.

@miniksa miniksa merged commit cbe389f into dev/miniksa/dpi Apr 20, 2020
@miniksa miniksa deleted the dev/migrie/dpi branch April 20, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants