From 068e3e7bc202a83e3bf497bca5981844c7e4fd39 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 12 Mar 2020 17:04:43 -0700 Subject: [PATCH] til::point (#4897) ## Summary of the Pull Request Introduces convenience type `til::point` which automatically implements our best practices for point-related types and provides automatic conversions in/out of the relevant types. ## PR Checklist * [x] In support of Differential Rendering #778 * [X] I work here. * [x] Tests added/passed * [x] I'm a core contributor. ## Detailed Description of the Pull Request / Additional comments - Automatically converts in from anything with a X/Y (console `COORD`) or x/y (Win32 `POINT`) - Automatically converts out to `COORD`, `POINT`, or `D2D1_POINT_2F`. - Constructs from bare integers written into source file - Default constructs to empty - Uses Chromium Math for all basic math operations (+, -, *, /) - Provides equality tests - Accessors for x/y - Type converting accessors (that use safe conversions and throw) for x/y - TAEF/WEX Output and Comparators so they will print very nicely with `VERIFY` and `Log` macros in our testing suite. - A natvis ## Validation Steps Performed - See automated tests of functionality. --- src/inc/til.h | 1 + src/inc/til/point.h | 248 ++++++++++ src/til/ut_til/PointTests.cpp | 461 ++++++++++++++++++ src/til/ut_til/til.unit.tests.vcxproj | 1 + src/til/ut_til/til.unit.tests.vcxproj.filters | 2 + tools/ConsoleTypes.natvis | 6 +- 6 files changed, 718 insertions(+), 1 deletion(-) create mode 100644 src/inc/til/point.h create mode 100644 src/til/ut_til/PointTests.cpp diff --git a/src/inc/til.h b/src/inc/til.h index 1519f9245..b4a6c4f80 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -7,6 +7,7 @@ #include "til/color.h" #include "til/some.h" #include "til/size.h" +#include "til/point.h" #include "til/u8u16convert.h" namespace til // Terminal Implementation Library. Also: "Today I Learned" diff --git a/src/inc/til/point.h b/src/inc/til/point.h new file mode 100644 index 000000000..01257934d --- /dev/null +++ b/src/inc/til/point.h @@ -0,0 +1,248 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#ifdef UNIT_TESTING +class PointTests; +#endif + +namespace til // Terminal Implementation Library. Also: "Today I Learned" +{ + class point + { + public: + constexpr point() noexcept : + point(0, 0) + { + } + + // On 64-bit processors, int and ptrdiff_t are different fundamental types. + // On 32-bit processors, they're the same which makes this a double-definition + // with the `ptrdiff_t` one below. +#if defined(_M_AMD64) || defined(_M_ARM64) + constexpr point(int x, int y) noexcept : + point(static_cast(x), static_cast(y)) + { + } +#endif + + point(size_t x, size_t y) + { + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(x).AssignIfValid(&_x)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(y).AssignIfValid(&_y)); + } + + constexpr point(ptrdiff_t x, ptrdiff_t y) noexcept : + _x(x), + _y(y) + { + } + + // This template will convert to size from anything that has an X and a Y field that appear convertable to an integer value + template + constexpr point(const TOther& other, std::enable_if_t().X)> && std::is_integral_v().Y)>, int> /*sentinel*/ = 0) : + point(static_cast(other.X), static_cast(other.Y)) + { + } + + // This template will convert to size from anything that has a x and a y field that appear convertable to an integer value + template + constexpr point(const TOther& other, std::enable_if_t().x)> && std::is_integral_v().y)>, int> /*sentinel*/ = 0) : + point(static_cast(other.x), static_cast(other.y)) + { + } + + constexpr bool operator==(const point& other) const noexcept + { + return _x == other._x && + _y == other._y; + } + + constexpr bool operator!=(const point& other) const noexcept + { + return !(*this == other); + } + + operator bool() const noexcept + { + return _x != 0 || _y != 0; + } + + constexpr bool operator<(const point& other) const noexcept + { + if (_y < other._y) + { + return true; + } + else if (_y > other._y) + { + return false; + } + else + { + return _x < other._x; + } + } + + constexpr bool operator>(const point& other) const noexcept + { + if (_y > other._y) + { + return true; + } + else if (_y < other._y) + { + return false; + } + else + { + return _x > other._x; + } + } + + point operator+(const point& other) const + { + ptrdiff_t x; + THROW_HR_IF(E_ABORT, !base::CheckAdd(_x, other._x).AssignIfValid(&x)); + + ptrdiff_t y; + THROW_HR_IF(E_ABORT, !base::CheckAdd(_y, other._y).AssignIfValid(&y)); + + return point{ x, y }; + } + + point operator-(const point& other) const + { + ptrdiff_t x; + THROW_HR_IF(E_ABORT, !base::CheckSub(_x, other._x).AssignIfValid(&x)); + + ptrdiff_t y; + THROW_HR_IF(E_ABORT, !base::CheckSub(_y, other._y).AssignIfValid(&y)); + + return point{ x, y }; + } + + point operator*(const point& other) const + { + ptrdiff_t x; + THROW_HR_IF(E_ABORT, !base::CheckMul(_x, other._x).AssignIfValid(&x)); + + ptrdiff_t y; + THROW_HR_IF(E_ABORT, !base::CheckMul(_y, other._y).AssignIfValid(&y)); + + return point{ x, y }; + } + + point operator/(const point& other) const + { + ptrdiff_t x; + THROW_HR_IF(E_ABORT, !base::CheckDiv(_x, other._x).AssignIfValid(&x)); + + ptrdiff_t y; + THROW_HR_IF(E_ABORT, !base::CheckDiv(_y, other._y).AssignIfValid(&y)); + + return point{ x, y }; + } + + constexpr ptrdiff_t x() const noexcept + { + return _x; + } + + template + T x() const + { + T ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(x()).AssignIfValid(&ret)); + return ret; + } + + constexpr ptrdiff_t y() const noexcept + { + return _y; + } + + template + T y() const + { + T ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(y()).AssignIfValid(&ret)); + return ret; + } + +#ifdef _WINCONTYPES_ + operator COORD() const + { + COORD ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_x).AssignIfValid(&ret.X)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_y).AssignIfValid(&ret.Y)); + return ret; + } +#endif + +#ifdef _WINDEF_ + operator POINT() const + { + POINT ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_x).AssignIfValid(&ret.x)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_y).AssignIfValid(&ret.y)); + return ret; + } +#endif + +#ifdef DCOMMON_H_INCLUDED + constexpr operator D2D1_POINT_2F() const noexcept + { + return D2D1_POINT_2F{ gsl::narrow_cast(_x), gsl::narrow_cast(_y) }; + } +#endif + + protected: + ptrdiff_t _x; + ptrdiff_t _y; + +#ifdef UNIT_TESTING + friend class ::PointTests; +#endif + }; +} + +#ifdef __WEX_COMMON_H__ +namespace WEX::TestExecution +{ + template<> + class VerifyOutputTraits<::til::point> + { + public: + static WEX::Common::NoThrowString ToString(const ::til::point& point) + { + return WEX::Common::NoThrowString().Format(L"(X:%td, Y:%td)", point.x(), point.y()); + } + }; + + template<> + class VerifyCompareTraits<::til::point, ::til::point> + { + public: + static bool AreEqual(const ::til::point& expected, const ::til::point& actual) noexcept + { + return expected == actual; + } + + static bool AreSame(const ::til::point& expected, const ::til::point& actual) noexcept + { + return &expected == &actual; + } + + static bool IsLessThan(const ::til::point& expectedLess, const ::til::point& expectedGreater) = delete; + + static bool IsGreaterThan(const ::til::point& expectedGreater, const ::til::point& expectedLess) = delete; + + static bool IsNull(const ::til::point& object) noexcept + { + return object == til::point{}; + } + }; +}; +#endif diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp new file mode 100644 index 000000000..59ef7ec9c --- /dev/null +++ b/src/til/ut_til/PointTests.cpp @@ -0,0 +1,461 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "til/point.h" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +class PointTests +{ + TEST_CLASS(PointTests); + + TEST_METHOD(DefaultConstruct) + { + const til::point pt; + VERIFY_ARE_EQUAL(0, pt._x); + VERIFY_ARE_EQUAL(0, pt._y); + } + + TEST_METHOD(RawConstruct) + { + const til::point pt{ 5, 10 }; + VERIFY_ARE_EQUAL(5, pt._x); + VERIFY_ARE_EQUAL(10, pt._y); + } + + TEST_METHOD(UnsignedConstruct) + { + Log::Comment(L"0.) Normal unsigned construct."); + { + const size_t x = 5; + const size_t y = 10; + + const til::point pt{ x, y }; + VERIFY_ARE_EQUAL(5, pt._x); + VERIFY_ARE_EQUAL(10, pt._y); + } + + Log::Comment(L"1.) Unsigned construct overflow on x."); + { + constexpr size_t x = std::numeric_limits().max(); + const size_t y = 10; + + auto fn = [&]() { + til::point pt{ x, y }; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Unsigned construct overflow on y."); + { + constexpr size_t y = std::numeric_limits().max(); + const size_t x = 10; + + auto fn = [&]() { + til::point pt{ x, y }; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(SignedConstruct) + { + const ptrdiff_t x = -5; + const ptrdiff_t y = -10; + + const til::point pt{ x, y }; + VERIFY_ARE_EQUAL(x, pt._x); + VERIFY_ARE_EQUAL(y, pt._y); + } + + TEST_METHOD(CoordConstruct) + { + COORD coord{ -5, 10 }; + + const til::point pt{ coord }; + VERIFY_ARE_EQUAL(coord.X, pt._x); + VERIFY_ARE_EQUAL(coord.Y, pt._y); + } + + TEST_METHOD(PointConstruct) + { + POINT point{ 5, -10 }; + + const til::point pt{ point }; + VERIFY_ARE_EQUAL(point.x, pt._x); + VERIFY_ARE_EQUAL(point.y, pt._y); + } + + TEST_METHOD(Equality) + { + Log::Comment(L"0.) Equal."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 == s2); + } + + Log::Comment(L"1.) Left Width changed."); + { + const til::point s1{ 4, 10 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"2.) Right Width changed."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 6, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"3.) Left Height changed."); + { + const til::point s1{ 5, 9 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"4.) Right Height changed."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 5, 11 }; + VERIFY_IS_FALSE(s1 == s2); + } + } + + TEST_METHOD(Inequality) + { + Log::Comment(L"0.) Equal."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 != s2); + } + + Log::Comment(L"1.) Left Width changed."); + { + const til::point s1{ 4, 10 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"2.) Right Width changed."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 6, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"3.) Left Height changed."); + { + const til::point s1{ 5, 9 }; + const til::point s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"4.) Right Height changed."); + { + const til::point s1{ 5, 10 }; + const til::point s2{ 5, 11 }; + VERIFY_IS_TRUE(s1 != s2); + } + } + + TEST_METHOD(Boolean) + { + const til::point empty; + VERIFY_IS_FALSE(empty); + + const til::point yOnly{ 0, 10 }; + VERIFY_IS_TRUE(yOnly); + + const til::point xOnly{ 10, 0 }; + VERIFY_IS_TRUE(xOnly); + + const til::point both{ 10, 10 }; + VERIFY_IS_TRUE(both); + } + + TEST_METHOD(Addition) + { + Log::Comment(L"0.) Addition of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() + pt2.x(), pt.y() + pt2.y() }; + + VERIFY_ARE_EQUAL(expected, pt + pt2); + } + + Log::Comment(L"1.) Addition results in value that is too large (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + pt + pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Addition results in value that is too large (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + pt + pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Subtraction) + { + Log::Comment(L"0.) Subtraction of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() - pt2.x(), pt.y() - pt2.y() }; + + VERIFY_ARE_EQUAL(expected, pt - pt2); + } + + Log::Comment(L"1.) Subtraction results in value that is too small (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ -2, -2 }; + + auto fn = [&]() { + pt2 - pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Subtraction results in value that is too small (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ -2, -2 }; + + auto fn = [&]() { + pt2 - pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Multiplication) + { + Log::Comment(L"0.) Multiplication of two things that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() * pt2.x(), pt.y() * pt2.y() }; + + VERIFY_ARE_EQUAL(expected, pt * pt2); + } + + Log::Comment(L"1.) Multiplication results in value that is too large (x)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 10, 10 }; + + auto fn = [&]() { + pt* pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Multiplication results in value that is too large (y)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ static_cast(0), bigSize }; + const til::point pt2{ 10, 10 }; + + auto fn = [&]() { + pt* pt2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Division) + { + Log::Comment(L"0.) Division of two things that should be in bounds."); + { + const til::point pt{ 555, 510 }; + const til::point pt2{ 23, 47 }; + + const til::point expected{ pt.x() / pt2.x(), pt.y() / pt2.y() }; + + VERIFY_ARE_EQUAL(expected, pt / pt2); + } + + Log::Comment(L"1.) Division by zero"); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::point pt{ bigSize, static_cast(0) }; + const til::point pt2{ 1, 1 }; + + auto fn = [&]() { + pt2 / pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(X) + { + const til::point pt{ 5, 10 }; + VERIFY_ARE_EQUAL(pt._x, pt.x()); + } + + TEST_METHOD(XCast) + { + const til::point pt{ 5, 10 }; + VERIFY_ARE_EQUAL(static_cast(pt._x), pt.x()); + } + + TEST_METHOD(Y) + { + const til::point pt{ 5, 10 }; + VERIFY_ARE_EQUAL(pt._y, pt.y()); + } + + TEST_METHOD(YCast) + { + const til::point pt{ 5, 10 }; + VERIFY_ARE_EQUAL(static_cast(pt._x), pt.x()); + } + + TEST_METHOD(CastToCoord) + { + Log::Comment(L"0.) Typical situation."); + { + const til::point pt{ 5, 10 }; + COORD val = pt; + VERIFY_ARE_EQUAL(5, val.X); + VERIFY_ARE_EQUAL(10, val.Y); + } + + Log::Comment(L"1.) Overflow on x."); + { + constexpr ptrdiff_t x = std::numeric_limits().max(); + const ptrdiff_t y = 10; + const til::point pt{ x, y }; + + auto fn = [&]() { + COORD val = pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Overflow on y."); + { + constexpr ptrdiff_t y = std::numeric_limits().max(); + const ptrdiff_t x = 10; + const til::point pt{ x, y }; + + auto fn = [&]() { + COORD val = pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(CastToPoint) + { + Log::Comment(L"0.) Typical situation."); + { + const til::point pt{ 5, 10 }; + POINT val = pt; + VERIFY_ARE_EQUAL(5, val.x); + VERIFY_ARE_EQUAL(10, val.y); + } + + Log::Comment(L"1.) Fit max x into POINT (may overflow)."); + { + constexpr ptrdiff_t x = std::numeric_limits().max(); + const ptrdiff_t y = 10; + const til::point pt{ x, y }; + + // On some platforms, ptrdiff_t will fit inside x/y + const bool overflowExpected = x > std::numeric_limits().max(); + + if (overflowExpected) + { + auto fn = [&]() { + POINT val = pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + else + { + POINT val = pt; + VERIFY_ARE_EQUAL(x, val.x); + } + } + + Log::Comment(L"2.) Fit max y into POINT (may overflow)."); + { + constexpr ptrdiff_t y = std::numeric_limits().max(); + const ptrdiff_t x = 10; + const til::point pt{ x, y }; + + // On some platforms, ptrdiff_t will fit inside x/y + const bool overflowExpected = y > std::numeric_limits().max(); + + if (overflowExpected) + { + auto fn = [&]() { + POINT val = pt; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + else + { + POINT val = pt; + VERIFY_ARE_EQUAL(y, val.y); + } + } + } + + TEST_METHOD(CastToD2D1Point2F) + { + Log::Comment(L"0.) Typical situation."); + { + const til::point pt{ 5, 10 }; + D2D1_POINT_2F val = pt; + VERIFY_ARE_EQUAL(5, val.x); + VERIFY_ARE_EQUAL(10, val.y); + } + + // All ptrdiff_ts fit into a float, so there's no exception tests. + } +}; diff --git a/src/til/ut_til/til.unit.tests.vcxproj b/src/til/ut_til/til.unit.tests.vcxproj index 750931c19..cb12b4ae7 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj +++ b/src/til/ut_til/til.unit.tests.vcxproj @@ -10,6 +10,7 @@ + diff --git a/src/til/ut_til/til.unit.tests.vcxproj.filters b/src/til/ut_til/til.unit.tests.vcxproj.filters index c78750fac..7b6da4d90 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj.filters +++ b/src/til/ut_til/til.unit.tests.vcxproj.filters @@ -8,6 +8,8 @@ + + diff --git a/tools/ConsoleTypes.natvis b/tools/ConsoleTypes.natvis index 8d09a1a67..9502b3f0f 100644 --- a/tools/ConsoleTypes.natvis +++ b/tools/ConsoleTypes.natvis @@ -77,7 +77,7 @@ - {{↓ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} + {{↓ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} {{↑ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} @@ -85,6 +85,10 @@ {{W: {_width,d} x H: {_height,d} -> A: {_width * _height, d}}} + + {{X: {_x,d}, Y: {_y,d}}} + + {{RGB: {(int)r,d}, {(int)g,d}, {(int)b,d}; α: {(int)a,d}}}