add til::math, use it for float conversions to point, size #5150

DHowett-MSFT · 2020-03-27T21:27:49Z

This pull request introduces the til::math namespace, which provides some casting functions to be used in support of til::point and til::size. When point/size want to ingest a floating-point structure, they must be instructed on how to convert those floating-point values into integers.

This enables:

Windows::Foundation::Point wfPoint = /* ... */;
til::point tp{ til::math::rounding, wfPoint };

Future thoughts: should the TilMath types be stackable? Right now, you cannot get "checked + rounding" behavior (where it throws if it doesn't fit) so everything is saturating.

PR Checklist

Closes a request by Michael
I've discussed this with core contributors already

src/til/ut_til/MathTests.cpp

miniksa

Thanks I love it.

zadjii-msft

okay so I'm assuming it's Hard to have a test with the actual winrt::Windows::Foundation types this is intended for, so

zadjii-msft · 2020-03-27T21:52:10Z

src/inc/til/size.h

+        // This template will convert to size from anything that has a X and a Y field that are floating-point;
+        // a math type is required.
+        template<typename TilMath, typename TOther>
+        constexpr size(TilMath, const TOther& other, std::enable_if_t<std::is_floating_point_v<decltype(std::declval<TOther>().X)> && std::is_floating_point_v<decltype(std::declval<TOther>().Y)>, int> /*sentinel*/ = 0) :
+            size(TilMath::template cast<ptrdiff_t>(other.X), TilMath::template cast<ptrdiff_t>(other.Y))
+        {
+        }


Are we sure we want the x/y version for size?

Size already has an X/Y version, this just expands it. @miniksa?

I think the only thing I was expressly avoiding is implicit from COORD to both point and size because it's been a source of confusion. I can see the need to explicitly put one into the other one.

ghost · 2020-03-27T22:32:28Z

Hello @DHowett-MSFT!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 6 hours 55 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

DHowett-MSFT · 2020-03-27T22:32:30Z

@msftbot merge this in 10 minutes

ghost · 2020-03-27T22:32:32Z

Hello @DHowett-MSFT!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Fri, 27 Mar 2020 22:42:32 GMT, which is in 10 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett-MSFT · 2020-03-27T22:48:10Z

notes: this only adds floating-point versions of already existing integral member-detecting constructors except for size's .Width and .Height constructor. This doesn't widen our contract in a way that would make us work with annoying old Windows types like SIZE and POINT insofar as we did/didn't already work with them

Improve wide glyph support in UIA (GH-4946) Add enhanced key support for ConPty (GH-5021) Set DxRenderer non-text alias mode (GH-5149) Reduce CursorChanged Events for Accessibility (GH-5196) Add more object ID tracing for Accessibility (GH-5215) Add SS3 cursor key encoding to ConPty (GH-5383) UIA: Prevent crash from invalid UTR endpoint comparison (GH-5399) Make CodepointWidthDetector::GetWidth faster (CC-3727) add til::math, use it for float conversions to point, size (GH-5150) Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI (GH-5353) Fix a deadlock and a bounding rects issue in UIA (GH-5385) Don't duplicate spaces from potentially-wrapped EOL-deferred lines (GH-5398) Reimplement the VT tab stop functionality (CC-5173) Clamp parameter values to a maximum of 32767. (CC-5200) Prevent the cursor type being reset when changing the visibility (CC-5251) Make RIS switch back to the main buffer (CC-5248) Add support for the DSR-OS operating status report (CC-5300) Update the virtual bottom location if the cursor moves below it (CC-5317) ci: run spell check in CI, fix remaining issues (CC-4799) (CC-5352) Set Cascadia Code as default font (GH-5121) Show a double width cursor for double width characters (GH-5319) Delegate all character input to the character event handler (CC-4192) Update til::bitmap to use dynamic_bitset<> + libpopcnt (GH-5092) Merged PR 4465022: [Git2Git] Merged PR 4464559: Console: Ingest OSS changes up to e055079 Correct scrolling invalidation region for tmux in pty w/ bitmap (GH-5122) Render row-by-row instead of invalidating entire screen (GH-5185) Make conechokey use ReadConsoleInputW by default (GH-5148) Manually pass mouse wheel messages to TermControls (GH-5131) This fixes C-M-space for WSL but not for Win32, but I'm not sure there's a problem in Win32 quite yet. (GH-5208) Fix copying wrapped lines by implementing better scrolling (GH-5181) Emit lines wrapped due to spaces at the end correctly (GH-5294) Remove unneeded whitespace (CC-5162)

add til::math and magic converters

138c6b9

miniksa reviewed Mar 27, 2020

View reviewed changes

src/til/ut_til/MathTests.cpp Show resolved Hide resolved

fix speeling, allowlist cxcy/isnan

dc1677c

miniksa approved these changes Mar 27, 2020

View reviewed changes

carlos-zamora approved these changes Mar 27, 2020

View reviewed changes

zadjii-msft approved these changes Mar 27, 2020

View reviewed changes

DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 27, 2020

DHowett-MSFT removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 27, 2020

DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 27, 2020

ghost merged commit dfc1578 into master Mar 27, 2020

ghost deleted the dev/duhowett/til_i_can_do_math branch March 27, 2020 22:48

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add til::math, use it for float conversions to point, size #5150

add til::math, use it for float conversions to point, size #5150

DHowett-MSFT commented Mar 27, 2020 •

edited

Loading

miniksa left a comment

zadjii-msft left a comment

zadjii-msft Mar 27, 2020

DHowett-MSFT Mar 27, 2020

miniksa Mar 27, 2020

ghost commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020

ghost commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020

add til::math, use it for float conversions to point, size #5150

add til::math, use it for float conversions to point, size #5150

Conversation

DHowett-MSFT commented Mar 27, 2020 • edited Loading

PR Checklist

miniksa left a comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Mar 27, 2020

Choose a reason for hiding this comment

DHowett-MSFT Mar 27, 2020

Choose a reason for hiding this comment

miniksa Mar 27, 2020

Choose a reason for hiding this comment

ghost commented Mar 27, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

DHowett-MSFT commented Mar 27, 2020

ghost commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.