From d637178cc0a511eea8ea626c00aa5c066910f5ee Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 16 Mar 2020 17:17:50 -0700 Subject: [PATCH 01/25] bring bitmap and supporting rectangle mods over, implement more bitmap tests. --- src/inc/til.h | 1 + src/inc/til/bitmap.h | 230 ++++++++++++++++++++++++++++++ src/inc/til/rectangle.h | 85 +++++++++++ src/til/ut_til/BitmapTests.cpp | 134 +++++++++++++++++ src/til/ut_til/RectangleTests.cpp | 79 ++++++++++ 5 files changed, 529 insertions(+) create mode 100644 src/inc/til/bitmap.h create mode 100644 src/til/ut_til/BitmapTests.cpp diff --git a/src/inc/til.h b/src/inc/til.h index a5d25f3f60e..6cbe6a0b128 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -9,6 +9,7 @@ #include "til/size.h" #include "til/point.h" #include "til/rectangle.h" +#include "til/bitmap.h" #include "til/u8u16convert.h" namespace til // Terminal Implementation Library. Also: "Today I Learned" diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h new file mode 100644 index 00000000000..740a4c35df7 --- /dev/null +++ b/src/inc/til/bitmap.h @@ -0,0 +1,230 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include +#include "size.h" + +#ifdef UNIT_TESTING +class BitmapTests; +#endif + +namespace til // Terminal Implementation Library. Also: "Today I Learned" +{ + class const_runerator // Run Iterator. Runerator. + { + public: + const_runerator(const std::vector& values, til::size sz, size_t pos) : + _values(values), + _size(sz), + _pos(pos) + { + _calculateArea(); + } + + const_runerator& operator++() + { + _pos = _nextPos; + _calculateArea(); + return (*this); + } + + bool operator==(const const_runerator& other) const + { + return _pos == other._pos && _values == other._values; + } + + bool operator!=(const const_runerator& other) const + { + return !(*this == other); + } + + bool operator<(const const_runerator& other) const + { + return _pos < other._pos; + } + + bool operator>(const const_runerator& other) const + { + return _pos > other._pos; + } + + til::rectangle operator*() const + { + return _run; + } + + private: + const std::vector& _values; + const til::size _size; + ptrdiff_t _pos; + ptrdiff_t _nextPos; + til::rectangle _run; + + // why? why isn't this just walking with a bitterator? + til::point _indexToPoint(ptrdiff_t index) + { + const auto width = _size.width(); + const auto row = base::CheckDiv(index, width); + const auto indexesConsumed = row * width; + const auto col = base::CheckSub(index, indexesConsumed); + + ptrdiff_t x, y; + THROW_HR_IF(E_ABORT, !row.AssignIfValid(&y)); + THROW_HR_IF(E_ABORT, !col.AssignIfValid(&x)); + + return til::point{ x, y }; + } + + void _calculateArea() + { + const ptrdiff_t end = _size.area(); + + _nextPos = _pos; + + while (_nextPos < end && !_values.at(_nextPos)) + { + ++_nextPos; + } + + if (_nextPos < end) + { + // pos is now at the first on bit. + const auto runStart = _indexToPoint(_nextPos); + const ptrdiff_t rowEndIndex = ((runStart.y() + 1) * _size.width()); + + ptrdiff_t runLength = 0; + + do + { + ++_nextPos; + ++runLength; + } while (_nextPos < end && _nextPos < rowEndIndex && _values.at(_nextPos)); + + _run = til::rectangle{ runStart, til::size{ runLength, static_cast(1) } }; + } + else + { + _pos = _nextPos; + _run = til::rectangle{}; + } + } + }; + + class bitmap + { + public: + bitmap() : + bitmap(0, 0) + { + } + + bitmap(size_t width, size_t height) : + bitmap(til::size{ width, height }) + { + } + + bitmap(til::size sz) : + _size(sz), + _bits(sz.area(), true), + _empty(false) + { + } + + const_runerator begin_runs() const + { + return const_runerator(_bits, _size, 0); + } + + const_runerator end_runs() const + { + return const_runerator(_bits, _size, _size.area()); + } + + void set(til::point pt) + { + _bits[pt.y() * _size.width() + pt.x()] = true; + _empty = false; + } + + void reset(til::point pt) + { + _bits[pt.y() * _size.width() + pt.x()] = false; + } + + void set(til::rectangle rc) + { + for (auto pt : rc) + { + set(pt); + } + } + + void reset(til::rectangle rc) + { + for (auto pt : rc) + { + reset(pt); + } + } + + void set_all() + { + // .clear() then .resize(_size(), true) throws an assert (unsupported operation) + // .assign(_size(), true) throws an assert (unsupported operation) + + set(til::rectangle{ til::point{ 0, 0 }, _size }); + } + + void reset_all() + { + // .clear() then .resize(_size(), false) throws an assert (unsupported operation) + // .assign(_size(), false) throws an assert (unsupported operation) + reset(til::rectangle{ til::point{ 0, 0 }, _size }); + _empty = true; + } + + void resize(til::size size) + { + // Don't resize if it's not different as we mark the whole thing dirty on resize. + // TODO: marking it dirty might not be necessary or we should be smart about it + // (mark none of it dirty on resize down, mark just the edges on up?) + if (_size != size) + { + _size = size; + // .resize(_size(), true) throws an assert (unsupported operation) + _bits = std::vector(_size.area(), true); + } + } + + void resize(size_t width, size_t height) + { + resize(til::size{ width, height }); + } + + constexpr bool empty() const + { + return _empty; + } + + const til::size& size() const + { + return _size; + } + + operator bool() const noexcept + { + return !_bits.empty(); + } + + private: + bool _empty; + til::size _size; + std::vector _bits; + +#ifdef UNIT_TESTING + friend class ::BitmapTests; +#endif + }; +} diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 61379a385c1..196f6623a43 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -13,9 +13,84 @@ class RectangleTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { + class recterator + { + public: + constexpr recterator(point topLeft, point bottomRight) : + _topLeft(topLeft), + _bottomRight(bottomRight), + _current(topLeft) + { + } + + constexpr recterator(point topLeft, point bottomRight, point start) : + _topLeft(topLeft), + _bottomRight(bottomRight), + _current(start) + { + } + + recterator& operator++() + { + ptrdiff_t nextX; + THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.x(), 1).AssignIfValid(&nextX)); + + if (nextX >= _bottomRight.x()) + { + ptrdiff_t nextY; + THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.y(), 1).AssignIfValid(&nextY)); + _current = { _topLeft.x(), nextY }; + } + else + { + _current = { nextX, _current.y() }; + } + + return (*this); + } + + constexpr bool operator==(const recterator& other) const + { + return _current == other._current && + _topLeft == other._topLeft && + _bottomRight == other._bottomRight; + } + + constexpr bool operator!=(const recterator& other) const + { + return !(*this == other); + } + + constexpr bool operator<(const recterator& other) const + { + return _current < other._current; + } + + constexpr bool operator>(const recterator& other) const + { + return _current > other._current; + } + + constexpr point operator*() const + { + return _current; + } + + protected: + point _current; + const point _topLeft; + const point _bottomRight; + +#ifdef UNIT_TESTING + friend class ::RectangleTests; +#endif + }; + class rectangle { public: + using const_iterator = recterator; + constexpr rectangle() noexcept : rectangle(til::point{ 0, 0 }, til::point{ 0, 0 }) { @@ -115,6 +190,16 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _topLeft.y() < _bottomRight.y(); } + constexpr const_iterator begin() const + { + return recterator(_topLeft, _bottomRight); + } + + constexpr const_iterator end() const + { + return recterator(_topLeft, _bottomRight, { _topLeft.x(), _bottomRight.y() }); + } + // OR = union constexpr rectangle operator|(const rectangle& other) const noexcept { diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp new file mode 100644 index 00000000000..73b1899ef67 --- /dev/null +++ b/src/til/ut_til/BitmapTests.cpp @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "til/bitmap.h" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +class BitmapTests +{ + TEST_CLASS(BitmapTests); + + TEST_METHOD(Construct) + { + COORD foo; + foo.X = 12; + foo.Y = 14; + til::point p(foo); + VERIFY_ARE_EQUAL(foo.X, p.x()); + VERIFY_ARE_EQUAL(foo.Y, p.y()); + + POINT pt; + pt.x = 88; + pt.y = 98; + til::point q(pt); + VERIFY_ARE_EQUAL(pt.x, q.x()); + VERIFY_ARE_EQUAL(pt.y, q.y()); + + SIZE sz; + sz.cx = 11; + sz.cy = 13; + til::size r(sz); + VERIFY_ARE_EQUAL(sz.cx, r.width()); + VERIFY_ARE_EQUAL(sz.cy, r.height()); + + COORD bar; + bar.X = 57; + bar.Y = 15; + til::size s(bar); + VERIFY_ARE_EQUAL(bar.X, s.width()); + VERIFY_ARE_EQUAL(bar.Y, s.height()); + + SIZE mapSize{ 10, 10 }; + til::bitmap x(mapSize); + x.set({ 4, 4 }); + + til::rectangle area(til::point{ 5, 5 }, til::size{ 2, 2 }); + x.set(area); + + Log::Comment(L"Row 4!"); + for (auto it = x.begin_row(4); it < x.end_row(4); ++it) + { + if (*it) + { + Log::Comment(L"True"); + } + else + { + Log::Comment(L"False"); + } + } + + Log::Comment(L"All!"); + auto start = x.begin(); + auto end = x.end(); + + for (const auto& y : x) + { + if (y) + { + Log::Comment(L"True"); + } + else + { + Log::Comment(L"False"); + } + } + + SMALL_RECT smrc; + smrc.Top = 31; + smrc.Bottom = 41; + smrc.Left = 59; + smrc.Right = 265; + + til::rectangle smrectangle(smrc); + + VERIFY_ARE_EQUAL(smrc.Top, smrectangle.top()); + VERIFY_ARE_EQUAL(smrc.Bottom, smrectangle.bottom()); + VERIFY_ARE_EQUAL(smrc.Left, smrectangle.left()); + VERIFY_ARE_EQUAL(smrc.Right, smrectangle.right()); + + RECT bgrc; + bgrc.top = 3; + bgrc.bottom = 5; + bgrc.left = 8; + bgrc.right = 9; + + til::rectangle bgrectangle(bgrc); + + VERIFY_ARE_EQUAL(bgrc.top, bgrectangle.top()); + VERIFY_ARE_EQUAL(bgrc.bottom, bgrectangle.bottom()); + VERIFY_ARE_EQUAL(bgrc.left, bgrectangle.left()); + VERIFY_ARE_EQUAL(bgrc.right, bgrectangle.right()); + } + + TEST_METHOD(Runerator) + { + til::bitmap foo{ til::size{ 4, 8 } }; + foo.reset_all(); + foo.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 2 } }); + + foo.set(til::rectangle{ til::point{ 3, 5 } }); + foo.set(til::rectangle{ til::point{ 0, 6 } }); + + std::deque expectedRects; + expectedRects.push_back(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 1 } }); + expectedRects.push_back(til::rectangle{ til::point{ 1, 2 }, til::size{ 2, 1 } }); + expectedRects.push_back(til::rectangle{ til::point{ 3, 5 } }); + expectedRects.push_back(til::rectangle{ til::point{ 0, 6 } }); + + for (auto it = foo.begin_runs(); it < foo.end_runs(); ++it) + { + const auto actual = *it; + const auto expected = expectedRects.front(); + + VERIFY_ARE_EQUAL(expected, actual); + + expectedRects.pop_front(); + } + } +}; diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 21557881bdf..7e99fdd91d4 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -929,4 +929,83 @@ class RectangleTests // All ptrdiff_ts fit into a float, so there's no exception tests. } + +#pragma region iterator + TEST_METHOD(Begin) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + const til::point expected{ rc.left(), rc.top() }; + const auto it = rc.begin(); + + VERIFY_ARE_EQUAL(expected, *it); + } + + TEST_METHOD(End) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + const til::point expected{ rc.left(), rc.bottom() }; + const auto it = rc.end(); + + VERIFY_ARE_EQUAL(expected, *it); + } + + TEST_METHOD(ConstIteratorIncrement) + { + const til::rectangle rc{ til::size{2, 2} }; + + auto it = rc.begin(); + auto expected = til::point{ 0, 0 }; + VERIFY_ARE_EQUAL(expected, *it); + + ++it; + expected = til::point{ 1, 0 }; + VERIFY_ARE_EQUAL(expected, *it); + + ++it; + expected = til::point{ 0, 1 }; + VERIFY_ARE_EQUAL(expected, *it); + + ++it; + expected = til::point{ 1, 1 }; + VERIFY_ARE_EQUAL(expected, *it); + + ++it; + expected = til::point{ 0, 2 }; + VERIFY_ARE_EQUAL(expected, *it); + VERIFY_ARE_EQUAL(expected, *rc.end()); + } + + TEST_METHOD(ConstIteratorEquality) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + VERIFY_IS_TRUE(rc.begin() == rc.begin()); + VERIFY_IS_FALSE(rc.begin() == rc.end()); + } + + TEST_METHOD(ConstIteratorInequality) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + VERIFY_IS_FALSE(rc.begin() != rc.begin()); + VERIFY_IS_TRUE(rc.begin() != rc.end()); + } + + TEST_METHOD(ConstIteratorLessThan) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + VERIFY_IS_TRUE(rc.begin() < rc.end()); + VERIFY_IS_FALSE(rc.end() < rc.begin()); + } + + TEST_METHOD(ConstIteratorGreaterThan) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + VERIFY_IS_TRUE(rc.end() > rc.begin()); + VERIFY_IS_FALSE(rc.begin() > rc.end()); + } + +#pragma endregion }; From 70567d46f722b6c8e94e00c37e3d7588c35f0965 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 14:25:43 -0700 Subject: [PATCH 02/25] Make bitmap tests. Add supplemental helpers to rectangle and some, also add associated tests. --- src/inc/til/bitmap.h | 131 ++++---- src/inc/til/rectangle.h | 67 +++- src/inc/til/some.h | 5 + src/til/ut_til/BitmapTests.cpp | 316 +++++++++++++----- src/til/ut_til/RectangleTests.cpp | 121 +++++++ src/til/ut_til/SomeTests.cpp | 12 + src/til/ut_til/til.unit.tests.vcxproj | 1 + src/til/ut_til/til.unit.tests.vcxproj.filters | 1 + 8 files changed, 486 insertions(+), 168 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 740a4c35df7..fd5979be22d 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -12,40 +12,41 @@ class BitmapTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { - class const_runerator // Run Iterator. Runerator. + class _bitmap_const_iterator { public: - const_runerator(const std::vector& values, til::size sz, size_t pos) : + _bitmap_const_iterator(const std::vector& values, til::rectangle rc, ptrdiff_t pos) : _values(values), - _size(sz), - _pos(pos) + _rc(rc), + _pos(pos), + _end(rc.size().area()) { _calculateArea(); } - const_runerator& operator++() + _bitmap_const_iterator& operator++() { _pos = _nextPos; _calculateArea(); return (*this); } - bool operator==(const const_runerator& other) const + bool operator==(const _bitmap_const_iterator& other) const { return _pos == other._pos && _values == other._values; } - bool operator!=(const const_runerator& other) const + bool operator!=(const _bitmap_const_iterator& other) const { return !(*this == other); } - bool operator<(const const_runerator& other) const + bool operator<(const _bitmap_const_iterator& other) const { return _pos < other._pos; } - bool operator>(const const_runerator& other) const + bool operator>(const _bitmap_const_iterator& other) const { return _pos > other._pos; } @@ -56,56 +57,52 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: + const std::vector& _values; - const til::size _size; + const til::rectangle _rc; ptrdiff_t _pos; ptrdiff_t _nextPos; + const ptrdiff_t _end; til::rectangle _run; - // why? why isn't this just walking with a bitterator? - til::point _indexToPoint(ptrdiff_t index) - { - const auto width = _size.width(); - const auto row = base::CheckDiv(index, width); - const auto indexesConsumed = row * width; - const auto col = base::CheckSub(index, indexesConsumed); - - ptrdiff_t x, y; - THROW_HR_IF(E_ABORT, !row.AssignIfValid(&y)); - THROW_HR_IF(E_ABORT, !col.AssignIfValid(&x)); - - return til::point{ x, y }; - } - void _calculateArea() { - const ptrdiff_t end = _size.area(); - + // Backup the position as the next one. _nextPos = _pos; - while (_nextPos < end && !_values.at(_nextPos)) + // Seek forward until we find an on bit. + while (_nextPos < _end && !_values.at(_nextPos)) { ++_nextPos; } - if (_nextPos < end) + // If we haven't reached the end yet... + if (_nextPos < _end) { // pos is now at the first on bit. - const auto runStart = _indexToPoint(_nextPos); - const ptrdiff_t rowEndIndex = ((runStart.y() + 1) * _size.width()); + const auto runStart = _rc.point_at(_nextPos); + + // We'll only count up until the end of this row. + // a run can be a max of one row tall. + const ptrdiff_t rowEndIndex = _rc.index_of(til::point(_rc.right() - 1, runStart.y())) + 1; + // Find the length for the rectangle. ptrdiff_t runLength = 0; + // We have at least 1 so start with a do/while. do { ++_nextPos; ++runLength; - } while (_nextPos < end && _nextPos < rowEndIndex && _values.at(_nextPos)); + } while (_nextPos < _end && _nextPos < rowEndIndex && _values.at(_nextPos)); + // Keep going until we reach end of row, end of the buffer, or the next bit is off. + // Assemble and store that run. _run = til::rectangle{ runStart, til::size{ runLength, static_cast(1) } }; } else { + // If we reached the end, set the pos because the run is empty. _pos = _nextPos; _run = til::rectangle{}; } @@ -115,42 +112,40 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" class bitmap { public: - bitmap() : - bitmap(0, 0) - { - } + using const_iterator = _bitmap_const_iterator; - bitmap(size_t width, size_t height) : - bitmap(til::size{ width, height }) + bitmap() : + bitmap(til::size{ 0, 0 }) { } bitmap(til::size sz) : - _size(sz), - _bits(sz.area(), true), - _empty(false) + _sz(sz), + _rc(sz), + _bits(sz.area(), false), + _empty(true) { } - const_runerator begin_runs() const + const_iterator begin() const { - return const_runerator(_bits, _size, 0); + return _bitmap_const_iterator(_bits, _sz, 0); } - const_runerator end_runs() const + const_iterator end() const { - return const_runerator(_bits, _size, _size.area()); + return _bitmap_const_iterator(_bits, _sz, _sz.area()); } void set(til::point pt) { - _bits[pt.y() * _size.width() + pt.x()] = true; + _bits[_rc.index_of(pt)] = true; _empty = false; } void reset(til::point pt) { - _bits[pt.y() * _size.width() + pt.x()] = false; + _bits[_rc.index_of(pt)] = false; } void set(til::rectangle rc) @@ -174,33 +169,34 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // .clear() then .resize(_size(), true) throws an assert (unsupported operation) // .assign(_size(), true) throws an assert (unsupported operation) - set(til::rectangle{ til::point{ 0, 0 }, _size }); + set(_rc); } void reset_all() { // .clear() then .resize(_size(), false) throws an assert (unsupported operation) // .assign(_size(), false) throws an assert (unsupported operation) - reset(til::rectangle{ til::point{ 0, 0 }, _size }); + reset(_rc); _empty = true; } - void resize(til::size size) + // True if we resized. False if it was the same size as before. + bool resize(til::size size) { // Don't resize if it's not different as we mark the whole thing dirty on resize. - // TODO: marking it dirty might not be necessary or we should be smart about it - // (mark none of it dirty on resize down, mark just the edges on up?) - if (_size != size) + if (_sz != size) { - _size = size; - // .resize(_size(), true) throws an assert (unsupported operation) - _bits = std::vector(_size.area(), true); + _sz = size; + _rc = til::rectangle{ size }; + // .resize(_size(), true/false) throws an assert (unsupported operation) + _bits = std::vector(_sz.area(), false); + _empty = true; + return true; + } + else + { + return false; } - } - - void resize(size_t width, size_t height) - { - resize(til::size{ width, height }); } constexpr bool empty() const @@ -208,19 +204,10 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _empty; } - const til::size& size() const - { - return _size; - } - - operator bool() const noexcept - { - return !_bits.empty(); - } - private: bool _empty; - til::size _size; + til::size _sz; + til::rectangle _rc; std::vector _bits; #ifdef UNIT_TESTING diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 196f6623a43..b18701a1f08 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -13,24 +13,24 @@ class RectangleTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { - class recterator + class _Rectangle_const_iterator { public: - constexpr recterator(point topLeft, point bottomRight) : + constexpr _Rectangle_const_iterator(point topLeft, point bottomRight) : _topLeft(topLeft), _bottomRight(bottomRight), _current(topLeft) { } - constexpr recterator(point topLeft, point bottomRight, point start) : + constexpr _Rectangle_const_iterator(point topLeft, point bottomRight, point start) : _topLeft(topLeft), _bottomRight(bottomRight), _current(start) { } - recterator& operator++() + _Rectangle_const_iterator& operator++() { ptrdiff_t nextX; THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.x(), 1).AssignIfValid(&nextX)); @@ -49,24 +49,24 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return (*this); } - constexpr bool operator==(const recterator& other) const + constexpr bool operator==(const _Rectangle_const_iterator& other) const { return _current == other._current && _topLeft == other._topLeft && _bottomRight == other._bottomRight; } - constexpr bool operator!=(const recterator& other) const + constexpr bool operator!=(const _Rectangle_const_iterator& other) const { return !(*this == other); } - constexpr bool operator<(const recterator& other) const + constexpr bool operator<(const _Rectangle_const_iterator& other) const { return _current < other._current; } - constexpr bool operator>(const recterator& other) const + constexpr bool operator>(const _Rectangle_const_iterator& other) const { return _current > other._current; } @@ -76,7 +76,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _current; } - protected: + protected: point _current; const point _topLeft; const point _bottomRight; @@ -89,7 +89,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" class rectangle { public: - using const_iterator = recterator; + using const_iterator = _Rectangle_const_iterator; constexpr rectangle() noexcept : rectangle(til::point{ 0, 0 }, til::point{ 0, 0 }) @@ -192,12 +192,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr const_iterator begin() const { - return recterator(_topLeft, _bottomRight); + return const_iterator(_topLeft, _bottomRight); } constexpr const_iterator end() const { - return recterator(_topLeft, _bottomRight, { _topLeft.x(), _bottomRight.y() }); + return const_iterator(_topLeft, _bottomRight, { _topLeft.x(), _bottomRight.y() }); } // OR = union @@ -485,6 +485,49 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return !operator bool(); } + constexpr bool contains(til::point pt) const + { + return pt.x() >= _topLeft.x() && pt.x() < _bottomRight.x() && + pt.y() >= _topLeft.y() && pt.y() < _bottomRight.y(); + } + + bool contains(ptrdiff_t index) const + { + return index >= 0 && index < size().area(); + } + + ptrdiff_t index_of(til::point pt) const + { + THROW_HR_IF(E_INVALIDARG, !contains(pt)); + + // Take Y away from the top to find how many rows down + auto check = base::CheckSub(pt.y(), top()); + + // Multiply by the width because we've passed that many + // widths-worth of indices. + check *= width(); + + // Then add in the last few indices in the x position this row + // and subtract left to find the offset from left edge. + check = check + pt.x() - left(); + + ptrdiff_t result; + THROW_HR_IF(E_ABORT, !check.AssignIfValid(&result)); + return result; + } + + til::point point_at(ptrdiff_t index) const + { + THROW_HR_IF(E_INVALIDARG, !contains(index)); + + const auto div = std::div(index, width()); + + // Not checking math on these because we're presuming + // that the point can't be in bounds of a rectangle where + // this would overflow on addition after the division. + return til::point{ div.rem + left(), div.quot + top() }; + } + #ifdef _WINCONTYPES_ // NOTE: This will convert back to INCLUSIVE on the way out because // that is generally how SMALL_RECTs are handled in console code and via the APIs. diff --git a/src/inc/til/some.h b/src/inc/til/some.h index aafaa0ec3ee..c8da45d0742 100644 --- a/src/inc/til/some.h +++ b/src/inc/til/some.h @@ -127,6 +127,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return !_used; } + constexpr void clear() noexcept + { + _used = 0; + } + constexpr const_reference at(size_type pos) const { if (_used <= pos) diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 73b1899ef67..976ac6c46a8 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -13,122 +13,270 @@ class BitmapTests { TEST_CLASS(BitmapTests); - TEST_METHOD(Construct) + TEST_METHOD(DefaultConstruct) { - COORD foo; - foo.X = 12; - foo.Y = 14; - til::point p(foo); - VERIFY_ARE_EQUAL(foo.X, p.x()); - VERIFY_ARE_EQUAL(foo.Y, p.y()); - - POINT pt; - pt.x = 88; - pt.y = 98; - til::point q(pt); - VERIFY_ARE_EQUAL(pt.x, q.x()); - VERIFY_ARE_EQUAL(pt.y, q.y()); - - SIZE sz; - sz.cx = 11; - sz.cy = 13; - til::size r(sz); - VERIFY_ARE_EQUAL(sz.cx, r.width()); - VERIFY_ARE_EQUAL(sz.cy, r.height()); - - COORD bar; - bar.X = 57; - bar.Y = 15; - til::size s(bar); - VERIFY_ARE_EQUAL(bar.X, s.width()); - VERIFY_ARE_EQUAL(bar.Y, s.height()); - - SIZE mapSize{ 10, 10 }; - til::bitmap x(mapSize); - x.set({ 4, 4 }); - - til::rectangle area(til::point{ 5, 5 }, til::size{ 2, 2 }); - x.set(area); - - Log::Comment(L"Row 4!"); - for (auto it = x.begin_row(4); it < x.end_row(4); ++it) + const til::bitmap bitmap; + const til::size expectedSize{ 0, 0 }; + const til::rectangle expectedRect{ 0, 0, 0, 0 }; + VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); + VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); + VERIFY_ARE_EQUAL(0, bitmap._bits.size()); + VERIFY_ARE_EQUAL(true, bitmap._empty); + } + + TEST_METHOD(SizeConstruct) + { + const til::size expectedSize{ 5, 10 }; + const til::rectangle expectedRect{ 0, 0, 5, 10 }; + const til::bitmap bitmap{ expectedSize }; + VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); + VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); + VERIFY_ARE_EQUAL(50, bitmap._bits.size()); + VERIFY_ARE_EQUAL(true, bitmap._empty); + } + + TEST_METHOD(SetReset) + { + const til::size sz{ 4, 4 }; + til::bitmap bitmap{ sz }; + + // Every bit should be false. + Log::Comment(L"All bits false on creation."); + for (auto bit : bitmap._bits) + { + VERIFY_IS_FALSE(bit); + } + + const til::point point{ 2, 2 }; + bitmap.set(point); + + // Point 2,2 is this index in a 4x4 rectangle. + const auto index = 4 + 4 + 2; + + // Run through every bit. Only the one we set should be true. + Log::Comment(L"Only the bit we set should be true."); + for (size_t i = 0; i < bitmap._bits.size(); ++i) + { + if (i == index) + { + VERIFY_IS_TRUE(bitmap._bits[i]); + } + else + { + VERIFY_IS_FALSE(bitmap._bits[i]); + } + } + + // Reset the same one. + bitmap.reset(point); + + // Now every bit should be false. + Log::Comment(L"Unsetting the one we just set should make it false again."); + for (auto bit : bitmap._bits) + { + VERIFY_IS_FALSE(bit); + } + + Log::Comment(L"Setting all should mean they're all true."); + bitmap.set_all(); + + for (auto bit : bitmap._bits) + { + VERIFY_IS_TRUE(bit); + } + + Log::Comment(L"Resetting just the one should leave a false in a field of true."); + bitmap.reset(point); + for (size_t i = 0; i < bitmap._bits.size(); ++i) + { + if (i == index) + { + VERIFY_IS_FALSE(bitmap._bits[i]); + } + else + { + VERIFY_IS_TRUE(bitmap._bits[i]); + } + } + + Log::Comment(L"Now reset them all."); + bitmap.reset_all(); + + for (auto bit : bitmap._bits) { - if (*it) + VERIFY_IS_FALSE(bit); + } + + til::rectangle totalZone{ sz }; + Log::Comment(L"Set a rectangle of bits and test they went on."); + // 0 0 0 0 |1 1|0 0 + // 0 0 0 0 --\ |1 1|0 0 + // 0 0 0 0 --/ |1 1|0 0 + // 0 0 0 0 0 0 0 0 + til::rectangle setZone{ til::point{ 0, 0 }, til::size{ 2, 3 } }; + bitmap.set(setZone); + + for (auto pt : totalZone) + { + const auto expected = setZone.contains(pt); + const auto actual = bitmap._bits[totalZone.index_of(pt)]; + // Do it this way and not with equality so you can see it in output. + if (expected) { - Log::Comment(L"True"); + VERIFY_IS_TRUE(actual); } else { - Log::Comment(L"False"); + VERIFY_IS_FALSE(actual); } } - Log::Comment(L"All!"); - auto start = x.begin(); - auto end = x.end(); + Log::Comment(L"Reset a rectangle of bits not totally overlapped and check only they were cleared."); + // 1 1 0 0 1 1 0 0 + // 1 1 0 0 --\ 1|0 0|0 + // 1 1 0 0 --/ 1|0 0|0 + // 0 0 0 0 0 0 0 0 + til::rectangle resetZone{ til::point{ 1, 1 }, til::size{ 2, 2 } }; + bitmap.reset(resetZone); - for (const auto& y : x) + for (auto pt : totalZone) { - if (y) + const auto expected = setZone.contains(pt) && !resetZone.contains(pt); + const auto actual = bitmap._bits[totalZone.index_of(pt)]; + // Do it this way and not with equality so you can see it in output. + if (expected) { - Log::Comment(L"True"); + VERIFY_IS_TRUE(actual); } else { - Log::Comment(L"False"); + VERIFY_IS_FALSE(actual); } } + } + + TEST_METHOD(Resize) + { + Log::Comment(L"Set up a bitmap with every location flagged."); + const til::size originalSize{ 2, 2 }; + til::bitmap bitmap{ originalSize }; + + bitmap.set_all(); - SMALL_RECT smrc; - smrc.Top = 31; - smrc.Bottom = 41; - smrc.Left = 59; - smrc.Right = 265; + Log::Comment(L"Attempt resize to the same size."); + VERIFY_IS_FALSE(bitmap.resize(originalSize)); - til::rectangle smrectangle(smrc); + Log::Comment(L"Attempt resize to a new size."); + VERIFY_IS_TRUE(bitmap.resize(til::size{ 3, 3 })); + } + + TEST_METHOD(Empty) + { + Log::Comment(L"When created, it should be empty."); + til::bitmap bitmap{ til::size{ 2, 2 } }; + VERIFY_IS_TRUE(bitmap.empty()); + + Log::Comment(L"When it is modified with a set, it should be non-empty."); + bitmap.set(til::point{ 0, 0 }); + VERIFY_IS_FALSE(bitmap.empty()); - VERIFY_ARE_EQUAL(smrc.Top, smrectangle.top()); - VERIFY_ARE_EQUAL(smrc.Bottom, smrectangle.bottom()); - VERIFY_ARE_EQUAL(smrc.Left, smrectangle.left()); - VERIFY_ARE_EQUAL(smrc.Right, smrectangle.right()); + // We don't track it becoming empty again by resetting points because + // we don't know for sure it's entirely empty unless we seek through the whole thing. + // It's just supposed to be a short-circuit optimization for between frames + // when the whole thing has been processed and cleared. + Log::Comment(L"Resetting the same point, it will still report non-empty."); + bitmap.reset(til::point{ 0, 0 }); + VERIFY_IS_FALSE(bitmap.empty()); - RECT bgrc; - bgrc.top = 3; - bgrc.bottom = 5; - bgrc.left = 8; - bgrc.right = 9; + Log::Comment(L"But resetting all, it will report empty again."); + bitmap.reset_all(); + VERIFY_IS_TRUE(bitmap.empty()); - til::rectangle bgrectangle(bgrc); + Log::Comment(L"And setting all will be non-empty again."); + bitmap.set_all(); + VERIFY_IS_FALSE(bitmap.empty()); - VERIFY_ARE_EQUAL(bgrc.top, bgrectangle.top()); - VERIFY_ARE_EQUAL(bgrc.bottom, bgrectangle.bottom()); - VERIFY_ARE_EQUAL(bgrc.left, bgrectangle.left()); - VERIFY_ARE_EQUAL(bgrc.right, bgrectangle.right()); + // FUTURE: We could make resize smart and + // crop off the shrink or only invalidate the new bits added. + Log::Comment(L"And resizing should make it empty*"); + bitmap.resize(til::size{ 3, 3 }); + VERIFY_IS_TRUE(bitmap.empty()); } - TEST_METHOD(Runerator) + TEST_METHOD(Iterate) { - til::bitmap foo{ til::size{ 4, 8 } }; - foo.reset_all(); - foo.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 2 } }); + // This map --> Those runs + // 1 1 0 1 A A _ B + // 1 0 1 1 C _ D D + // 0 0 1 0 _ _ E _ + // 0 1 1 0 _ F F _ + Log::Comment(L"Set up a bitmap with some runs."); + + // Note: we'll test setting/resetting some overlapping rects and points. + + til::bitmap map{ til::size{ 4, 4 } }; + + // 0 0 0 0 |1 1 1 1| + // 0 0 0 0 |1 1 1 1| + // 0 0 0 0 --> 0 0 0 0 + // 0 0 0 0 0 0 0 0 + map.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 2 } }); + + // 1 1 1 1 1 1 1 1 + // 1 1 1 1 1|1 1|1 + // 0 0 0 0 --> 0|1 1|0 + // 0 0 0 0 0|1 1|0 + map.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 3 } }); - foo.set(til::rectangle{ til::point{ 3, 5 } }); - foo.set(til::rectangle{ til::point{ 0, 6 } }); + // 1 1 1 1 1 1 1 1 + // 1 1 1 1 1|0|1 1 + // 0 1 1 0 --> 0|0|1 0 + // 0 1 1 0 0 1 1 0 + map.reset(til::rectangle{ til::point{ 1, 1 }, til::size{ 1, 2 } }); - std::deque expectedRects; - expectedRects.push_back(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 1 } }); - expectedRects.push_back(til::rectangle{ til::point{ 1, 2 }, til::size{ 2, 1 } }); - expectedRects.push_back(til::rectangle{ til::point{ 3, 5 } }); - expectedRects.push_back(til::rectangle{ til::point{ 0, 6 } }); + // 1 1 1 1 1 1|0|1 + // 1 0 1 1 1 0 1 1 + // 0 0 1 0 --> 0 0 1 0 + // 0 1 1 0 0 1 1 0 + map.reset(til::point{ 2, 0 }); - for (auto it = foo.begin_runs(); it < foo.end_runs(); ++it) + Log::Comment(L"Building the expected run rectangles."); + + // Reminder, we're making 6 rectangle runs A-F like this: + // A A _ B + // C _ D D + // _ _ E _ + // _ F F _ + til::some expected; + expected.push_back(til::rectangle{ til::point{ 0, 0 }, til::size{ 2, 1 } }); + expected.push_back(til::rectangle{ til::point{ 3, 0 }, til::size{ 1, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 1 }, til::size{ 1, 1 } }); + expected.push_back(til::rectangle{ til::point{ 2, 1 }, til::size{ 2, 1 } }); + expected.push_back(til::rectangle{ til::point{ 2, 2 }, til::size{ 1, 1 } }); + expected.push_back(til::rectangle{ til::point{ 1, 3 }, til::size{ 2, 1 } }); + + Log::Comment(L"Run the iterator and collect the runs."); + til::some actual; + for (auto run : map) { - const auto actual = *it; - const auto expected = expectedRects.front(); + actual.push_back(run); + } + + Log::Comment(L"Verify they match what we expected."); + VERIFY_ARE_EQUAL(expected, actual); - VERIFY_ARE_EQUAL(expected, actual); + Log::Comment(L"Clear the map and iterate and make sure we get no results."); + map.reset_all(); - expectedRects.pop_front(); + expected.clear(); + actual.clear(); + for (auto run : map) + { + actual.push_back(run); } + + Log::Comment(L"Verify they're empty."); + VERIFY_ARE_EQUAL(expected, actual); + } }; diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 7e99fdd91d4..eab79afbcbe 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -725,6 +725,127 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, actual.empty()); } + TEST_METHOD(ContainsPoint) + { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:x", L"{-1000,0,4,5,6,14,15,16,1000}") + TEST_METHOD_PROPERTY(L"Data:y", L"{-1000,0,9,10,11,19,20,21,1000}") + END_TEST_METHOD_PROPERTIES() + + ptrdiff_t x, y; + VERIFY_SUCCEEDED_RETURN(TestData::TryGetValue(L"x", x)); + VERIFY_SUCCEEDED_RETURN(TestData::TryGetValue(L"y", y)); + + const til::rectangle rc{ 5, 10, 15, 20 }; + const til::point pt{ x, y }; + + const bool xInBounds = x >= 5 && x < 15; + const bool yInBounds = y >= 10 && y < 20; + const bool expected = xInBounds && yInBounds; + if (expected) + { + Log::Comment(L"Expected in bounds."); + } + else + { + Log::Comment(L"Expected OUT of bounds."); + } + + VERIFY_ARE_EQUAL(expected, rc.contains(pt)); + } + + TEST_METHOD(ContainsIndex) + { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:idx", L"{-1000,-1,0, 1,50,99,100,101, 1000}") + END_TEST_METHOD_PROPERTIES() + + ptrdiff_t idx; + VERIFY_SUCCEEDED_RETURN(TestData::TryGetValue(L"idx", idx)); + + const til::rectangle rc{ 5, 10, 15, 20 }; // 10x10 rectangle. + const ptrdiff_t area = (15 - 5) * (20 - 10); + const bool expected = idx >= 0 && idx < area; + if (expected) + { + Log::Comment(L"Expected in bounds."); + } + else + { + Log::Comment(L"Expected OUT of bounds."); + } + + VERIFY_ARE_EQUAL(expected, rc.contains(idx)); + } + + TEST_METHOD(IndexOfPoint) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + Log::Comment(L"0.) Normal in bounds."); + { + const til::point pt{ 7, 17 }; + const ptrdiff_t expected = 72; + VERIFY_ARE_EQUAL(expected, rc.index_of(pt)); + } + + Log::Comment(L"1.) Out of bounds."); + { + auto fn = [&]() { + const til::point pt{ 1, 1 }; + rc.index_of(pt); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + + Log::Comment(L"2.) Overflow."); + { + auto fn = [&]() { + constexpr const ptrdiff_t min = static_cast(0); + constexpr const ptrdiff_t max = std::numeric_limits().max(); + const til::rectangle bigRc{ min, min, max, max }; + const til::point pt{ max - 1, max - 1 }; + bigRc.index_of(pt); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(PointAtIndex) + { + const til::rectangle rc{ 5, 10, 15, 20 }; + + Log::Comment(L"0.) Normal in bounds."); + { + const ptrdiff_t index = 72; + const til::point expected{ 7, 17 }; + + VERIFY_ARE_EQUAL(expected, rc.point_at(index)); + } + + Log::Comment(L"1.) Out of bounds too low."); + { + auto fn = [&]() { + const ptrdiff_t index = -1; + rc.point_at(index); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + + Log::Comment(L"2.) Out of bounds too high."); + { + auto fn = [&]() { + const ptrdiff_t index = 1000; + rc.point_at(index); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + } + TEST_METHOD(CastToSmallRect) { Log::Comment(L"0.) Typical situation."); diff --git a/src/til/ut_til/SomeTests.cpp b/src/til/ut_til/SomeTests.cpp index c2cde75dcc2..ab9128181b9 100644 --- a/src/til/ut_til/SomeTests.cpp +++ b/src/til/ut_til/SomeTests.cpp @@ -166,6 +166,18 @@ class SomeTests VERIFY_IS_TRUE(s.empty()); } + TEST_METHOD(Clear) + { + til::some s; + VERIFY_IS_TRUE(s.empty()); + s.push_back(12); + VERIFY_IS_FALSE(s.empty()); + VERIFY_ARE_EQUAL(1, s.size()); + s.clear(); + VERIFY_IS_TRUE(s.empty()); + VERIFY_ARE_EQUAL(0, s.size()); + } + TEST_METHOD(Data) { til::some s; diff --git a/src/til/ut_til/til.unit.tests.vcxproj b/src/til/ut_til/til.unit.tests.vcxproj index b7d47a9b6da..644322da1c1 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 d03581af346..e07ae69b990 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj.filters +++ b/src/til/ut_til/til.unit.tests.vcxproj.filters @@ -11,6 +11,7 @@ + From 69b44ae7472fc2b4fb9dd2737d1890a4e803a7b5 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 14:45:40 -0700 Subject: [PATCH 03/25] Add tests for setting/resetting points/rectangles out of bounds. Add support method for checking if rectangle contains another, add test for that too. --- src/inc/til/bitmap.h | 8 +++++++ src/inc/til/rectangle.h | 8 +++++++ src/til/ut_til/BitmapTests.cpp | 40 +++++++++++++++++++++++++++++++ src/til/ut_til/RectangleTests.cpp | 14 +++++++++++ 4 files changed, 70 insertions(+) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index fd5979be22d..9728be7038b 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -139,17 +139,23 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" void set(til::point pt) { + THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); + _bits[_rc.index_of(pt)] = true; _empty = false; } void reset(til::point pt) { + THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); + _bits[_rc.index_of(pt)] = false; } void set(til::rectangle rc) { + THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); + for (auto pt : rc) { set(pt); @@ -158,6 +164,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" void reset(til::rectangle rc) { + THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); + for (auto pt : rc) { reset(pt); diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index b18701a1f08..4834052168c 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -496,6 +496,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return index >= 0 && index < size().area(); } + constexpr bool contains(til::rectangle rc) const + { + // Union the other rectangle and ourselves. + // If the result of that didn't grow at all, then we already + // fully contained the rectangle we were given. + return (*this | rc) == *this; + } + ptrdiff_t index_of(til::point pt) const { THROW_HR_IF(E_INVALIDARG, !contains(pt)); diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 976ac6c46a8..78d6fd1856e 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -155,6 +155,46 @@ class BitmapTests } } + TEST_METHOD(SetResetExceptions) + { + til::bitmap map{ til::size{4, 4} }; + Log::Comment(L"1.) SetPoint out of bounds."); + { + auto fn = [&]() { + map.set(til::point{ 10, 10 }); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + + Log::Comment(L"2.) SetRectangle out of bounds."); + { + auto fn = [&]() { + map.set(til::rectangle{ til::point{2, 2,}, til::size{10, 10} }); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + + Log::Comment(L"3.) ResetPoint out of bounds."); + { + auto fn = [&]() { + map.reset(til::point{ 10, 10 }); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + + Log::Comment(L"4.) ResetRectangle out of bounds."); + { + auto fn = [&]() { + map.reset(til::rectangle{ til::point{2, 2,}, til::size{10, 10} }); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); + } + } + TEST_METHOD(Resize) { Log::Comment(L"Set up a bitmap with every location flagged."); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index eab79afbcbe..a05f1db3d13 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -778,6 +778,20 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, rc.contains(idx)); } + TEST_METHOD(ContainsRectangle) + { + const til::rectangle rc{ 5, 10, 15, 20 }; // 10x10 rectangle. + + const til::rectangle fitsInside{ 8, 12, 10, 18 }; + const til::rectangle spillsOut{ 0, 0, 50, 50 }; + const til::rectangle sticksOut{ 14, 12, 30, 13 }; + + VERIFY_IS_TRUE(rc.contains(rc), L"We contain ourself."); + VERIFY_IS_TRUE(rc.contains(fitsInside), L"We fully contain a smaller rectangle."); + VERIFY_IS_FALSE(rc.contains(spillsOut), L"We do not fully contain rectangle larger than us."); + VERIFY_IS_FALSE(rc.contains(sticksOut), L"We do not contain a rectangle that is smaller, but sticks out our edge."); + } + TEST_METHOD(IndexOfPoint) { const til::rectangle rc{ 5, 10, 15, 20 }; From fca07facaa739df3fee37b72f03786ec8564d22d Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 14:47:06 -0700 Subject: [PATCH 04/25] code format pass. --- src/inc/til/bitmap.h | 1 - src/inc/til/rectangle.h | 4 ++-- src/til/ut_til/BitmapTests.cpp | 15 +++++++++++---- src/til/ut_til/RectangleTests.cpp | 6 +++--- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 9728be7038b..d854722bf4c 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -57,7 +57,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: - const std::vector& _values; const til::rectangle _rc; ptrdiff_t _pos; diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 4834052168c..d399aa08770 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -76,7 +76,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _current; } - protected: + protected: point _current; const point _topLeft; const point _bottomRight; @@ -488,7 +488,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool contains(til::point pt) const { return pt.x() >= _topLeft.x() && pt.x() < _bottomRight.x() && - pt.y() >= _topLeft.y() && pt.y() < _bottomRight.y(); + pt.y() >= _topLeft.y() && pt.y() < _bottomRight.y(); } bool contains(ptrdiff_t index) const diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 78d6fd1856e..6fcfa68d9cd 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -157,7 +157,7 @@ class BitmapTests TEST_METHOD(SetResetExceptions) { - til::bitmap map{ til::size{4, 4} }; + til::bitmap map{ til::size{ 4, 4 } }; Log::Comment(L"1.) SetPoint out of bounds."); { auto fn = [&]() { @@ -170,7 +170,11 @@ class BitmapTests Log::Comment(L"2.) SetRectangle out of bounds."); { auto fn = [&]() { - map.set(til::rectangle{ til::point{2, 2,}, til::size{10, 10} }); + map.set(til::rectangle{ til::point{ + 2, + 2, + }, + til::size{ 10, 10 } }); }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); @@ -188,7 +192,11 @@ class BitmapTests Log::Comment(L"4.) ResetRectangle out of bounds."); { auto fn = [&]() { - map.reset(til::rectangle{ til::point{2, 2,}, til::size{10, 10} }); + map.reset(til::rectangle{ til::point{ + 2, + 2, + }, + til::size{ 10, 10 } }); }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); @@ -317,6 +325,5 @@ class BitmapTests Log::Comment(L"Verify they're empty."); VERIFY_ARE_EQUAL(expected, actual); - } }; diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index a05f1db3d13..108d6fa81d6 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -835,7 +835,7 @@ class RectangleTests { const ptrdiff_t index = 72; const til::point expected{ 7, 17 }; - + VERIFY_ARE_EQUAL(expected, rc.point_at(index)); } @@ -1086,7 +1086,7 @@ class RectangleTests TEST_METHOD(ConstIteratorIncrement) { - const til::rectangle rc{ til::size{2, 2} }; + const til::rectangle rc{ til::size{ 2, 2 } }; auto it = rc.begin(); auto expected = til::point{ 0, 0 }; @@ -1113,7 +1113,7 @@ class RectangleTests TEST_METHOD(ConstIteratorEquality) { const til::rectangle rc{ 5, 10, 15, 20 }; - + VERIFY_IS_TRUE(rc.begin() == rc.begin()); VERIFY_IS_FALSE(rc.begin() == rc.end()); } From 79d1f332c78e9c9406ccfc8e6c0597e7bf53f286 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 14:50:40 -0700 Subject: [PATCH 05/25] Don't include stuff at the top of bitmap. Don't use reserved naming for iterator. Fix unsigned issues for x86. --- src/inc/til/bitmap.h | 3 --- src/inc/til/rectangle.h | 18 +++++++++--------- src/til/ut_til/BitmapTests.cpp | 4 ++-- src/til/ut_til/SomeTests.cpp | 4 ++-- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index d854722bf4c..a99c01c38c7 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -3,9 +3,6 @@ #pragma once -#include -#include "size.h" - #ifdef UNIT_TESTING class BitmapTests; #endif diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index d399aa08770..a0acbae26bf 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -13,24 +13,24 @@ class RectangleTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { - class _Rectangle_const_iterator + class _rectangle_const_iterator { public: - constexpr _Rectangle_const_iterator(point topLeft, point bottomRight) : + constexpr _rectangle_const_iterator(point topLeft, point bottomRight) : _topLeft(topLeft), _bottomRight(bottomRight), _current(topLeft) { } - constexpr _Rectangle_const_iterator(point topLeft, point bottomRight, point start) : + constexpr _rectangle_const_iterator(point topLeft, point bottomRight, point start) : _topLeft(topLeft), _bottomRight(bottomRight), _current(start) { } - _Rectangle_const_iterator& operator++() + _rectangle_const_iterator& operator++() { ptrdiff_t nextX; THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.x(), 1).AssignIfValid(&nextX)); @@ -49,24 +49,24 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return (*this); } - constexpr bool operator==(const _Rectangle_const_iterator& other) const + constexpr bool operator==(const _rectangle_const_iterator& other) const { return _current == other._current && _topLeft == other._topLeft && _bottomRight == other._bottomRight; } - constexpr bool operator!=(const _Rectangle_const_iterator& other) const + constexpr bool operator!=(const _rectangle_const_iterator& other) const { return !(*this == other); } - constexpr bool operator<(const _Rectangle_const_iterator& other) const + constexpr bool operator<(const _rectangle_const_iterator& other) const { return _current < other._current; } - constexpr bool operator>(const _Rectangle_const_iterator& other) const + constexpr bool operator>(const _rectangle_const_iterator& other) const { return _current > other._current; } @@ -89,7 +89,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" class rectangle { public: - using const_iterator = _Rectangle_const_iterator; + using const_iterator = _rectangle_const_iterator; constexpr rectangle() noexcept : rectangle(til::point{ 0, 0 }, til::point{ 0, 0 }) diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 6fcfa68d9cd..35ffab3faab 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -20,7 +20,7 @@ class BitmapTests const til::rectangle expectedRect{ 0, 0, 0, 0 }; VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); - VERIFY_ARE_EQUAL(0, bitmap._bits.size()); + VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); VERIFY_ARE_EQUAL(true, bitmap._empty); } @@ -31,7 +31,7 @@ class BitmapTests const til::bitmap bitmap{ expectedSize }; VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); - VERIFY_ARE_EQUAL(50, bitmap._bits.size()); + VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); VERIFY_ARE_EQUAL(true, bitmap._empty); } diff --git a/src/til/ut_til/SomeTests.cpp b/src/til/ut_til/SomeTests.cpp index ab9128181b9..a3010176e8c 100644 --- a/src/til/ut_til/SomeTests.cpp +++ b/src/til/ut_til/SomeTests.cpp @@ -172,10 +172,10 @@ class SomeTests VERIFY_IS_TRUE(s.empty()); s.push_back(12); VERIFY_IS_FALSE(s.empty()); - VERIFY_ARE_EQUAL(1, s.size()); + VERIFY_ARE_EQUAL(1u, s.size()); s.clear(); VERIFY_IS_TRUE(s.empty()); - VERIFY_ARE_EQUAL(0, s.size()); + VERIFY_ARE_EQUAL(0u, s.size()); } TEST_METHOD(Data) From 81af5aaf2a0d0fed20766cb61dcb5e70dfff5dd6 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 14:54:31 -0700 Subject: [PATCH 06/25] Fix comment --- src/inc/til/bitmap.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index a99c01c38c7..595d7b566ef 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -172,7 +172,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { // .clear() then .resize(_size(), true) throws an assert (unsupported operation) // .assign(_size(), true) throws an assert (unsupported operation) - set(_rc); } @@ -187,7 +186,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // True if we resized. False if it was the same size as before. bool resize(til::size size) { - // Don't resize if it's not different as we mark the whole thing dirty on resize. + // Don't resize if it's not different if (_sz != size) { _sz = size; From 09139c449eaf703ddebe8218f95ee2933a78db67 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 15:02:05 -0700 Subject: [PATCH 07/25] SA fixes. --- src/inc/til/bitmap.h | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 595d7b566ef..b0ae7377576 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -28,27 +28,27 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return (*this); } - bool operator==(const _bitmap_const_iterator& other) const + constexpr bool operator==(const _bitmap_const_iterator& other) const noexcept { return _pos == other._pos && _values == other._values; } - bool operator!=(const _bitmap_const_iterator& other) const + constexpr bool operator!=(const _bitmap_const_iterator& other) const noexcept { return !(*this == other); } - bool operator<(const _bitmap_const_iterator& other) const + constexpr bool operator<(const _bitmap_const_iterator& other) const noexcept { return _pos < other._pos; } - bool operator>(const _bitmap_const_iterator& other) const + constexpr bool operator>(const _bitmap_const_iterator& other) const noexcept { return _pos > other._pos; } - til::rectangle operator*() const + constexpr til::rectangle operator*() const noexcept { return _run; } @@ -110,8 +110,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" public: using const_iterator = _bitmap_const_iterator; - bitmap() : - bitmap(til::size{ 0, 0 }) + bitmap() noexcept : + _sz{}, + _rc{}, + _bits{}, + _empty{ true } { } @@ -133,36 +136,36 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _bitmap_const_iterator(_bits, _sz, _sz.area()); } - void set(til::point pt) + void set(const til::point pt) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); - _bits[_rc.index_of(pt)] = true; + til::at(_bits, _rc.index_of(pt)) = true; _empty = false; } - void reset(til::point pt) + void reset(const til::point pt) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); - _bits[_rc.index_of(pt)] = false; + til::at(_bits, _rc.index_of(pt)) = false; } - void set(til::rectangle rc) + void set(const til::rectangle rc) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); - for (auto pt : rc) + for (const auto pt : rc) { set(pt); } } - void reset(til::rectangle rc) + void reset(const til::rectangle rc) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); - for (auto pt : rc) + for (const auto pt : rc) { reset(pt); } From 43f952c183a6751c0da0b0872ef45058efe0df1a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 17 Mar 2020 16:06:47 -0700 Subject: [PATCH 08/25] Change the entire interface to just use til::rectangle. --- src/interactivity/onecore/BgfxEngine.cpp | 2 +- src/interactivity/onecore/BgfxEngine.hpp | 2 +- src/renderer/base/renderer.cpp | 2 +- src/renderer/dx/DxRenderer.cpp | 2 +- src/renderer/dx/DxRenderer.hpp | 2 +- src/renderer/gdi/gdirenderer.hpp | 2 +- src/renderer/gdi/math.cpp | 2 +- src/renderer/inc/IRenderEngine.hpp | 2 +- src/renderer/uia/UiaRenderer.cpp | 2 +- src/renderer/uia/UiaRenderer.hpp | 2 +- src/renderer/vt/math.cpp | 2 +- src/renderer/vt/vtrenderer.hpp | 2 +- src/renderer/wddmcon/WddmConRenderer.cpp | 2 +- src/renderer/wddmcon/WddmConRenderer.hpp | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index ab6abe8b765..bfdcfa720f3 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -232,7 +232,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid return S_OK; } -std::vector BgfxEngine::GetDirtyArea() +std::vector BgfxEngine::GetDirtyArea() { SMALL_RECT r; r.Bottom = _displayHeight > 0 ? (SHORT)(_displayHeight - 1) : 0; diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 786da48d3f6..d2fc5488bbf 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -69,7 +69,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT GetProposedFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo, int const iDpi) noexcept override; - std::vector GetDirtyArea() override; + std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 0d9827a71e0..dfe4bdd3a6d 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -862,7 +862,7 @@ void Renderer::_PaintOverlay(IRenderEngine& engine, // Set it up in a Viewport helper structure and trim it the IME viewport to be within the full console viewport. Viewport viewConv = Viewport::FromInclusive(srCaView); - for (auto srDirty : engine.GetDirtyArea()) + for (SMALL_RECT srDirty : engine.GetDirtyArea()) { // Dirty is an inclusive rectangle, but oddly enough the IME was an exclusive one, so correct it. srDirty.Bottom++; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 2ed766fdcbc..1345e6b23d1 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1686,7 +1686,7 @@ float DxEngine::GetScaling() const noexcept // - // Return Value: // - Rectangle describing dirty area in characters. -[[nodiscard]] std::vector DxEngine::GetDirtyArea() +[[nodiscard]] std::vector DxEngine::GetDirtyArea() { SMALL_RECT r; r.Top = gsl::narrow(floor(_invalidRect.top / _glyphCell.cy)); diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 68ef13b35a1..b8521d9b1ee 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -95,7 +95,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT GetProposedFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo, int const iDpi) noexcept override; - [[nodiscard]] std::vector GetDirtyArea() override; + [[nodiscard]] std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 356ce0deebd..50f2d543af0 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -68,7 +68,7 @@ namespace Microsoft::Console::Render _Out_ FontInfo& Font, const int iDpi) noexcept override; - [[nodiscard]] std::vector GetDirtyArea() override; + [[nodiscard]] std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; diff --git a/src/renderer/gdi/math.cpp b/src/renderer/gdi/math.cpp index 7b2372ba083..7b5f5ccc684 100644 --- a/src/renderer/gdi/math.cpp +++ b/src/renderer/gdi/math.cpp @@ -16,7 +16,7 @@ using namespace Microsoft::Console::Render; // Return Value: // - The character dimensions of the current dirty area of the frame. // This is an Inclusive rect. -std::vector GdiEngine::GetDirtyArea() +std::vector GdiEngine::GetDirtyArea() { RECT rc = _psInvalidData.rcPaint; diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 97951577b00..a55e0b64b3b 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -117,7 +117,7 @@ namespace Microsoft::Console::Render _Out_ FontInfo& FontInfo, const int iDpi) noexcept = 0; - virtual std::vector GetDirtyArea() = 0; + virtual std::vector GetDirtyArea() = 0; [[nodiscard]] virtual HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept = 0; [[nodiscard]] virtual HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateTitle(const std::wstring& newTitle) noexcept = 0; diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index 6a20a7a171b..cf42b8a036d 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -403,7 +403,7 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : // - // Return Value: // - Rectangle describing dirty area in characters. -[[nodiscard]] std::vector UiaEngine::GetDirtyArea() +[[nodiscard]] std::vector UiaEngine::GetDirtyArea() { return { Viewport::Empty().ToInclusive() }; } diff --git a/src/renderer/uia/UiaRenderer.hpp b/src/renderer/uia/UiaRenderer.hpp index 4b0eb55b13f..24269ee9cf9 100644 --- a/src/renderer/uia/UiaRenderer.hpp +++ b/src/renderer/uia/UiaRenderer.hpp @@ -71,7 +71,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT GetProposedFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo, int const iDpi) noexcept override; - [[nodiscard]] std::vector GetDirtyArea() override; + [[nodiscard]] std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; diff --git a/src/renderer/vt/math.cpp b/src/renderer/vt/math.cpp index f7faad04778..bd1f81e8f65 100644 --- a/src/renderer/vt/math.cpp +++ b/src/renderer/vt/math.cpp @@ -17,7 +17,7 @@ using namespace Microsoft::Console::Types; // Return Value: // - The character dimensions of the current dirty area of the frame. // This is an Inclusive rect. -std::vector VtEngine::GetDirtyArea() +std::vector VtEngine::GetDirtyArea() { SMALL_RECT dirty = _invalidRect.ToInclusive(); if (dirty.Top < _virtualTop) diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 8ce51bbcf1c..6db9c5efb9f 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -89,7 +89,7 @@ namespace Microsoft::Console::Render _Out_ FontInfo& Font, const int iDpi) noexcept override; - std::vector GetDirtyArea() override; + std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index 475fb85125a..5819511c0a3 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -355,7 +355,7 @@ bool WddmConEngine::IsInitialized() return S_OK; } -std::vector WddmConEngine::GetDirtyArea() +std::vector WddmConEngine::GetDirtyArea() { SMALL_RECT r; r.Bottom = _displayHeight > 0 ? (SHORT)(_displayHeight - 1) : 0; diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index 1818ae3c649..ad137131c77 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -61,7 +61,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT GetProposedFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo, int const iDpi) noexcept override; - std::vector GetDirtyArea() override; + std::vector GetDirtyArea() override; [[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override; [[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override; From de20c5ab6bdc9ea3fba6922ff1c19feefaee421b Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 18 Mar 2020 09:43:56 -0700 Subject: [PATCH 09/25] make bitmap track a rectangle inside. remove empty and add bitset methods of all/any/none. also add convenience one() method in prediction of what conpty will be looking for. --- src/inc/til/bitmap.h | 222 +++++++++++++++---------------- src/inc/til/rectangle.h | 15 +++ src/til/ut_til/BitmapTests.cpp | 232 +++++++++++++++++++-------------- 3 files changed, 261 insertions(+), 208 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index b0ae7377576..33ac91bec47 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -9,112 +9,115 @@ class BitmapTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { - class _bitmap_const_iterator + namespace details { - public: - _bitmap_const_iterator(const std::vector& values, til::rectangle rc, ptrdiff_t pos) : - _values(values), - _rc(rc), - _pos(pos), - _end(rc.size().area()) - { - _calculateArea(); - } - - _bitmap_const_iterator& operator++() - { - _pos = _nextPos; - _calculateArea(); - return (*this); - } - - constexpr bool operator==(const _bitmap_const_iterator& other) const noexcept - { - return _pos == other._pos && _values == other._values; - } - - constexpr bool operator!=(const _bitmap_const_iterator& other) const noexcept - { - return !(*this == other); - } + class _bitmap_const_iterator + { + public: + _bitmap_const_iterator(const std::vector& values, til::rectangle rc, ptrdiff_t pos) : + _values(values), + _rc(rc), + _pos(pos), + _end(rc.size().area()) + { + _calculateArea(); + } - constexpr bool operator<(const _bitmap_const_iterator& other) const noexcept - { - return _pos < other._pos; - } + _bitmap_const_iterator& operator++() + { + _pos = _nextPos; + _calculateArea(); + return (*this); + } - constexpr bool operator>(const _bitmap_const_iterator& other) const noexcept - { - return _pos > other._pos; - } + constexpr bool operator==(const _bitmap_const_iterator& other) const noexcept + { + return _pos == other._pos && _values == other._values; + } - constexpr til::rectangle operator*() const noexcept - { - return _run; - } + constexpr bool operator!=(const _bitmap_const_iterator& other) const noexcept + { + return !(*this == other); + } - private: - const std::vector& _values; - const til::rectangle _rc; - ptrdiff_t _pos; - ptrdiff_t _nextPos; - const ptrdiff_t _end; - til::rectangle _run; - - void _calculateArea() - { - // Backup the position as the next one. - _nextPos = _pos; + constexpr bool operator<(const _bitmap_const_iterator& other) const noexcept + { + return _pos < other._pos; + } - // Seek forward until we find an on bit. - while (_nextPos < _end && !_values.at(_nextPos)) + constexpr bool operator>(const _bitmap_const_iterator& other) const noexcept { - ++_nextPos; + return _pos > other._pos; } - // If we haven't reached the end yet... - if (_nextPos < _end) + constexpr til::rectangle operator*() const noexcept { - // pos is now at the first on bit. - const auto runStart = _rc.point_at(_nextPos); + return _run; + } - // We'll only count up until the end of this row. - // a run can be a max of one row tall. - const ptrdiff_t rowEndIndex = _rc.index_of(til::point(_rc.right() - 1, runStart.y())) + 1; + private: + const std::vector& _values; + const til::rectangle _rc; + ptrdiff_t _pos; + ptrdiff_t _nextPos; + const ptrdiff_t _end; + til::rectangle _run; - // Find the length for the rectangle. - ptrdiff_t runLength = 0; + void _calculateArea() + { + // Backup the position as the next one. + _nextPos = _pos; - // We have at least 1 so start with a do/while. - do + // Seek forward until we find an on bit. + while (_nextPos < _end && !_values.at(_nextPos)) { ++_nextPos; - ++runLength; - } while (_nextPos < _end && _nextPos < rowEndIndex && _values.at(_nextPos)); - // Keep going until we reach end of row, end of the buffer, or the next bit is off. + } - // Assemble and store that run. - _run = til::rectangle{ runStart, til::size{ runLength, static_cast(1) } }; - } - else - { - // If we reached the end, set the pos because the run is empty. - _pos = _nextPos; - _run = til::rectangle{}; + // If we haven't reached the end yet... + if (_nextPos < _end) + { + // pos is now at the first on bit. + const auto runStart = _rc.point_at(_nextPos); + + // We'll only count up until the end of this row. + // a run can be a max of one row tall. + const ptrdiff_t rowEndIndex = _rc.index_of(til::point(_rc.right() - 1, runStart.y())) + 1; + + // Find the length for the rectangle. + ptrdiff_t runLength = 0; + + // We have at least 1 so start with a do/while. + do + { + ++_nextPos; + ++runLength; + } while (_nextPos < rowEndIndex && _values.at(_nextPos)); + // Keep going until we reach end of row, end of the buffer, or the next bit is off. + + // Assemble and store that run. + _run = til::rectangle{ runStart, til::size{ runLength, static_cast(1) } }; + } + else + { + // If we reached the end, set the pos because the run is empty. + _pos = _nextPos; + _run = til::rectangle{}; + } } - } - }; + }; + } class bitmap { public: - using const_iterator = _bitmap_const_iterator; + using const_iterator = details::_bitmap_const_iterator; bitmap() noexcept : _sz{}, _rc{}, _bits{}, - _empty{ true } + _dirty{} { } @@ -122,18 +125,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(sz.area(), false), - _empty(true) + _dirty() { } const_iterator begin() const { - return _bitmap_const_iterator(_bits, _sz, 0); + return const_iterator(_bits, _sz, 0); } const_iterator end() const { - return _bitmap_const_iterator(_bits, _sz, _sz.area()); + return const_iterator(_bits, _sz, _sz.area()); } void set(const til::point pt) @@ -141,14 +144,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); til::at(_bits, _rc.index_of(pt)) = true; - _empty = false; - } - - void reset(const til::point pt) - { - THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); - - til::at(_bits, _rc.index_of(pt)) = false; + _dirty |= til::rectangle{ pt }; } void set(const til::rectangle rc) @@ -157,33 +153,27 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" for (const auto pt : rc) { - set(pt); + til::at(_bits, _rc.index_of(pt)) = true; } - } - void reset(const til::rectangle rc) - { - THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); - - for (const auto pt : rc) - { - reset(pt); - } + _dirty |= rc; } void set_all() { // .clear() then .resize(_size(), true) throws an assert (unsupported operation) // .assign(_size(), true) throws an assert (unsupported operation) - set(_rc); + _bits = std::vector(_sz.area(), true); + _dirty = _rc; } void reset_all() { // .clear() then .resize(_size(), false) throws an assert (unsupported operation) // .assign(_size(), false) throws an assert (unsupported operation) - reset(_rc); - _empty = true; + + _bits = std::vector(_sz.area(), false); + _dirty = {}; } // True if we resized. False if it was the same size as before. @@ -195,8 +185,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz = size; _rc = til::rectangle{ size }; // .resize(_size(), true/false) throws an assert (unsupported operation) - _bits = std::vector(_sz.area(), false); - _empty = true; + reset_all(); return true; } else @@ -205,13 +194,28 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } } - constexpr bool empty() const + bool one() const + { + return _dirty.size() == til::size{ 1, 1 }; + } + + bool any() const + { + return !none(); + } + + bool none() const + { + return _dirty.empty(); + } + + bool all() const { - return _empty; + return _dirty == _rc; } private: - bool _empty; + til::rectangle _dirty; til::size _sz; til::rectangle _rc; std::vector _bits; diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index a0acbae26bf..e380939f47c 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -39,6 +39,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { ptrdiff_t nextY; THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.y(), 1).AssignIfValid(&nextY)); + // Note for the standard Left-to-Right, Top-to-Bottom walk, + // the end position is one cell below the bottom left. + // (or more accurately, on the exclusive bottom line in the inclusive left column.) _current = { _topLeft.x(), nextY }; } else @@ -197,6 +200,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr const_iterator end() const { + // For the standard walk: Left-To-Right then Top-To-Bottom + // the end box is one cell below the left most column. + // |----| 5x2 square. Remember bottom & right are exclusive + // | | while top & left are inclusive. + // X----- X is the end position. + return const_iterator(_topLeft, _bottomRight, { _topLeft.x(), _bottomRight.y() }); } @@ -232,6 +241,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return rectangle{ l, t, r, b }; } + constexpr rectangle& operator|=(const rectangle& other) noexcept + { + *this = *this | other; + return *this; + } + // AND = intersect constexpr rectangle operator&(const rectangle& other) const noexcept { diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 35ffab3faab..e304e47f5ef 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -21,7 +21,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(true, bitmap._empty); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); } TEST_METHOD(SizeConstruct) @@ -32,7 +32,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(true, bitmap._empty); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); } TEST_METHOD(SetReset) @@ -67,16 +67,6 @@ class BitmapTests } } - // Reset the same one. - bitmap.reset(point); - - // Now every bit should be false. - Log::Comment(L"Unsetting the one we just set should make it false again."); - for (auto bit : bitmap._bits) - { - VERIFY_IS_FALSE(bit); - } - Log::Comment(L"Setting all should mean they're all true."); bitmap.set_all(); @@ -85,20 +75,6 @@ class BitmapTests VERIFY_IS_TRUE(bit); } - Log::Comment(L"Resetting just the one should leave a false in a field of true."); - bitmap.reset(point); - for (size_t i = 0; i < bitmap._bits.size(); ++i) - { - if (i == index) - { - VERIFY_IS_FALSE(bitmap._bits[i]); - } - else - { - VERIFY_IS_TRUE(bitmap._bits[i]); - } - } - Log::Comment(L"Now reset them all."); bitmap.reset_all(); @@ -131,27 +107,12 @@ class BitmapTests } } - Log::Comment(L"Reset a rectangle of bits not totally overlapped and check only they were cleared."); - // 1 1 0 0 1 1 0 0 - // 1 1 0 0 --\ 1|0 0|0 - // 1 1 0 0 --/ 1|0 0|0 - // 0 0 0 0 0 0 0 0 - til::rectangle resetZone{ til::point{ 1, 1 }, til::size{ 2, 2 } }; - bitmap.reset(resetZone); + Log::Comment(L"Reset all."); + bitmap.reset_all(); for (auto pt : totalZone) { - const auto expected = setZone.contains(pt) && !resetZone.contains(pt); - const auto actual = bitmap._bits[totalZone.index_of(pt)]; - // Do it this way and not with equality so you can see it in output. - if (expected) - { - VERIFY_IS_TRUE(actual); - } - else - { - VERIFY_IS_FALSE(actual); - } + VERIFY_IS_FALSE(bitmap._bits[totalZone.index_of(pt)]); } } @@ -179,28 +140,6 @@ class BitmapTests VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); } - - Log::Comment(L"3.) ResetPoint out of bounds."); - { - auto fn = [&]() { - map.reset(til::point{ 10, 10 }); - }; - - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); - } - - Log::Comment(L"4.) ResetRectangle out of bounds."); - { - auto fn = [&]() { - map.reset(til::rectangle{ til::point{ - 2, - 2, - }, - til::size{ 10, 10 } }); - }; - - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; }); - } } TEST_METHOD(Resize) @@ -218,37 +157,126 @@ class BitmapTests VERIFY_IS_TRUE(bitmap.resize(til::size{ 3, 3 })); } - TEST_METHOD(Empty) + TEST_METHOD(One) { - Log::Comment(L"When created, it should be empty."); + Log::Comment(L"When created, it should be not be one."); til::bitmap bitmap{ til::size{ 2, 2 } }; - VERIFY_IS_TRUE(bitmap.empty()); + VERIFY_IS_FALSE(bitmap.one()); + + Log::Comment(L"When a single point is set, it should be one."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.one()); - Log::Comment(L"When it is modified with a set, it should be non-empty."); + Log::Comment(L"Setting the same point again, should still be one."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.one()); + + Log::Comment(L"Setting another point, it should no longer be one."); bitmap.set(til::point{ 0, 0 }); - VERIFY_IS_FALSE(bitmap.empty()); + VERIFY_IS_FALSE(bitmap.one()); + + Log::Comment(L"Clearing it, still not one."); + bitmap.reset_all(); + VERIFY_IS_FALSE(bitmap.one()); + + Log::Comment(L"Set one point, one again."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.one()); + + Log::Comment(L"And setting all will no longer be one again."); + bitmap.set_all(); + VERIFY_IS_FALSE(bitmap.one()); + } + + TEST_METHOD(Any) + { + Log::Comment(L"When created, it should be not be any."); + til::bitmap bitmap{ til::size{ 2, 2 } }; + VERIFY_IS_FALSE(bitmap.any()); + + Log::Comment(L"When a single point is set, it should be any."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.any()); - // We don't track it becoming empty again by resetting points because - // we don't know for sure it's entirely empty unless we seek through the whole thing. - // It's just supposed to be a short-circuit optimization for between frames - // when the whole thing has been processed and cleared. - Log::Comment(L"Resetting the same point, it will still report non-empty."); - bitmap.reset(til::point{ 0, 0 }); - VERIFY_IS_FALSE(bitmap.empty()); + Log::Comment(L"Setting the same point again, should still be any."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.any()); - Log::Comment(L"But resetting all, it will report empty again."); + Log::Comment(L"Setting another point, it should still be any."); + bitmap.set(til::point{ 0, 0 }); + VERIFY_IS_TRUE(bitmap.any()); + + Log::Comment(L"Clearing it, no longer any."); bitmap.reset_all(); - VERIFY_IS_TRUE(bitmap.empty()); + VERIFY_IS_FALSE(bitmap.any()); + + Log::Comment(L"Set one point, one again, it's any."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(bitmap.any()); - Log::Comment(L"And setting all will be non-empty again."); + Log::Comment(L"And setting all will be any as well."); bitmap.set_all(); - VERIFY_IS_FALSE(bitmap.empty()); + VERIFY_IS_TRUE(bitmap.any()); + } + + TEST_METHOD(None) + { + Log::Comment(L"When created, it should be none."); + til::bitmap bitmap{ til::size{ 2, 2 } }; + VERIFY_IS_TRUE(bitmap.none()); + + Log::Comment(L"When it is modified with a set, it should no longer be none."); + bitmap.set(til::point{ 0, 0 }); + VERIFY_IS_FALSE(bitmap.none()); + + Log::Comment(L"Resetting all, it will report none again."); + bitmap.reset_all(); + VERIFY_IS_TRUE(bitmap.none()); + + Log::Comment(L"And setting all will no longer be none again."); + bitmap.set_all(); + VERIFY_IS_FALSE(bitmap.none()); // FUTURE: We could make resize smart and // crop off the shrink or only invalidate the new bits added. Log::Comment(L"And resizing should make it empty*"); bitmap.resize(til::size{ 3, 3 }); - VERIFY_IS_TRUE(bitmap.empty()); + VERIFY_IS_TRUE(bitmap.none()); + } + + TEST_METHOD(All) + { + Log::Comment(L"When created, it should be not be all."); + til::bitmap bitmap{ til::size{ 2, 2 } }; + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"When a single point is set, it should not be all."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"Setting the same point again, should still not be all."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"Setting another point, it should still not be all."); + bitmap.set(til::point{ 0, 0 }); + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"Clearing it, still not all."); + bitmap.reset_all(); + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"Set one point, one again, not all."); + bitmap.set(til::point{ 1, 0 }); + VERIFY_IS_FALSE(bitmap.all()); + + Log::Comment(L"And setting all will finally be all."); + bitmap.set_all(); + VERIFY_IS_TRUE(bitmap.all()); + + Log::Comment(L"Clearing it, back to not all."); + bitmap.reset_all(); + VERIFY_IS_FALSE(bitmap.all()); } TEST_METHOD(Iterate) @@ -264,29 +292,35 @@ class BitmapTests til::bitmap map{ til::size{ 4, 4 } }; - // 0 0 0 0 |1 1 1 1| - // 0 0 0 0 |1 1 1 1| + // 0 0 0 0 |1 1|0 0 + // 0 0 0 0 0 0 0 0 // 0 0 0 0 --> 0 0 0 0 // 0 0 0 0 0 0 0 0 - map.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 2 } }); + map.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 2, 1 } }); - // 1 1 1 1 1 1 1 1 - // 1 1 1 1 1|1 1|1 - // 0 0 0 0 --> 0|1 1|0 - // 0 0 0 0 0|1 1|0 - map.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 3 } }); + // 1 1 0 0 1 1 0 0 + // 0 0 0 0 0 0|1|0 + // 0 0 0 0 --> 0 0|1|0 + // 0 0 0 0 0 0|1|0 + map.set(til::rectangle{ til::point{ 2, 1 }, til::size{ 1, 3 } }); - // 1 1 1 1 1 1 1 1 - // 1 1 1 1 1|0|1 1 - // 0 1 1 0 --> 0|0|1 0 - // 0 1 1 0 0 1 1 0 - map.reset(til::rectangle{ til::point{ 1, 1 }, til::size{ 1, 2 } }); + // 1 1 0 0 1 1 0|1| + // 0 0 1 0 0 0 1|1| + // 0 0 1 0 --> 0 0 1 0 + // 0 0 1 0 0 0 1 0 + map.set(til::rectangle{ til::point{ 3, 0 }, til::size{ 1, 2 } }); + + // 1 1 0 1 1 1 0 1 + // 0 0 1 1 |1|0 1 1 + // 0 0 1 0 --> 0 0 1 0 + // 0 0 1 0 0 0 1 0 + map.set(til::point{ 0, 1 }); - // 1 1 1 1 1 1|0|1 + // 1 1 0 1 1 1 0 1 // 1 0 1 1 1 0 1 1 // 0 0 1 0 --> 0 0 1 0 - // 0 1 1 0 0 1 1 0 - map.reset(til::point{ 2, 0 }); + // 0 0 1 0 0|1|1 0 + map.set(til::point{ 1, 3 }); Log::Comment(L"Building the expected run rectangles."); From a86b8fa6dd1599f38ce027550a9fc4a1f621ff43 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 18 Mar 2020 10:27:17 -0700 Subject: [PATCH 10/25] Some frees members, some comments about the rectangle iterator and move it to details namespace. --- src/inc/til/rectangle.h | 123 +++++++++++++++--------------- src/inc/til/some.h | 13 ++++ src/til/ut_til/RectangleTests.cpp | 6 ++ src/til/ut_til/SomeTests.cpp | 22 ++++++ 4 files changed, 104 insertions(+), 60 deletions(-) diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index e380939f47c..49fd6714993 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -13,86 +13,89 @@ class RectangleTests; namespace til // Terminal Implementation Library. Also: "Today I Learned" { - class _rectangle_const_iterator + namespace details { - public: - constexpr _rectangle_const_iterator(point topLeft, point bottomRight) : - _topLeft(topLeft), - _bottomRight(bottomRight), - _current(topLeft) - { - } - - constexpr _rectangle_const_iterator(point topLeft, point bottomRight, point start) : - _topLeft(topLeft), - _bottomRight(bottomRight), - _current(start) + class _rectangle_const_iterator { - } - - _rectangle_const_iterator& operator++() - { - ptrdiff_t nextX; - THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.x(), 1).AssignIfValid(&nextX)); - - if (nextX >= _bottomRight.x()) + public: + constexpr _rectangle_const_iterator(point topLeft, point bottomRight) : + _topLeft(topLeft), + _bottomRight(bottomRight), + _current(topLeft) { - ptrdiff_t nextY; - THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.y(), 1).AssignIfValid(&nextY)); - // Note for the standard Left-to-Right, Top-to-Bottom walk, - // the end position is one cell below the bottom left. - // (or more accurately, on the exclusive bottom line in the inclusive left column.) - _current = { _topLeft.x(), nextY }; } - else + + constexpr _rectangle_const_iterator(point topLeft, point bottomRight, point start) : + _topLeft(topLeft), + _bottomRight(bottomRight), + _current(start) { - _current = { nextX, _current.y() }; } - return (*this); - } + _rectangle_const_iterator& operator++() + { + ptrdiff_t nextX; + THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.x(), 1).AssignIfValid(&nextX)); - constexpr bool operator==(const _rectangle_const_iterator& other) const - { - return _current == other._current && - _topLeft == other._topLeft && - _bottomRight == other._bottomRight; - } + if (nextX >= _bottomRight.x()) + { + ptrdiff_t nextY; + THROW_HR_IF(E_ABORT, !::base::CheckAdd(_current.y(), 1).AssignIfValid(&nextY)); + // Note for the standard Left-to-Right, Top-to-Bottom walk, + // the end position is one cell below the bottom left. + // (or more accurately, on the exclusive bottom line in the inclusive left column.) + _current = { _topLeft.x(), nextY }; + } + else + { + _current = { nextX, _current.y() }; + } - constexpr bool operator!=(const _rectangle_const_iterator& other) const - { - return !(*this == other); - } + return (*this); + } - constexpr bool operator<(const _rectangle_const_iterator& other) const - { - return _current < other._current; - } + constexpr bool operator==(const _rectangle_const_iterator& other) const + { + return _current == other._current && + _topLeft == other._topLeft && + _bottomRight == other._bottomRight; + } - constexpr bool operator>(const _rectangle_const_iterator& other) const - { - return _current > other._current; - } + constexpr bool operator!=(const _rectangle_const_iterator& other) const + { + return !(*this == other); + } - constexpr point operator*() const - { - return _current; - } + constexpr bool operator<(const _rectangle_const_iterator& other) const + { + return _current < other._current; + } - protected: - point _current; - const point _topLeft; - const point _bottomRight; + constexpr bool operator>(const _rectangle_const_iterator& other) const + { + return _current > other._current; + } + + constexpr point operator*() const + { + return _current; + } + + protected: + point _current; + const point _topLeft; + const point _bottomRight; #ifdef UNIT_TESTING - friend class ::RectangleTests; + friend class ::RectangleTests; #endif - }; + }; + } class rectangle { public: - using const_iterator = _rectangle_const_iterator; + using const_iterator = details::_rectangle_const_iterator; constexpr rectangle() noexcept : rectangle(til::point{ 0, 0 }, til::point{ 0, 0 }) diff --git a/src/inc/til/some.h b/src/inc/til/some.h index c8da45d0742..9620bdce777 100644 --- a/src/inc/til/some.h +++ b/src/inc/til/some.h @@ -130,6 +130,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr void clear() noexcept { _used = 0; + _array = {}; // should free members, if necessary. } constexpr const_reference at(size_type pos) const @@ -174,6 +175,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" ++_used; } + void push_back(T&& val) + { + if (_used >= N) + { + _outOfRange(); + } + + til::at(_array, _used) = std::move(val); + + ++_used; + } + void pop_back() { if (_used <= 0) diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 108d6fa81d6..47e17da9430 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -1108,6 +1108,12 @@ class RectangleTests expected = til::point{ 0, 2 }; VERIFY_ARE_EQUAL(expected, *it); VERIFY_ARE_EQUAL(expected, *rc.end()); + + // We wouldn't normally walk one past, but validate it keeps going + // like any STL iterator would. + ++it; + expected = til::point{ 1, 2 }; + VERIFY_ARE_EQUAL(expected, *it); } TEST_METHOD(ConstIteratorEquality) diff --git a/src/til/ut_til/SomeTests.cpp b/src/til/ut_til/SomeTests.cpp index a3010176e8c..98ea25fd64d 100644 --- a/src/til/ut_til/SomeTests.cpp +++ b/src/til/ut_til/SomeTests.cpp @@ -178,6 +178,28 @@ class SomeTests VERIFY_ARE_EQUAL(0u, s.size()); } + TEST_METHOD(ClearFreesMembers) + { + til::some, 2> s; + + auto a = std::make_shared(4); + auto weakA = std::weak_ptr(a); + + auto b = std::make_shared(6); + auto weakB = std::weak_ptr(b); + + s.push_back(std::move(a)); + s.push_back(std::move(b)); + + VERIFY_IS_FALSE(weakA.expired()); + VERIFY_IS_FALSE(weakB.expired()); + + s.clear(); + + VERIFY_IS_TRUE(weakA.expired()); + VERIFY_IS_TRUE(weakB.expired()); + } + TEST_METHOD(Data) { til::some s; From c4df795a1972c8bc2dda4bc5f2d700f540a7ccb2 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 18 Mar 2020 15:58:56 -0700 Subject: [PATCH 11/25] Make resize work and add tests. Add constructors that let you allocate a bitmap already filled. Generalize the bit checking method in the tests. --- src/inc/til/bitmap.h | 54 ++++++++-- src/inc/til/rectangle.h | 4 +- src/til/ut_til/BitmapTests.cpp | 178 ++++++++++++++++++++++++--------- 3 files changed, 179 insertions(+), 57 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 33ac91bec47..88ab038e3d9 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -122,10 +122,15 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } bitmap(til::size sz) : + bitmap(sz, false) + { + } + + bitmap(til::size sz, bool fill) : _sz(sz), _rc(sz), - _bits(sz.area(), false), - _dirty() + _bits(sz.area(), fill), + _dirty(fill ? sz : til::rectangle{}) { } @@ -177,15 +182,50 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // True if we resized. False if it was the same size as before. - bool resize(til::size size) + // Set fill if you want the new region (on growing) to be marked dirty. + bool resize(til::size size, bool fill = false) { + // FYI .resize(_size(), true/false) throws an assert (unsupported operation) + // Don't resize if it's not different if (_sz != size) { - _sz = size; - _rc = til::rectangle{ size }; - // .resize(_size(), true/false) throws an assert (unsupported operation) - reset_all(); + // Make a new bitmap for the other side, empty initially. + auto newMap = bitmap(size, false); + + // Copy any regions that overlap from this map to the new one. + // Just iterate our runs... + for (const auto run : *this) + { + // intersect them with the new map + // so we don't attempt to set bits that fit outside + // the new one. + const auto intersect = run & newMap._rc; + + // and if there is still anything left, set them. + if (!intersect.empty()) + { + newMap.set(intersect); + } + } + + // Then, if we were requested to fill the new space on growing, + // find the space in the new rectangle that wasn't in the old + // and fill it up. + if (fill) + { + // A subtraction will yield anything in the new that isn't + // a part of the old. + const auto newAreas = newMap._rc - _rc; + for (const auto& area : newAreas) + { + newMap.set(area); + } + } + + // Swap and return. + std::swap(newMap, *this); + return true; } else diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 49fd6714993..ef0676e96f2 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -57,8 +57,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool operator==(const _rectangle_const_iterator& other) const { return _current == other._current && - _topLeft == other._topLeft && - _bottomRight == other._bottomRight; + _topLeft == other._topLeft && + _bottomRight == other._bottomRight; } constexpr bool operator!=(const _rectangle_const_iterator& other) const diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index e304e47f5ef..a034c1145d1 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -13,6 +13,49 @@ class BitmapTests { TEST_CLASS(BitmapTests); + void _checkBits(const til::rectangle& bitsOn, + const til::bitmap& map) + { + _checkBits(std::vector{ bitsOn }, map); + } + + void _checkBits(const std::vector& bitsOn, + const til::bitmap& map) + { + Log::Comment(L"Check dirty rectangles."); + + // Union up all the dirty rectangles into one big one. + auto dirtyExpected = bitsOn.front(); + for (auto it = bitsOn.cbegin() + 1; it < bitsOn.cend(); ++it) + { + dirtyExpected |= *it; + } + + // Check if it matches. + VERIFY_ARE_EQUAL(dirtyExpected, map._dirty); + + Log::Comment(L"Check all bits in map."); + // For every point in the map... + for (const auto pt : map._rc) + { + // If any of the rectangles we were given contains this point, we expect it should be on. + const auto expected = std::any_of(bitsOn.cbegin(), bitsOn.cend(), [&pt](auto bitRect) { return bitRect.contains(pt); }); + + // Get the actual bit out of the map. + const auto actual = map._bits[map._rc.index_of(pt)]; + + // Do it this way and not with equality so you can see it in output. + if (expected) + { + VERIFY_IS_TRUE(actual); + } + else + { + VERIFY_IS_FALSE(actual); + } + } + } + TEST_METHOD(DefaultConstruct) { const til::bitmap bitmap; @@ -22,6 +65,10 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + + // The find will go from begin to end in the bits looking for a "true". + // It should miss so the result should be "cend" and turn out true here. + VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), true)); } TEST_METHOD(SizeConstruct) @@ -33,6 +80,42 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + + // The find will go from begin to end in the bits looking for a "true". + // It should miss so the result should be "cend" and turn out true here. + VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), true)); + } + + TEST_METHOD(SizeConstructWithFill) + { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:fill", L"{true, false}") + END_TEST_METHOD_PROPERTIES() + + bool fill; + VERIFY_SUCCEEDED_RETURN(TestData::TryGetValue(L"fill", fill)); + + const til::size expectedSize{ 5, 10 }; + const til::rectangle expectedRect{ 0, 0, 5, 10 }; + const til::bitmap bitmap{ expectedSize, fill }; + VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); + VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); + VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); + + if (!fill) + { + // The find will go from begin to end in the bits looking for a "true". + // It should miss so the result should be "cend" and turn out true here. + VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), true)); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + } + else + { + // The find will go from begin to end in the bits looking for a "false". + // It should miss so the result should be "cend" and turn out true here. + VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), false)); + VERIFY_ARE_EQUAL(expectedRect, bitmap._dirty); + } } TEST_METHOD(SetReset) @@ -50,38 +133,23 @@ class BitmapTests const til::point point{ 2, 2 }; bitmap.set(point); - // Point 2,2 is this index in a 4x4 rectangle. - const auto index = 4 + 4 + 2; + til::rectangle expectedSet{ point }; // Run through every bit. Only the one we set should be true. Log::Comment(L"Only the bit we set should be true."); - for (size_t i = 0; i < bitmap._bits.size(); ++i) - { - if (i == index) - { - VERIFY_IS_TRUE(bitmap._bits[i]); - } - else - { - VERIFY_IS_FALSE(bitmap._bits[i]); - } - } + _checkBits(expectedSet, bitmap); Log::Comment(L"Setting all should mean they're all true."); bitmap.set_all(); - for (auto bit : bitmap._bits) - { - VERIFY_IS_TRUE(bit); - } + expectedSet = til::rectangle{ bitmap._rc }; + _checkBits(expectedSet, bitmap); Log::Comment(L"Now reset them all."); bitmap.reset_all(); - for (auto bit : bitmap._bits) - { - VERIFY_IS_FALSE(bit); - } + expectedSet = {}; + _checkBits(expectedSet, bitmap); til::rectangle totalZone{ sz }; Log::Comment(L"Set a rectangle of bits and test they went on."); @@ -92,28 +160,14 @@ class BitmapTests til::rectangle setZone{ til::point{ 0, 0 }, til::size{ 2, 3 } }; bitmap.set(setZone); - for (auto pt : totalZone) - { - const auto expected = setZone.contains(pt); - const auto actual = bitmap._bits[totalZone.index_of(pt)]; - // Do it this way and not with equality so you can see it in output. - if (expected) - { - VERIFY_IS_TRUE(actual); - } - else - { - VERIFY_IS_FALSE(actual); - } - } + expectedSet = setZone; + _checkBits(expectedSet, bitmap); Log::Comment(L"Reset all."); bitmap.reset_all(); - for (auto pt : totalZone) - { - VERIFY_IS_FALSE(bitmap._bits[totalZone.index_of(pt)]); - } + expectedSet = {}; + _checkBits(expectedSet, bitmap); } TEST_METHOD(SetResetExceptions) @@ -146,15 +200,49 @@ class BitmapTests { Log::Comment(L"Set up a bitmap with every location flagged."); const til::size originalSize{ 2, 2 }; - til::bitmap bitmap{ originalSize }; + til::bitmap bitmap{ originalSize, true }; - bitmap.set_all(); + std::vector expectedFillRects; + + // 1 1 + // 1 1 + expectedFillRects.emplace_back(til::rectangle{ originalSize }); + _checkBits(expectedFillRects, bitmap); Log::Comment(L"Attempt resize to the same size."); VERIFY_IS_FALSE(bitmap.resize(originalSize)); - Log::Comment(L"Attempt resize to a new size."); + // 1 1 + // 1 1 + _checkBits(expectedFillRects, bitmap); + + Log::Comment(L"Attempt resize to a new size where both dimensions grow and we didn't ask for fill."); VERIFY_IS_TRUE(bitmap.resize(til::size{ 3, 3 })); + + // 1 1 0 + // 1 1 0 + // 0 0 0 + _checkBits(expectedFillRects, bitmap); + + Log::Comment(L"Set a bit out in the new space and check it."); + const til::point spaceBit{ 1, 2 }; + expectedFillRects.emplace_back(til::rectangle{ spaceBit }); + bitmap.set(spaceBit); + + // 1 1 0 + // 1 1 0 + // 0 1 0 + _checkBits(expectedFillRects, bitmap); + + Log::Comment(L"Grow vertically and shrink horizontally at the same time. Fill any new space."); + expectedFillRects.emplace_back(til::rectangle{ til::point{ 0, 3 }, til::size{ 2, 1 } }); + bitmap.resize(til::size{ 2, 4 }, true); + + // 1 1 + // 1 1 + // 0 1 + // 1 1 + _checkBits(expectedFillRects, bitmap); } TEST_METHOD(One) @@ -236,12 +324,6 @@ class BitmapTests Log::Comment(L"And setting all will no longer be none again."); bitmap.set_all(); VERIFY_IS_FALSE(bitmap.none()); - - // FUTURE: We could make resize smart and - // crop off the shrink or only invalidate the new bits added. - Log::Comment(L"And resizing should make it empty*"); - bitmap.resize(til::size{ 3, 3 }); - VERIFY_IS_TRUE(bitmap.none()); } TEST_METHOD(All) From 824657f89a70dfe079ec301f3ef45c24da9fff73 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 18 Mar 2020 16:06:31 -0700 Subject: [PATCH 12/25] Static analysis pass. --- src/inc/til/bitmap.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 88ab038e3d9..322956ed849 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -239,17 +239,17 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _dirty.size() == til::size{ 1, 1 }; } - bool any() const + constexpr bool any() const noexcept { return !none(); } - bool none() const + constexpr bool none() const noexcept { return _dirty.empty(); } - bool all() const + constexpr bool all() const noexcept { return _dirty == _rc; } From 83e28f95fca8f0379d245bb05936a264213a7b94 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 10:34:21 -0700 Subject: [PATCH 13/25] Add run caching to bitmap, make iterator into real input iterator so it fits nicely into vector emplace, start using bitmap in ConPTY (Vt renderer). --- src/inc/til/bitmap.h | 48 ++++++++++++++++++++++++-- src/renderer/vt/paint.cpp | 8 ++--- src/renderer/vt/tracing.hpp | 3 +- src/renderer/vt/vtrenderer.hpp | 4 +-- src/til/ut_til/BitmapTests.cpp | 63 +++++++++++++++++++++++++++++++--- 5 files changed, 110 insertions(+), 16 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 322956ed849..506e7d2a1d6 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -14,6 +14,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" class _bitmap_const_iterator { public: + using iterator_category = typename std::input_iterator_tag; + using value_type = typename const til::rectangle; + using difference_type = typename ptrdiff_t; + using pointer = typename const til::rectangle*; + using reference = typename const til::rectangle&; + _bitmap_const_iterator(const std::vector& values, til::rectangle rc, ptrdiff_t pos) : _values(values), _rc(rc), @@ -30,6 +36,13 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return (*this); } + _bitmap_const_iterator operator++(int) + { + const auto prev = *this; + ++*this; + return prev; + } + constexpr bool operator==(const _bitmap_const_iterator& other) const noexcept { return _pos == other._pos && _values == other._values; @@ -50,11 +63,16 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _pos > other._pos; } - constexpr til::rectangle operator*() const noexcept + constexpr reference operator*() const noexcept { return _run; } + constexpr pointer operator->() const noexcept + { + return &_run; + } + private: const std::vector& _values; const til::rectangle _rc; @@ -117,7 +135,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz{}, _rc{}, _bits{}, - _dirty{} + _dirty{}, + _runs{} { } @@ -130,7 +149,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(sz.area(), fill), - _dirty(fill ? sz : til::rectangle{}) + _dirty(fill ? sz : til::rectangle{}), + _runs{} { } @@ -144,9 +164,22 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return const_iterator(_bits, _sz, _sz.area()); } + std::vector& runs() + { + // If we don't have cached runs, rebuild. + if (!_runs.has_value()) + { + _runs.emplace(begin(), end()); + } + + // Return a reference to the runs. + return _runs.value(); + } + void set(const til::point pt) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); + _runs.reset(); // reset cached runs on any non-const method til::at(_bits, _rc.index_of(pt)) = true; _dirty |= til::rectangle{ pt }; @@ -155,6 +188,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" void set(const til::rectangle rc) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc)); + _runs.reset(); // reset cached runs on any non-const method for (const auto pt : rc) { @@ -166,6 +200,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" void set_all() { + _runs.reset(); // reset cached runs on any non-const method + // .clear() then .resize(_size(), true) throws an assert (unsupported operation) // .assign(_size(), true) throws an assert (unsupported operation) _bits = std::vector(_sz.area(), true); @@ -174,6 +210,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" void reset_all() { + _runs.reset(); // reset cached runs on any non-const method + // .clear() then .resize(_size(), false) throws an assert (unsupported operation) // .assign(_size(), false) throws an assert (unsupported operation) @@ -185,6 +223,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // Set fill if you want the new region (on growing) to be marked dirty. bool resize(til::size size, bool fill = false) { + _runs.reset(); // reset cached runs on any non-const method + // FYI .resize(_size(), true/false) throws an assert (unsupported operation) // Don't resize if it's not different @@ -260,6 +300,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" til::rectangle _rc; std::vector _bits; + std::optional> _runs; + #ifdef UNIT_TESTING friend class ::BitmapTests; #endif diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index dc292f74f3d..6f3078ced42 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -26,13 +26,13 @@ using namespace Microsoft::Console::Types; } // If there's nothing to do, quick return - bool somethingToDo = _fInvalidRectUsed || + bool somethingToDo = _invalidMap.any() || (_scrollDelta.X != 0 || _scrollDelta.Y != 0) || _cursorMoved || _titleChanged; _quickReturn = !somethingToDo; - _trace.TraceStartPaint(_quickReturn, _fInvalidRectUsed, _invalidRect, _lastViewport, _scrollDelta, _cursorMoved); + _trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport, _scrollDelta, _cursorMoved); return _quickReturn ? S_FALSE : S_OK; } @@ -50,8 +50,8 @@ using namespace Microsoft::Console::Types; { _trace.TraceEndPaint(); - _invalidRect = Viewport::Empty(); - _fInvalidRectUsed = false; + _invalidMap.reset_all(); + _scrollDelta = { 0 }; _clearedAllThisFrame = false; _cursorMoved = false; diff --git a/src/renderer/vt/tracing.hpp b/src/renderer/vt/tracing.hpp index 0fcd1a97aac..7e71f638cb5 100644 --- a/src/renderer/vt/tracing.hpp +++ b/src/renderer/vt/tracing.hpp @@ -35,8 +35,7 @@ namespace Microsoft::Console::VirtualTerminal void TraceInvalidateAll(const Microsoft::Console::Types::Viewport view) const; void TraceTriggerCircling(const bool newFrame) const; void TraceStartPaint(const bool quickReturn, - const bool invalidRectUsed, - const Microsoft::Console::Types::Viewport invalidRect, + const til::bitmap invalidMap, const Microsoft::Console::Types::Viewport lastViewport, const COORD scrollDelta, const bool cursorMoved) const; diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 6db9c5efb9f..d08e804ac57 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -121,9 +121,9 @@ namespace Microsoft::Console::Render bool _lastWasBold; Microsoft::Console::Types::Viewport _lastViewport; - Microsoft::Console::Types::Viewport _invalidRect; + + til::bitmap _invalidMap; - bool _fInvalidRectUsed; COORD _lastRealCursor; COORD _lastText; COORD _scrollDelta; diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index a034c1145d1..dd70668f029 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -361,7 +361,7 @@ class BitmapTests VERIFY_IS_FALSE(bitmap.all()); } - TEST_METHOD(Iterate) + TEST_METHOD(Runs) { // This map --> Those runs // 1 1 0 1 A A _ B @@ -370,8 +370,6 @@ class BitmapTests // 0 1 1 0 _ F F _ Log::Comment(L"Set up a bitmap with some runs."); - // Note: we'll test setting/resetting some overlapping rects and points. - til::bitmap map{ til::size{ 4, 4 } }; // 0 0 0 0 |1 1|0 0 @@ -421,7 +419,7 @@ class BitmapTests Log::Comment(L"Run the iterator and collect the runs."); til::some actual; - for (auto run : map) + for (auto run : map.runs()) { actual.push_back(run); } @@ -434,12 +432,67 @@ class BitmapTests expected.clear(); actual.clear(); - for (auto run : map) + for (auto run : map.runs()) { actual.push_back(run); } Log::Comment(L"Verify they're empty."); VERIFY_ARE_EQUAL(expected, actual); + + Log::Comment(L"Set point and validate runs updated."); + const til::point setPoint{ 2, 2 }; + expected.push_back(til::rectangle{ setPoint }); + map.set(setPoint); + + for (auto run : map.runs()) + { + actual.push_back(run); + } + VERIFY_ARE_EQUAL(expected, actual); + + Log::Comment(L"Set rectangle and validate runs updated."); + const til::rectangle setRect{ setPoint, til::size{2, 2} }; + expected.clear(); + expected.push_back(til::rectangle{ til::point{2, 2}, til::size{2, 1} }); + expected.push_back(til::rectangle{ til::point{2, 3}, til::size{2, 1} }); + map.set(setRect); + + actual.clear(); + for (auto run : map.runs()) + { + actual.push_back(run); + } + VERIFY_ARE_EQUAL(expected, actual); + + Log::Comment(L"Set all and validate runs updated."); + expected.clear(); + expected.push_back(til::rectangle{ til::point{0, 0}, til::size{4, 1} }); + expected.push_back(til::rectangle{ til::point{0, 1}, til::size{4, 1} }); + expected.push_back(til::rectangle{ til::point{0, 2}, til::size{4, 1} }); + expected.push_back(til::rectangle{ til::point{0, 3}, til::size{4, 1} }); + map.set_all(); + + actual.clear(); + for (auto run : map.runs()) + { + actual.push_back(run); + } + VERIFY_ARE_EQUAL(expected, actual); + + Log::Comment(L"Resize and validate runs updated."); + const til::size newSize{ 3, 3 }; + expected.clear(); + expected.push_back(til::rectangle{ til::point{0, 0}, til::size{3, 1} }); + expected.push_back(til::rectangle{ til::point{0, 1}, til::size{3, 1} }); + expected.push_back(til::rectangle{ til::point{0, 2}, til::size{3, 1} }); + map.resize(newSize); + + actual.clear(); + for (auto run : map.runs()) + { + actual.push_back(run); + } + VERIFY_ARE_EQUAL(expected, actual); } }; From d705527239f0d92d77fde959e9be30e3e6a97885 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 11:34:33 -0700 Subject: [PATCH 14/25] Set up string printing for til types, use that inside the TAEF string printing. Update VT tracing to use the same strings. Roll out bitmap through most of VT. --- src/inc/til/bitmap.h | 84 +++++++++++++++++++++++++++++- src/inc/til/point.h | 7 ++- src/inc/til/rectangle.h | 7 ++- src/inc/til/size.h | 7 ++- src/inc/til/some.h | 24 +++++---- src/renderer/vt/invalidate.cpp | 61 ++++------------------ src/renderer/vt/math.cpp | 33 +++++++----- src/renderer/vt/paint.cpp | 2 +- src/renderer/vt/state.cpp | 32 ++---------- src/renderer/vt/tracing.cpp | 95 +++++++++++----------------------- src/renderer/vt/tracing.hpp | 14 ++--- src/renderer/vt/vtrenderer.hpp | 3 -- 12 files changed, 187 insertions(+), 182 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 506e7d2a1d6..a67a755e9d0 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -154,6 +154,20 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { } + constexpr bool operator==(const bitmap& other) const noexcept + { + return _sz == other._sz && + _rc == other._rc && + _bits == other._bits && + _dirty == other._dirty; + // _runs excluded because it's a cache of generated state. + } + + constexpr bool operator!=(const bitmap& other) const noexcept + { + return !(*this == other); + } + const_iterator begin() const { return const_iterator(_bits, _sz, 0); @@ -164,12 +178,24 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return const_iterator(_bits, _sz, _sz.area()); } - std::vector& runs() + const std::vector& runs() const { // If we don't have cached runs, rebuild. if (!_runs.has_value()) { - _runs.emplace(begin(), end()); + // If there's only one square dirty, quick save it off and be done. + if (one()) + { + // We're const for the sake of actually manipulating the dirty state. + // But we remove const for updating the cache. + const_cast(this)->_runs.emplace(_dirty); + } + else + { + // We're const for the sake of actually manipulating the dirty state. + // But we remove const for updating the cache. + const_cast(this)->_runs.emplace(begin(), end()); + } } // Return a reference to the runs. @@ -294,6 +320,20 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _dirty == _rc; } + std::wstring to_string() const + { + std::wstringstream wss; + wss << std::endl << L"Bitmap of size " << _sz.to_string() << " contains the following dirty regions:" << std::endl; + wss << L"Runs:" << std::endl; + + for (auto& item : *this) + { + wss << L"\t- " << item.to_string() << std::endl; + } + + return wss.str(); + } + private: til::rectangle _dirty; til::size _sz; @@ -307,3 +347,43 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" #endif }; } + +#ifdef __WEX_COMMON_H__ +namespace WEX::TestExecution +{ + template<> + class VerifyOutputTraits<::til::bitmap> + { + public: + static WEX::Common::NoThrowString ToString(const ::til::bitmap& rect) + { + return WEX::Common::NoThrowString(rect.to_string().c_str()); + } + }; + + template<> + class VerifyCompareTraits<::til::bitmap, ::til::bitmap> + { + public: + static bool AreEqual(const ::til::bitmap& expected, const ::til::bitmap& actual) noexcept + { + return expected == actual; + } + + static bool AreSame(const ::til::bitmap& expected, const ::til::bitmap& actual) noexcept + { + return &expected == &actual; + } + + static bool IsLessThan(const ::til::bitmap& expectedLess, const ::til::bitmap& expectedGreater) = delete; + + static bool IsGreaterThan(const ::til::bitmap& expectedGreater, const ::til::bitmap& expectedLess) = delete; + + static bool IsNull(const ::til::bitmap& object) noexcept + { + return object == til::bitmap{}; + } + }; + +}; +#endif diff --git a/src/inc/til/point.h b/src/inc/til/point.h index 49b78a22d65..807537eaeac 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -198,6 +198,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } #endif + std::wstring to_string() const + { + return wil::str_printf(L"(X:%td, Y:%td)", x(), y()); + } + protected: ptrdiff_t _x; ptrdiff_t _y; @@ -217,7 +222,7 @@ namespace WEX::TestExecution public: static WEX::Common::NoThrowString ToString(const ::til::point& point) { - return WEX::Common::NoThrowString().Format(L"(X:%td, Y:%td)", point.x(), point.y()); + return WEX::Common::NoThrowString(point.to_string().c_str()); } }; diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index ef0676e96f2..476ed2273ce 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -587,6 +587,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } #endif + std::wstring to_string() const + { + return wil::str_printf(L"(L:%td, T:%td, R:%td, B:%td) [W:%td, H:%td]", left(), top(), right(), bottom(), width(), height()); + } + protected: til::point _topLeft; til::point _bottomRight; @@ -606,7 +611,7 @@ namespace WEX::TestExecution public: static WEX::Common::NoThrowString ToString(const ::til::rectangle& rect) { - return WEX::Common::NoThrowString().Format(L"(L:%td, T:%td, R:%td, B:%td) [W:%td, H:%td]", rect.left(), rect.top(), rect.right(), rect.bottom(), rect.width(), rect.height()); + return WEX::Common::NoThrowString(rect.to_string().c_str()); } }; diff --git a/src/inc/til/size.h b/src/inc/til/size.h index b3327896d1e..b44bbbf670a 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -215,6 +215,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } #endif + std::wstring to_string() const + { + return wil::str_printf(L"[W:%td, H:%td]", width(), height()); + } + protected: ptrdiff_t _width; ptrdiff_t _height; @@ -234,7 +239,7 @@ namespace WEX::TestExecution public: static WEX::Common::NoThrowString ToString(const ::til::size& size) { - return WEX::Common::NoThrowString().Format(L"[W:%td, H:%td]", size.width(), size.height()); + return WEX::Common::NoThrowString(size.to_string().c_str()); } }; diff --git a/src/inc/til/some.h b/src/inc/til/some.h index 9620bdce777..5e5c209b27f 100644 --- a/src/inc/til/some.h +++ b/src/inc/til/some.h @@ -208,6 +208,20 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { throw std::out_of_range("invalid some subscript"); } + + std::wstring to_string() const + { + std::wstringstream wss; + wss << std::endl << L"Some contains " << size() << " of max size " << max_size() << ":" << std::endl; + wss << L"Elements:" << std::endl; + + for (auto& item : *this) + { + wss << L"\t- " << item.to_string() << std::endl; + } + + return wss.str(); + } }; } @@ -220,15 +234,7 @@ namespace WEX::TestExecution public: static WEX::Common::NoThrowString ToString(const ::til::some& some) { - auto str = WEX::Common::NoThrowString().Format(L"\r\nSome contains %d of max size %d:\r\nElements:\r\n", some.size(), some.max_size()); - - for (auto& item : some) - { - const auto itemStr = WEX::TestExecution::VerifyOutputTraits::ToString(item); - str.AppendFormat(L"\t- %ws\r\n", (const wchar_t*)itemStr); - } - - return str; + return WEX::Common::NoThrowString(some.to_string().c_str()); } }; diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index c3042ad6e1b..9dd1a287957 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -47,12 +47,14 @@ using namespace Microsoft::Console::Render; // Return Value: // - S_OK, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::Invalidate(const SMALL_RECT* const psrRegion) noexcept +try { - Viewport newInvalid = Viewport::FromExclusive(*psrRegion); - _trace.TraceInvalidate(newInvalid); - - return this->_InvalidCombine(newInvalid); + const til::rectangle rect{ *psrRegion }; + _trace.TraceInvalidate(rect); + _invalidMap.set(rect); + return S_OK; } +CATCH_RETURN(); // Routine Description: // - Notifies us that the console has changed the position of the cursor. @@ -87,10 +89,13 @@ using namespace Microsoft::Console::Render; // Return Value: // - S_OK, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT VtEngine::InvalidateAll() noexcept +try { - _trace.TraceInvalidateAll(_lastViewport.ToOrigin()); - return this->_InvalidCombine(_lastViewport.ToOrigin()); + _trace.TraceInvalidateAll(_lastViewport.ToOrigin().ToInclusive()); + _invalidMap.set_all(); + return S_OK; } +CATCH_RETURN(); // Method Description: // - Notifies us that we're about to circle the buffer, giving us a chance to @@ -133,33 +138,6 @@ using namespace Microsoft::Console::Render; return S_OK; } -// Routine Description: -// - Helper to combine the given rectangle into the invalid region to be -// updated on the next paint -// Expects EXCLUSIVE rectangles. -// Arguments: -// - invalid - A viewport containing the character region that should be -// repainted on the next frame -// Return Value: -// - S_OK, else an appropriate HRESULT for failing to allocate or write. -[[nodiscard]] HRESULT VtEngine::_InvalidCombine(const Viewport invalid) noexcept -{ - if (!_fInvalidRectUsed) - { - _invalidRect = invalid; - _fInvalidRectUsed = true; - } - else - { - _invalidRect = Viewport::Union(_invalidRect, invalid); - } - - // Ensure invalid areas remain within bounds of window. - RETURN_IF_FAILED(_InvalidRestrict()); - - return S_OK; -} - // Routine Description: // - Helper to adjust the invalid region by the given offset such as when a // scroll operation occurs. @@ -187,20 +165,3 @@ using namespace Microsoft::Console::Render; return S_OK; } - -// Routine Description: -// - Helper to ensure the invalid region remains within the bounds of the viewport. -// Arguments: -// - -// Return Value: -// - S_OK, else an appropriate HRESULT for failing to allocate or safemath failure. -[[nodiscard]] HRESULT VtEngine::_InvalidRestrict() noexcept -{ - SMALL_RECT oldInvalid = _invalidRect.ToExclusive(); - - _lastViewport.ToOrigin().TrimToViewport(&oldInvalid); - - _invalidRect = Viewport::FromExclusive(oldInvalid); - - return S_OK; -} diff --git a/src/renderer/vt/math.cpp b/src/renderer/vt/math.cpp index bd1f81e8f65..4198dcdad0c 100644 --- a/src/renderer/vt/math.cpp +++ b/src/renderer/vt/math.cpp @@ -19,12 +19,7 @@ using namespace Microsoft::Console::Types; // This is an Inclusive rect. std::vector VtEngine::GetDirtyArea() { - SMALL_RECT dirty = _invalidRect.ToInclusive(); - if (dirty.Top < _virtualTop) - { - dirty.Top = _virtualTop; - } - return { dirty }; + return _invalidMap.runs(); } // Routine Description: @@ -68,17 +63,27 @@ void VtEngine::_OrRect(_Inout_ SMALL_RECT* const pRectExisting, const SMALL_RECT // - true iff only the next character is invalid bool VtEngine::_WillWriteSingleChar() const { - COORD currentCursor = _lastText; - SMALL_RECT _srcInvalid = _invalidRect.ToExclusive(); - bool noScrollDelta = (_scrollDelta.X == 0 && _scrollDelta.Y == 0); + // If there is scroll delta, return false. + if (til::point{ 0, 0 } != til::point{ _scrollDelta }) + { + return false; + } + + // If there is more than one invalid char, return false. + if (!_invalidMap.one()) + { + return false; + } + + // Get the single point at which things are invalid. + const auto invalidPoint = _invalidMap.runs().front().origin(); - bool invalidIsOneChar = (_invalidRect.Width() == 1) && - (_invalidRect.Height() == 1); + // Either the next character to the right or the immediately previous // character should follow this code path // (The immediate previous character would suggest a backspace) - bool invalidIsNext = (_srcInvalid.Top == _lastText.Y) && (_srcInvalid.Left == _lastText.X); - bool invalidIsLast = (_srcInvalid.Top == _lastText.Y) && (_srcInvalid.Left == (_lastText.X - 1)); + bool invalidIsNext = invalidPoint == til::point{ _lastText }; + bool invalidIsLast = invalidPoint == til::point{ _lastText.X - 1, _lastText.Y }; - return noScrollDelta && invalidIsOneChar && (invalidIsNext || invalidIsLast); + return invalidIsNext || invalidIsLast; } diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index 6f3078ced42..babc6606caa 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -32,7 +32,7 @@ using namespace Microsoft::Console::Types; _titleChanged; _quickReturn = !somethingToDo; - _trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport, _scrollDelta, _cursorMoved); + _trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport.ToInclusive(), _scrollDelta, _cursorMoved); return _quickReturn ? S_FALSE : S_OK; } diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index c83ac780ddd..a50179cd506 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -35,8 +35,7 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe, _LastBG(INVALID_COLOR), _lastWasBold(false), _lastViewport(initialViewport), - _invalidRect(Viewport::Empty()), - _fInvalidRectUsed(false), + _invalidMap(initialViewport.Dimensions()), _lastRealCursor({ 0 }), _lastText({ 0 }), _scrollDelta({ 0 }), @@ -317,40 +316,19 @@ CATCH_RETURN(); // buffer will have triggered it's own invalidations for what it knows is // invalid. Previously, we'd invalidate everything if the width changed, // because we couldn't be sure if lines were reflowed. + _invalidMap.resize(newView.Dimensions()); } else { if (SUCCEEDED(hr)) { + _invalidMap.resize(newView.Dimensions(), true); // resize while filling in new space with repaint requests. + // Viewport is smaller now - just update it all. if (oldView.Height() > newView.Height() || oldView.Width() > newView.Width()) { hr = InvalidateAll(); } - else - { - // At least one of the directions grew. - // First try and add everything to the right of the old viewport, - // then everything below where the old viewport ended. - if (oldView.Width() < newView.Width()) - { - short left = oldView.RightExclusive(); - short top = 0; - short right = newView.RightInclusive(); - short bottom = oldView.BottomInclusive(); - Viewport rightOfOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); - hr = _InvalidCombine(rightOfOldViewport); - } - if (SUCCEEDED(hr) && oldView.Height() < newView.Height()) - { - short left = 0; - short top = oldView.BottomExclusive(); - short right = newView.RightInclusive(); - short bottom = newView.BottomInclusive(); - Viewport belowOldViewport = Viewport::FromInclusive({ left, top, right, bottom }); - hr = _InvalidCombine(belowOldViewport); - } - } } } @@ -416,7 +394,7 @@ void VtEngine::SetTestCallback(_In_ std::function Date: Thu, 19 Mar 2020 14:42:16 -0700 Subject: [PATCH 15/25] Operators on rectangle with points and sizes, operators between points and sizes, translation operator on the bitmap. A ton of tests for all that behavior. Replacing the rest of the VT renderer stuff with the invalidation map. --- src/inc/til.h | 1 + src/inc/til/bitmap.h | 114 +++- src/inc/til/operators.h | 61 ++ src/inc/til/rectangle.h | 263 +++++++++ src/renderer/vt/XtermEngine.cpp | 8 +- src/renderer/vt/invalidate.cpp | 28 - src/til/ut_til/BitmapTests.cpp | 536 +++++++++++++++++- src/til/ut_til/OperatorTests.cpp | 87 +++ src/til/ut_til/RectangleTests.cpp | 232 ++++++++ src/til/ut_til/til.unit.tests.vcxproj | 1 + src/til/ut_til/til.unit.tests.vcxproj.filters | 1 + 11 files changed, 1263 insertions(+), 69 deletions(-) create mode 100644 src/inc/til/operators.h create mode 100644 src/til/ut_til/OperatorTests.cpp diff --git a/src/inc/til.h b/src/inc/til.h index 6cbe6a0b128..289a8c52efb 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -9,6 +9,7 @@ #include "til/size.h" #include "til/point.h" #include "til/rectangle.h" +#include "til/operators.h" #include "til/bitmap.h" #include "til/u8u16convert.h" diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index a67a755e9d0..fa58ae9e7a3 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -149,17 +149,21 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(sz.area(), fill), - _dirty(fill ? sz : til::rectangle{}), + _dirty(), _runs{} { + if (fill) + { + _dirty = til::rectangle{ sz }; + } } constexpr bool operator==(const bitmap& other) const noexcept { return _sz == other._sz && - _rc == other._rc && - _bits == other._bits && - _dirty == other._dirty; + _rc == other._rc && + _dirty == other._dirty && // dirty is before bits because it's a rough estimate of bits and a faster comparison. + _bits == other._bits; // _runs excluded because it's a cache of generated state. } @@ -188,13 +192,13 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { // We're const for the sake of actually manipulating the dirty state. // But we remove const for updating the cache. - const_cast(this)->_runs.emplace(_dirty); + _runs.emplace({ _dirty.value() }); } else { // We're const for the sake of actually manipulating the dirty state. // But we remove const for updating the cache. - const_cast(this)->_runs.emplace(begin(), end()); + _runs.emplace(begin(), end()); } } @@ -202,13 +206,87 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return _runs.value(); } + // optional fill the uncovered area with bits. + void translate(const til::point delta, bool fill = false) + { + // FUTURE: PERF: GH #4015: This could use in-place walk semantics instead of a temporary. + til::bitmap other{ _sz }; + + for (auto run : *this) + { + // Offset by the delta + run += delta; + + // Intersect with the bounds of our bitmap area + // as part of it could have slid out of bounds. + run &= _rc; + + // Set it into the new bitmap. + other.set(run); + } + + // If we were asked to fill... find the uncovered region. + if (fill) + { + // Original Rect of As. + // + // X <-- origin + // A A A A + // A A A A + // A A A A + // A A A A + const auto originalRect = _rc; + + // If Delta = (2, 2) + // Translated Rect of Bs. + // + // X <-- origin + // + // + // B B B B + // B B B B + // B B B B + // B B B B + const auto translatedRect = _rc + delta; + + // Subtract the B from the A one to see what wasn't filled by the move. + // C is the overlap of A and B: + // + // X <-- origin + // A A A A 1 1 1 1 + // A A A A 1 1 1 1 + // A A C C B B subtract 2 2 + // A A C C B B ---------> 2 2 + // B B B B A - B + // B B B B + // + // 1 and 2 are the spaces to fill that are "uncovered". + const auto fillRects = originalRect - translatedRect; + for (const auto& f : fillRects) + { + other.set(f); + } + } + + // Swap us with the temporary one. + std::swap(other, *this); + } + void set(const til::point pt) { THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt)); _runs.reset(); // reset cached runs on any non-const method til::at(_bits, _rc.index_of(pt)) = true; - _dirty |= til::rectangle{ pt }; + + if (_dirty.has_value()) + { + _dirty.value() |= til::rectangle{ pt }; + } + else + { + _dirty = til::rectangle{ pt }; + } } void set(const til::rectangle rc) @@ -221,7 +299,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" til::at(_bits, _rc.index_of(pt)) = true; } - _dirty |= rc; + if (_dirty.has_value()) + { + _dirty.value() |= rc; + } + else + { + _dirty = rc; + } } void set_all() @@ -302,7 +387,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" bool one() const { - return _dirty.size() == til::size{ 1, 1 }; + return _dirty.has_value() && _dirty.value().size() == til::size{ 1, 1 }; } constexpr bool any() const noexcept @@ -312,18 +397,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool none() const noexcept { - return _dirty.empty(); + return !_dirty.has_value(); } constexpr bool all() const noexcept { - return _dirty == _rc; + return _dirty.has_value() && _dirty.value() == _rc; } std::wstring to_string() const { std::wstringstream wss; - wss << std::endl << L"Bitmap of size " << _sz.to_string() << " contains the following dirty regions:" << std::endl; + wss << std::endl + << L"Bitmap of size " << _sz.to_string() << " contains the following dirty regions:" << std::endl; wss << L"Runs:" << std::endl; for (auto& item : *this) @@ -335,12 +421,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: - til::rectangle _dirty; + std::optional _dirty; til::size _sz; til::rectangle _rc; std::vector _bits; - std::optional> _runs; + mutable std::optional> _runs; #ifdef UNIT_TESTING friend class ::BitmapTests; diff --git a/src/inc/til/operators.h b/src/inc/til/operators.h new file mode 100644 index 00000000000..a9a19d4bf80 --- /dev/null +++ b/src/inc/til/operators.h @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "rectangle.h" +#include "size.h" +#include "bitmap.h" + +#define _TIL_INLINEPREFIX __declspec(noinline) inline + +namespace til // Terminal Implementation Library. Also: "Today I Learned" +{ + // Operators go here when they involve two headers that can't/don't include each other. + +#pragma region POINT VS SIZE + // This is a convenience and will take X vs WIDTH and Y vs HEIGHT. + _TIL_INLINEPREFIX point operator+(const point& lhs, const size& rhs) + { + return lhs + til::point{ rhs.width(), rhs.height() }; + } + + _TIL_INLINEPREFIX point operator-(const point& lhs, const size& rhs) + { + return lhs - til::point{ rhs.width(), rhs.height() }; + } + + _TIL_INLINEPREFIX point operator*(const point& lhs, const size& rhs) + { + return lhs * til::point{ rhs.width(), rhs.height() }; + } + + _TIL_INLINEPREFIX point operator/(const point& lhs, const size& rhs) + { + return lhs / til::point{ rhs.width(), rhs.height() }; + } +#pragma endregion + +#pragma region SIZE VS POINT + // This is a convenience and will take WIDTH vs X and HEIGHT vs Y. + _TIL_INLINEPREFIX size operator+(const size& lhs, const point& rhs) + { + return lhs + til::size(rhs.x(), rhs.y()); + } + + _TIL_INLINEPREFIX size operator-(const size& lhs, const point& rhs) + { + return lhs - til::size(rhs.x(), rhs.y()); + } + + _TIL_INLINEPREFIX size operator*(const size& lhs, const point& rhs) + { + return lhs * til::size(rhs.x(), rhs.y()); + } + + _TIL_INLINEPREFIX size operator/(const size& lhs, const point& rhs) + { + return lhs / til::size(rhs.x(), rhs.y()); + } +#pragma endregion +} diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 476ed2273ce..b1da32791c8 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -212,6 +212,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return const_iterator(_topLeft, _bottomRight, { _topLeft.x(), _bottomRight.y() }); } +#pragma region RECTANGLE OPERATORS // OR = union constexpr rectangle operator|(const rectangle& other) const noexcept { @@ -274,6 +275,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return rectangle{ l, t, r, b }; } + constexpr rectangle& operator&=(const rectangle& other) noexcept + { + *this = *this & other; + return *this; + } + // - = subtract some operator-(const rectangle& other) const { @@ -405,6 +412,262 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return result; } +#pragma endregion + +#pragma region RECTANGLE VS POINT + // ADD will translate (offset) the rectangle by the point. + rectangle operator+(const point& point) const + { + // Fetch the pieces of the rectangle. + auto l = left(); + auto r = right(); + auto t = top(); + auto b = bottom(); + + l += point.x(); + r += point.x(); + t += point.y(); + b += point.y(); + + return til::rectangle{ til::point{ l, t }, til::point{ r, b } }; + } + + rectangle& operator+=(const point& point) + { + *this = *this + point; + return *this; + } + + // SUB will translate (offset) the rectangle by the point. + rectangle operator-(const point& point) const + { + // Fetch the pieces of the rectangle. + auto l = left(); + auto r = right(); + auto t = top(); + auto b = bottom(); + + l -= point.x(); + r -= point.x(); + t -= point.y(); + b -= point.y(); + + return til::rectangle{ til::point{ l, t }, til::point{ r, b } }; + } + + rectangle& operator-=(const point& point) + { + *this = *this - point; + return *this; + } + +#pragma endregion + +#pragma region RECTANGLE VS SIZE + // ADD will grow the total area of the rectangle. The sign is the direction to grow. + rectangle operator+(const size& size) const + { + // Fetch the pieces of the rectangle. + auto l = left(); + auto r = right(); + auto t = top(); + auto b = bottom(); + + // Fetch the scale factors we're using. + const auto width = size.width(); + const auto height = size.height(); + + // Since this is the add operation versus a size, the result + // should grow the total rectangle area. + // The sign determines which edge of the rectangle moves. + // We use the magnitude as how far to move. + if (width > 0) + { + // Adding the positive makes the rectangle "grow" + // because right stretches outward (to the right). + // + // Example with adding width 3... + // |-- x = origin + // V + // x---------| x------------| + // | | | | + // | | | | + // |---------| |------------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckAdd(r, width).AssignIfValid(&r)); + } + else + { + // Adding the negative makes the rectangle "grow" + // because left stretches outward (to the left). + // + // Example with adding width -3... + // |-- x = origin + // V + // x---------| |--x---------| + // | | | | + // | | | | + // |---------| |------------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckAdd(l, width).AssignIfValid(&l)); + } + + if (height > 0) + { + // Adding the positive makes the rectangle "grow" + // because bottom stretches outward (to the down). + // + // Example with adding height 2... + // |-- x = origin + // V + // x---------| x---------| + // | | | | + // | | | | + // |---------| | | + // | | + // |---------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckAdd(b, height).AssignIfValid(&b)); + } + else + { + // Adding the negative makes the rectangle "grow" + // because top stretches outward (to the up). + // + // Example with adding height -2... + // |-- x = origin + // | + // | |---------| + // V | | + // x---------| x | + // | | | | + // | | | | + // |---------| |---------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckAdd(t, height).AssignIfValid(&t)); + } + + return rectangle{ til::point{ l, t }, til::point{ r, b } }; + } + + rectangle& operator+=(const size& size) + { + *this = *this + size; + return *this; + } + + // SUB will shrink the total area of the rectangle. The sign is the direction to shrink. + rectangle operator-(const size& size) const + { + // Fetch the pieces of the rectangle. + auto l = left(); + auto r = right(); + auto t = top(); + auto b = bottom(); + + // Fetch the scale factors we're using. + const auto width = size.width(); + const auto height = size.height(); + + // Since this is the subtract operation versus a size, the result + // should shrink the total rectangle area. + // The sign determines which edge of the rectangle moves. + // We use the magnitude as how far to move. + if (width > 0) + { + // Subtracting the positive makes the rectangle "shrink" + // because right pulls inward (to the left). + // + // Example with subtracting width 3... + // |-- x = origin + // V + // x---------| x------| + // | | | | + // | | | | + // |---------| |------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckSub(r, width).AssignIfValid(&r)); + } + else + { + // Subtracting the negative makes the rectangle "shrink" + // because left pulls inward (to the right). + // + // Example with subtracting width -3... + // |-- x = origin + // V + // x---------| x |------| + // | | | | + // | | | | + // |---------| |------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckSub(l, width).AssignIfValid(&l)); + } + + if (height > 0) + { + // Subtracting the positive makes the rectangle "shrink" + // because bottom pulls inward (to the up). + // + // Example with subtracting height 2... + // |-- x = origin + // V + // x---------| x---------| + // | | |---------| + // | | + // |---------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckSub(b, height).AssignIfValid(&b)); + } + else + { + // Subtracting the positive makes the rectangle "shrink" + // because top pulls inward (to the down). + // + // Example with subtracting height -2... + // |-- x = origin + // V + // x---------| x + // | | + // | | |---------| + // |---------| |---------| + // BEFORE AFTER + THROW_HR_IF(E_ABORT, !base::CheckSub(t, height).AssignIfValid(&t)); + } + + return rectangle{ til::point{ l, t }, til::point{ r, b } }; + } + + rectangle& operator-=(const size& size) + { + *this = *this - size; + return *this; + } + + // MUL will scale the entire rectangle by the size L/R * WIDTH and T/B * HEIGHT. + rectangle operator*(const size& size) const + { + ptrdiff_t l; + THROW_HR_IF(E_ABORT, !base::CheckMul(left(), size.width()).AssignIfValid(&l)); + + ptrdiff_t t; + THROW_HR_IF(E_ABORT, !base::CheckMul(top(), size.height()).AssignIfValid(&t)); + + ptrdiff_t r; + THROW_HR_IF(E_ABORT, !base::CheckMul(right(), size.width()).AssignIfValid(&r)); + + ptrdiff_t b; + THROW_HR_IF(E_ABORT, !base::CheckMul(bottom(), size.height()).AssignIfValid(&b)); + + return til::rectangle{ l, t, r, b }; + } + + rectangle& operator*=(const size& size) + { + *this = *this * size; + return *this; + } +#pragma endregion constexpr ptrdiff_t top() const noexcept { diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 145d972fdba..f5e1eec88bc 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -408,9 +408,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, if (dx != 0 || dy != 0) { - // Scroll the current offset - RETURN_IF_FAILED(_InvalidOffset(pcoordDelta)); - + // Scroll the current offset and invalidate the revealed area + _invalidMap.translate(til::point(*pcoordDelta), true); + // Add the top/bottom of the window to the invalid area SMALL_RECT invalid = _lastViewport.ToOrigin().ToExclusive(); @@ -422,7 +422,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { invalid.Top = invalid.Bottom + dy; } - LOG_IF_FAILED(_InvalidCombine(Viewport::FromExclusive(invalid))); + _invalidMap.set(til::rectangle(Viewport::FromExclusive(invalid).ToInclusive())); COORD invalidScrollNew; RETURN_IF_FAILED(ShortAdd(_scrollDelta.X, dx, &invalidScrollNew.X)); diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 9dd1a287957..784006496bd 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -137,31 +137,3 @@ CATCH_RETURN(); *pForcePaint = true; return S_OK; } - -// Routine Description: -// - Helper to adjust the invalid region by the given offset such as when a -// scroll operation occurs. -// Arguments: -// - ppt - Distances by which we should move the invalid region in response to a scroll -// Return Value: -// - S_OK, else an appropriate HRESULT for failing to allocate or write. -[[nodiscard]] HRESULT VtEngine::_InvalidOffset(const COORD* const pCoord) noexcept -{ - if (_fInvalidRectUsed) - { - try - { - Viewport newInvalid = Viewport::Offset(_invalidRect, *pCoord); - - // Add the scrolled invalid rectangle to what was left behind to get the new invalid area. - // This is the equivalent of adding in the "update rectangle" that we would get out of ScrollWindowEx/ScrollDC. - _invalidRect = Viewport::Union(_invalidRect, newInvalid); - - // Ensure invalid areas remain within bounds of window. - RETURN_IF_FAILED(_InvalidRestrict()); - } - CATCH_RETURN(); - } - - return S_OK; -} diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index dd70668f029..6b494e4a9e0 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -25,14 +25,21 @@ class BitmapTests Log::Comment(L"Check dirty rectangles."); // Union up all the dirty rectangles into one big one. - auto dirtyExpected = bitsOn.front(); - for (auto it = bitsOn.cbegin() + 1; it < bitsOn.cend(); ++it) + if (bitsOn.empty()) { - dirtyExpected |= *it; + VERIFY_ARE_EQUAL(std::nullopt, map._dirty); } + else + { + auto dirtyExpected = bitsOn.front(); + for (auto it = bitsOn.cbegin() + 1; it < bitsOn.cend(); ++it) + { + dirtyExpected |= *it; + } - // Check if it matches. - VERIFY_ARE_EQUAL(dirtyExpected, map._dirty); + // Check if it matches. + VERIFY_ARE_EQUAL(dirtyExpected, map._dirty); + } Log::Comment(L"Check all bits in map."); // For every point in the map... @@ -64,7 +71,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -79,7 +86,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -107,7 +114,7 @@ class BitmapTests // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), true)); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); + VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); } else { @@ -118,6 +125,486 @@ class BitmapTests } } + TEST_METHOD(Equality) + { + Log::Comment(L"0.) Defaults are equal"); + { + const til::bitmap one; + const til::bitmap two; + VERIFY_IS_TRUE(one == two); + } + + Log::Comment(L"1.) Different sizes are unequal"); + { + const til::bitmap one{ til::size{ 2, 2 } }; + const til::bitmap two{ til::size{ 3, 3 } }; + VERIFY_IS_FALSE(one == two); + } + + Log::Comment(L"2.) Same bits set are equal"); + { + til::bitmap one{ til::size{ 2, 2 } }; + til::bitmap two{ til::size{ 2, 2 } }; + one.set(til::point{ 0, 1 }); + one.set(til::point{ 1, 0 }); + two.set(til::point{ 0, 1 }); + two.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(one == two); + } + + Log::Comment(L"3.) Different bits set are not equal"); + { + til::bitmap one{ til::size{ 2, 2 } }; + til::bitmap two{ til::size{ 2, 2 } }; + one.set(til::point{ 0, 1 }); + two.set(til::point{ 1, 0 }); + VERIFY_IS_FALSE(one == two); + } + } + + TEST_METHOD(Inequality) + { + Log::Comment(L"0.) Defaults are equal"); + { + const til::bitmap one; + const til::bitmap two; + VERIFY_IS_FALSE(one != two); + } + + Log::Comment(L"1.) Different sizes are unequal"); + { + const til::bitmap one{ til::size{ 2, 2 } }; + const til::bitmap two{ til::size{ 3, 3 } }; + VERIFY_IS_TRUE(one != two); + } + + Log::Comment(L"2.) Same bits set are equal"); + { + til::bitmap one{ til::size{ 2, 2 } }; + til::bitmap two{ til::size{ 2, 2 } }; + one.set(til::point{ 0, 1 }); + one.set(til::point{ 1, 0 }); + two.set(til::point{ 0, 1 }); + two.set(til::point{ 1, 0 }); + VERIFY_IS_FALSE(one != two); + } + + Log::Comment(L"3.) Different bits set are not equal"); + { + til::bitmap one{ til::size{ 2, 2 } }; + til::bitmap two{ til::size{ 2, 2 } }; + one.set(til::point{ 0, 1 }); + two.set(til::point{ 1, 0 }); + VERIFY_IS_TRUE(one != two); + } + } + + TEST_METHOD(Translate) + { + const til::size mapSize{ 4, 4 }; + til::bitmap map{ mapSize }; + + // set the middle four bits of the map. + // 0 0 0 0 + // 0 1 1 0 + // 0 1 1 0 + // 0 0 0 0 + map.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 2 } }); + + Log::Comment(L"1.) Move down and right"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v --> --> + const til::point delta{ 2, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 0 0 0 0 0 0 0 0 + // 0 1 1 0 0 0 0 0 0 0 0 0 + // 0 1 1 0 v --> 0 0 0 0 --> 0 0 0 0 + // 0 0 0 0 v 0 1 1 0 0 0 0 1 + // ->-> + expected.set(til::point{ 3, 3 }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Move down"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v + const til::point delta{ 0, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 0 0 0 0 + // 0 1 1 0 0 0 0 0 + // 0 1 1 0 v --> 0 0 0 0 + // 0 0 0 0 v 0 1 1 0 + expected.set(til::rectangle{ til::point{ 1, 3 }, til::size{ 2, 1 } }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Move down and left"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v <-- <-- + const til::point delta{ -2, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 0 0 0 0 0 0 0 0 + // 0 1 1 0 0 0 0 0 0 0 0 0 + // 0 1 1 0 v --> 0 0 0 0 --> 0 0 0 0 + // 0 0 0 0 v 0 1 1 0 1 0 0 0 + // <-<- + expected.set(til::point{ 0, 3 }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Move left"); + { + auto actual = map; + // Move all contents + // <-- <-- + const til::point delta{ -2, 0 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 0 0 0 0 + // 0 1 1 0 1 0 0 0 + // 0 1 1 0 --> 1 0 0 0 + // 0 0 0 0 0 0 0 0 + // <--<-- + expected.set(til::rectangle{ til::point{ 0, 1 }, til::size{ 1, 2 } }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"5.) Move up and left"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | <-- <-- + const til::point delta{ -2, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 ^ 0 1 1 0 1 0 0 0 + // 0 1 1 0 ^ 0 0 0 0 0 0 0 0 + // 0 1 1 0 --> 0 0 0 0 --> 0 0 0 0 + // 0 0 0 0 0 0 0 0 0 0 0 0 + // <-<- + expected.set(til::point{ 0, 0 }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"6.) Move up"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | + const til::point delta{ 0, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 ^ 0 1 1 0 + // 0 1 1 0 ^ 0 0 0 0 + // 0 1 1 0 --> 0 0 0 0 + // 0 0 0 0 0 0 0 0 + expected.set(til::rectangle{ til::point{ 1, 0 }, til::size{ 2, 1 } }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"7.) Move up and right"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | --> --> + const til::point delta{ 2, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 ^ 0 1 1 0 0 0 0 1 + // 0 1 1 0 ^ 0 0 0 0 0 0 0 0 + // 0 1 1 0 --> 0 0 0 0 --> 0 0 0 0 + // 0 0 0 0 0 0 0 0 0 0 0 0 + // ->-> + expected.set(til::point{ 3, 0 }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"8.) Move right"); + { + auto actual = map; + // Move all contents + // --> --> + const til::point delta{ 2, 0 }; + + til::bitmap expected{ mapSize }; + // Expected: + // 0 0 0 0 0 0 0 0 + // 0 1 1 0 0 0 0 1 + // 0 1 1 0 --> 0 0 0 1 + // 0 0 0 0 0 0 0 0 + // ->-> + expected.set(til::rectangle{ til::point{ 3, 1 }, til::size{ 1, 2 } }); + + actual.translate(delta); + + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(TranslateWithFill) + { + const til::size mapSize{ 4, 4 }; + til::bitmap map{ mapSize }; + + // set the middle four bits of the map. + // 0 0 0 0 + // 0 1 1 0 + // 0 1 1 0 + // 0 0 0 0 + map.set(til::rectangle{ til::point{ 1, 1 }, til::size{ 2, 2 } }); + + Log::Comment(L"1.) Move down and right"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v --> --> + const til::point delta{ 2, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 F F F F F F F F + // 0 1 1 0 F F F F F F F F + // 0 1 1 0 v --> 0 0 0 0 --> F F 0 0 + // 0 0 0 0 v 0 1 1 0 F F 0 1 + // ->-> + expected.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 2 } }); + expected.set(til::rectangle{ til::point{ 0, 2 }, til::size{ 2, 2 } }); + expected.set(til::point{ 3, 3 }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Move down"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v + const til::point delta{ 0, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 F F F F + // 0 1 1 0 F F F F + // 0 1 1 0 v --> 0 0 0 0 + // 0 0 0 0 v 0 1 1 0 + expected.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 2 } }); + expected.set(til::rectangle{ til::point{ 1, 3 }, til::size{ 2, 1 } }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Move down and left"); + { + auto actual = map; + // Move all contents + // | + // v + // | + // v <-- <-- + const til::point delta{ -2, 2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 F F F F F F F F + // 0 1 1 0 F F F F F F F F + // 0 1 1 0 v --> 0 0 0 0 --> 0 0 F F + // 0 0 0 0 v 0 1 1 0 1 0 F F + // <-<- + expected.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 2 } }); + expected.set(til::rectangle{ til::point{ 2, 2 }, til::size{ 2, 2 } }); + expected.set(til::point{ 0, 3 }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Move left"); + { + auto actual = map; + // Move all contents + // <-- <-- + const til::point delta{ -2, 0 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 0 0 F F + // 0 1 1 0 1 0 F F + // 0 1 1 0 --> 1 0 F F + // 0 0 0 0 0 0 F F + // <--<-- + expected.set(til::rectangle{ til::point{ 2, 0 }, til::size{ 2, 4 } }); + expected.set(til::rectangle{ til::point{ 0, 1 }, til::size{ 1, 2 } }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"5.) Move up and left"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | <-- <-- + const til::point delta{ -2, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 ^ 0 1 1 0 1 0 F F + // 0 1 1 0 ^ 0 0 0 0 0 0 F F + // 0 1 1 0 --> F F F F --> F F F F + // 0 0 0 0 F F F F F F F F + // <-<- + expected.set(til::rectangle{ til::point{ 2, 0 }, til::size{ 2, 2 } }); + expected.set(til::rectangle{ til::point{ 0, 2 }, til::size{ 4, 2 } }); + expected.set(til::point{ 0, 0 }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"6.) Move up"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | + const til::point delta{ 0, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 ^ 0 1 1 0 + // 0 1 1 0 ^ 0 0 0 0 + // 0 1 1 0 --> F F F F + // 0 0 0 0 F F F F + expected.set(til::rectangle{ til::point{ 1, 0 }, til::size{ 2, 1 } }); + expected.set(til::rectangle{ til::point{ 0, 2 }, til::size{ 4, 2 } }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"7.) Move up and right"); + { + auto actual = map; + // Move all contents + // ^ + // | + // ^ + // | --> --> + const til::point delta{ 2, -2 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 ^ 0 1 1 0 F F 0 1 + // 0 1 1 0 ^ 0 0 0 0 F F 0 0 + // 0 1 1 0 --> F F F F --> F F F F + // 0 0 0 0 F F F F F F F F + // ->-> + expected.set(til::point{ 3, 0 }); + expected.set(til::rectangle{ til::point{ 0, 2 }, til::size{ 4, 2 } }); + expected.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 2, 2 } }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"8.) Move right"); + { + auto actual = map; + // Move all contents + // --> --> + const til::point delta{ 2, 0 }; + + til::bitmap expected{ mapSize }; + // Expected: (F is filling uncovered value) + // 0 0 0 0 F F 0 0 + // 0 1 1 0 F F 0 1 + // 0 1 1 0 --> F F 0 1 + // 0 0 0 0 F F 0 0 + // ->-> + expected.set(til::rectangle{ til::point{ 3, 1 }, til::size{ 1, 2 } }); + expected.set(til::rectangle{ til::point{ 0, 0 }, til::size{ 2, 4 } }); + + actual.translate(delta, true); + + VERIFY_ARE_EQUAL(expected, actual); + } + } + TEST_METHOD(SetReset) { const til::size sz{ 4, 4 }; @@ -133,7 +620,8 @@ class BitmapTests const til::point point{ 2, 2 }; bitmap.set(point); - til::rectangle expectedSet{ point }; + std::vector expectedSet; + expectedSet.emplace_back(til::rectangle{ point }); // Run through every bit. Only the one we set should be true. Log::Comment(L"Only the bit we set should be true."); @@ -142,13 +630,14 @@ class BitmapTests Log::Comment(L"Setting all should mean they're all true."); bitmap.set_all(); - expectedSet = til::rectangle{ bitmap._rc }; + expectedSet.clear(); + expectedSet.emplace_back(til::rectangle{ bitmap._rc }); _checkBits(expectedSet, bitmap); Log::Comment(L"Now reset them all."); bitmap.reset_all(); - expectedSet = {}; + expectedSet.clear(); _checkBits(expectedSet, bitmap); til::rectangle totalZone{ sz }; @@ -160,13 +649,14 @@ class BitmapTests til::rectangle setZone{ til::point{ 0, 0 }, til::size{ 2, 3 } }; bitmap.set(setZone); - expectedSet = setZone; + expectedSet.clear(); + expectedSet.emplace_back(setZone); _checkBits(expectedSet, bitmap); Log::Comment(L"Reset all."); bitmap.reset_all(); - expectedSet = {}; + expectedSet.clear(); _checkBits(expectedSet, bitmap); } @@ -452,10 +942,10 @@ class BitmapTests VERIFY_ARE_EQUAL(expected, actual); Log::Comment(L"Set rectangle and validate runs updated."); - const til::rectangle setRect{ setPoint, til::size{2, 2} }; + const til::rectangle setRect{ setPoint, til::size{ 2, 2 } }; expected.clear(); - expected.push_back(til::rectangle{ til::point{2, 2}, til::size{2, 1} }); - expected.push_back(til::rectangle{ til::point{2, 3}, til::size{2, 1} }); + expected.push_back(til::rectangle{ til::point{ 2, 2 }, til::size{ 2, 1 } }); + expected.push_back(til::rectangle{ til::point{ 2, 3 }, til::size{ 2, 1 } }); map.set(setRect); actual.clear(); @@ -467,10 +957,10 @@ class BitmapTests Log::Comment(L"Set all and validate runs updated."); expected.clear(); - expected.push_back(til::rectangle{ til::point{0, 0}, til::size{4, 1} }); - expected.push_back(til::rectangle{ til::point{0, 1}, til::size{4, 1} }); - expected.push_back(til::rectangle{ til::point{0, 2}, til::size{4, 1} }); - expected.push_back(til::rectangle{ til::point{0, 3}, til::size{4, 1} }); + expected.push_back(til::rectangle{ til::point{ 0, 0 }, til::size{ 4, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 1 }, til::size{ 4, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 2 }, til::size{ 4, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 3 }, til::size{ 4, 1 } }); map.set_all(); actual.clear(); @@ -483,9 +973,9 @@ class BitmapTests Log::Comment(L"Resize and validate runs updated."); const til::size newSize{ 3, 3 }; expected.clear(); - expected.push_back(til::rectangle{ til::point{0, 0}, til::size{3, 1} }); - expected.push_back(til::rectangle{ til::point{0, 1}, til::size{3, 1} }); - expected.push_back(til::rectangle{ til::point{0, 2}, til::size{3, 1} }); + expected.push_back(til::rectangle{ til::point{ 0, 0 }, til::size{ 3, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 1 }, til::size{ 3, 1 } }); + expected.push_back(til::rectangle{ til::point{ 0, 2 }, til::size{ 3, 1 } }); map.resize(newSize); actual.clear(); diff --git a/src/til/ut_til/OperatorTests.cpp b/src/til/ut_til/OperatorTests.cpp new file mode 100644 index 00000000000..191c09e2484 --- /dev/null +++ b/src/til/ut_til/OperatorTests.cpp @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "til/operators.h" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +class OperatorTests +{ + TEST_CLASS(OperatorTests); + + TEST_METHOD(PointAddSize) + { + const til::point pt{ 5, 10 }; + const til::size sz{ 2, 4 }; + const til::point expected{ 5 + 2, 10 + 4 }; + const auto actual = pt + sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(PointSubSize) + { + const til::point pt{ 5, 10 }; + const til::size sz{ 2, 4 }; + const til::point expected{ 5 - 2, 10 - 4 }; + const auto actual = pt - sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(PointMulSize) + { + const til::point pt{ 5, 10 }; + const til::size sz{ 2, 4 }; + const til::point expected{ 5 * 2, 10 * 4 }; + const auto actual = pt * sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(PointDivSize) + { + const til::point pt{ 5, 10 }; + const til::size sz{ 2, 4 }; + const til::point expected{ 5 / 2, 10 / 4 }; + const auto actual = pt / sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(SizeAddPoint) + { + const til::size pt{ 5, 10 }; + const til::point sz{ 2, 4 }; + const til::size expected{ 5 + 2, 10 + 4 }; + const auto actual = pt + sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(SizeSubPoint) + { + const til::size pt{ 5, 10 }; + const til::point sz{ 2, 4 }; + const til::size expected{ 5 - 2, 10 - 4 }; + const auto actual = pt - sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(SizeMulPoint) + { + const til::size pt{ 5, 10 }; + const til::point sz{ 2, 4 }; + const til::size expected{ 5 * 2, 10 * 4 }; + const auto actual = pt * sz; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(SizeDivPoint) + { + const til::size pt{ 5, 10 }; + const til::point sz{ 2, 4 }; + const til::size expected{ 5 / 2, 10 / 4 }; + const auto actual = pt / sz; + VERIFY_ARE_EQUAL(expected, actual); + } +}; diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 47e17da9430..59fbf048ccc 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -440,6 +440,16 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, actual); } + TEST_METHOD(OrUnionInplace) + { + til::rectangle one{ 4, 6, 10, 14 }; + const til::rectangle two{ 5, 2, 13, 10 }; + + const til::rectangle expected{ 4, 2, 13, 14 }; + one |= two; + VERIFY_ARE_EQUAL(expected, one); + } + TEST_METHOD(AndIntersect) { const til::rectangle one{ 4, 6, 10, 14 }; @@ -450,6 +460,16 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, actual); } + TEST_METHOD(AndIntersectInplace) + { + til::rectangle one{ 4, 6, 10, 14 }; + const til::rectangle two{ 5, 2, 13, 10 }; + + const til::rectangle expected{ 5, 6, 10, 10 }; + one &= two; + VERIFY_ARE_EQUAL(expected, one); + } + TEST_METHOD(MinusSubtractSame) { const til::rectangle original{ 0, 0, 10, 10 }; @@ -585,6 +605,218 @@ class RectangleTests VERIFY_ARE_EQUAL(expected, actual); } + TEST_METHOD(AdditionPoint) + { + const til::rectangle start{ 10, 20, 30, 40 }; + const til::point pt{ 3, 7 }; + const til::rectangle expected{ 10 + 3, 20 + 7, 30 + 3, 40 + 7 }; + const auto actual = start + pt; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(AdditionPointInplace) + { + til::rectangle start{ 10, 20, 30, 40 }; + const til::point pt{ 3, 7 }; + const til::rectangle expected{ 10 + 3, 20 + 7, 30 + 3, 40 + 7 }; + start += pt; + VERIFY_ARE_EQUAL(expected, start); + } + + TEST_METHOD(SubtractionPoint) + { + const til::rectangle start{ 10, 20, 30, 40 }; + const til::point pt{ 3, 7 }; + const til::rectangle expected{ 10 - 3, 20 - 7, 30 - 3, 40 - 7 }; + const auto actual = start - pt; + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(SubtractionPointInplace) + { + til::rectangle start{ 10, 20, 30, 40 }; + const til::point pt{ 3, 7 }; + const til::rectangle expected{ 10 - 3, 20 - 7, 30 - 3, 40 - 7 }; + start -= pt; + VERIFY_ARE_EQUAL(expected, start); + } + + TEST_METHOD(AdditionSize) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Add size to bottom and right"); + { + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10, 20, 33, 47 }; + const auto actual = start + scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Add size to top and left"); + { + const til::size scale{ -3, -7 }; + const til::rectangle expected{ 7, 13, 30, 40 }; + const auto actual = start + scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Add size to bottom and left"); + { + const til::size scale{ -3, 7 }; + const til::rectangle expected{ 7, 20, 30, 47 }; + const auto actual = start + scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Add size to top and right"); + { + const til::size scale{ 3, -7 }; + const til::rectangle expected{ 10, 13, 33, 40 }; + const auto actual = start + scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(AdditionSizeInplace) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Add size to bottom and right"); + { + auto actual = start; + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10, 20, 33, 47 }; + actual += scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Add size to top and left"); + { + auto actual = start; + const til::size scale{ -3, -7 }; + const til::rectangle expected{ 7, 13, 30, 40 }; + actual += scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Add size to bottom and left"); + { + auto actual = start; + const til::size scale{ -3, 7 }; + const til::rectangle expected{ 7, 20, 30, 47 }; + actual += scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Add size to top and right"); + { + auto actual = start; + const til::size scale{ 3, -7 }; + const til::rectangle expected{ 10, 13, 33, 40 }; + actual += scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(SubtractionSize) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Subtract size from bottom and right"); + { + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10, 20, 27, 33 }; + const auto actual = start - scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Subtract size from top and left"); + { + const til::size scale{ -3, -7 }; + const til::rectangle expected{ 13, 27, 30, 40 }; + const auto actual = start - scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Subtract size from bottom and left"); + { + const til::size scale{ -3, 7 }; + const til::rectangle expected{ 13, 20, 30, 33 }; + const auto actual = start - scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Subtract size from top and right"); + { + const til::size scale{ 3, -6 }; + const til::rectangle expected{ 10, 26, 27, 40 }; + const auto actual = start - scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(SubtractionSizeInplace) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Subtract size from bottom and right"); + { + auto actual = start; + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10, 20, 27, 33 }; + actual -= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Subtract size from top and left"); + { + auto actual = start; + const til::size scale{ -3, -7 }; + const til::rectangle expected{ 13, 27, 30, 40 }; + actual -= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"3.) Subtract size from bottom and left"); + { + auto actual = start; + const til::size scale{ -3, 7 }; + const til::rectangle expected{ 13, 20, 30, 33 }; + actual -= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"4.) Subtract size from top and right"); + { + auto actual = start; + const til::size scale{ 3, -6 }; + const til::rectangle expected{ 10, 26, 27, 40 }; + actual -= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(MultiplicationSize) + { + const til::rectangle start{ 10, 20, 30, 40 }; + const til::size size{ 3, 7 }; + const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; + const auto actual = start * size; + + VERIFY_ARE_EQUAL(expected, actual); + } + + TEST_METHOD(MultiplicationSizeInplace) + { + til::rectangle start{ 10, 20, 30, 40 }; + const til::size size{ 3, 7 }; + const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; + start *= size; + + VERIFY_ARE_EQUAL(expected, start); + } + TEST_METHOD(Top) { const til::rectangle rc{ 5, 10, 15, 20 }; diff --git a/src/til/ut_til/til.unit.tests.vcxproj b/src/til/ut_til/til.unit.tests.vcxproj index 644322da1c1..d2e91bf250d 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj +++ b/src/til/ut_til/til.unit.tests.vcxproj @@ -11,6 +11,7 @@ + diff --git a/src/til/ut_til/til.unit.tests.vcxproj.filters b/src/til/ut_til/til.unit.tests.vcxproj.filters index e07ae69b990..21bf4896239 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj.filters +++ b/src/til/ut_til/til.unit.tests.vcxproj.filters @@ -12,6 +12,7 @@ + From 31eadef078c56077355460997c5b8b642452a4cb Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 15:15:09 -0700 Subject: [PATCH 16/25] finish xterm engine changes, make tests work again with the new invalidation, run codeformat pass. --- src/host/ut_host/VtRendererTests.cpp | 154 +++++++++++++++++++++------ src/inc/til/some.h | 3 +- src/renderer/vt/XtermEngine.cpp | 3 +- src/renderer/vt/math.cpp | 5 +- src/renderer/vt/tracing.cpp | 2 - src/renderer/vt/vtrenderer.hpp | 2 +- 6 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 92410495822..3f1980aad75 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -257,13 +257,13 @@ void VtRendererTest::Xterm256TestInvalidate() VERIFY_IS_FALSE(engine->_firstPaint); }); - Viewport view = SetUpViewport(); + const Viewport view = SetUpViewport(); Log::Comment(NoThrowString().Format( L"Make sure that invalidating all invalidates the whole viewport.")); VERIFY_SUCCEEDED(engine->InvalidateAll()); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); }); Log::Comment(NoThrowString().Format( @@ -271,7 +271,8 @@ void VtRendererTest::Xterm256TestInvalidate() SMALL_RECT invalid = { 1, 1, 1, 1 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + VERIFY_IS_TRUE(engine->_invalidMap.one()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( @@ -284,7 +285,9 @@ void VtRendererTest::Xterm256TestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 1; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(1u, runs.size()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); qExpectedInput.push_back("\x1b[H"); // Go Home qExpectedInput.push_back("\x1b[L"); // insert a line @@ -300,7 +303,17 @@ void VtRendererTest::Xterm256TestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); // We would expect a CUP here, but the cursor is already at the home position qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -314,7 +327,9 @@ void VtRendererTest::Xterm256TestInvalidate() invalid = view.ToExclusive(); invalid.Top = invalid.Bottom - 1; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(1u, runs.size()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); qExpectedInput.push_back("\x1b[32;1H"); // Bottom of buffer qExpectedInput.push_back("\n"); // Scroll down once @@ -329,7 +344,17 @@ void VtRendererTest::Xterm256TestInvalidate() invalid = view.ToExclusive(); invalid.Top = invalid.Bottom - 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); // We would expect a CUP here, but we're already at the bottom from the last call. qExpectedInput.push_back("\n\n\n"); // Scroll down three times @@ -349,7 +374,18 @@ void VtRendererTest::Xterm256TestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + qExpectedInput.push_back("\x1b[H"); // Go to home qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -357,21 +393,27 @@ void VtRendererTest::Xterm256TestInvalidate() scrollDelta = { 0, 1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - Log::Comment(NoThrowString().Format( - VerifyOutputTraits::ToString(engine->_invalidRect.ToExclusive()))); + Log::Comment(engine->_invalidMap.to_string().c_str()); scrollDelta = { 0, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - Log::Comment(NoThrowString().Format( - VerifyOutputTraits::ToString(engine->_invalidRect.ToExclusive()))); + Log::Comment(engine->_invalidMap.to_string().c_str()); qExpectedInput.push_back("\x1b[2J"); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one down and one up, nothing should change ----" L" But it still does for now MSFT:14169294")); - invalid = view.ToExclusive(); - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + + const auto runs = engine->_invalidMap.runs(); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the entire view + VERIFY_ARE_EQUAL(til::rectangle{ view.ToInclusive() }, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); }); @@ -730,7 +772,7 @@ void VtRendererTest::XtermTestInvalidate() L"Make sure that invalidating all invalidates the whole viewport.")); VERIFY_SUCCEEDED(engine->InvalidateAll()); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); }); Log::Comment(NoThrowString().Format( @@ -738,7 +780,8 @@ void VtRendererTest::XtermTestInvalidate() SMALL_RECT invalid = { 1, 1, 1, 1 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + VERIFY_IS_TRUE(engine->_invalidMap.one()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( @@ -751,7 +794,9 @@ void VtRendererTest::XtermTestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 1; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(1u, runs.size()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); qExpectedInput.push_back("\x1b[H"); // Go Home qExpectedInput.push_back("\x1b[L"); // insert a line @@ -766,7 +811,17 @@ void VtRendererTest::XtermTestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); // We would expect a CUP here, but the cursor is already at the home position qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -780,7 +835,9 @@ void VtRendererTest::XtermTestInvalidate() invalid = view.ToExclusive(); invalid.Top = invalid.Bottom - 1; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(1u, runs.size()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); qExpectedInput.push_back("\x1b[32;1H"); // Bottom of buffer qExpectedInput.push_back("\n"); // Scroll down once @@ -795,7 +852,17 @@ void VtRendererTest::XtermTestInvalidate() invalid = view.ToExclusive(); invalid.Top = invalid.Bottom - 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); // We would expect a CUP here, but we're already at the bottom from the last call. qExpectedInput.push_back("\n\n\n"); // Scroll down three times @@ -815,7 +882,18 @@ void VtRendererTest::XtermTestInvalidate() invalid = view.ToExclusive(); invalid.Bottom = 3; - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + // we should have 3 runs and build a rectangle out of them + const auto runs = engine->_invalidMap.runs(); + VERIFY_ARE_EQUAL(3u, runs.size()); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the invalid one. + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + qExpectedInput.push_back("\x1b[H"); // Go to home qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -823,21 +901,27 @@ void VtRendererTest::XtermTestInvalidate() scrollDelta = { 0, 1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - Log::Comment(NoThrowString().Format( - VerifyOutputTraits::ToString(engine->_invalidRect.ToExclusive()))); + Log::Comment(engine->_invalidMap.to_string().c_str()); scrollDelta = { 0, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - Log::Comment(NoThrowString().Format( - VerifyOutputTraits::ToString(engine->_invalidRect.ToExclusive()))); + Log::Comment(engine->_invalidMap.to_string().c_str()); qExpectedInput.push_back("\x1b[2J"); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one down and one up, nothing should change ----" L" But it still does for now MSFT:14169294")); - invalid = view.ToExclusive(); - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + + const auto runs = engine->_invalidMap.runs(); + auto invalidRect = runs.front(); + for (size_t i = 1; i < runs.size(); ++i) + { + invalidRect |= runs[i]; + } + + // verify the rect matches the entire view + VERIFY_ARE_EQUAL(til::rectangle{ view.ToInclusive() }, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); }); @@ -1051,7 +1135,7 @@ void VtRendererTest::WinTelnetTestInvalidate() L"Make sure that invalidating all invalidates the whole viewport.")); VERIFY_SUCCEEDED(engine->InvalidateAll()); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); }); Log::Comment(NoThrowString().Format( @@ -1059,7 +1143,7 @@ void VtRendererTest::WinTelnetTestInvalidate() SMALL_RECT invalid = { 1, 1, 1, 1 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); + VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( @@ -1067,7 +1151,7 @@ void VtRendererTest::WinTelnetTestInvalidate() COORD scrollDelta = { 0, 1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); // sentinel VERIFY_SUCCEEDED(engine->ScrollFrame()); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback @@ -1076,7 +1160,7 @@ void VtRendererTest::WinTelnetTestInvalidate() scrollDelta = { 0, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->ScrollFrame()); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback @@ -1085,7 +1169,7 @@ void VtRendererTest::WinTelnetTestInvalidate() scrollDelta = { 1, 0 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->ScrollFrame()); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback @@ -1094,7 +1178,7 @@ void VtRendererTest::WinTelnetTestInvalidate() scrollDelta = { -1, 0 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->ScrollFrame()); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback @@ -1103,7 +1187,7 @@ void VtRendererTest::WinTelnetTestInvalidate() scrollDelta = { 1, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(view, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); VERIFY_SUCCEEDED(engine->ScrollFrame()); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback @@ -1351,7 +1435,7 @@ void VtRendererTest::TestResize() VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive())); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(newView, engine->_invalidRect); + VERIFY_IS_TRUE(engine->_invalidMap.all()); VERIFY_IS_FALSE(engine->_firstPaint); VERIFY_IS_FALSE(engine->_suppressResizeRepaint); }); diff --git a/src/inc/til/some.h b/src/inc/til/some.h index 5e5c209b27f..96d523c486b 100644 --- a/src/inc/til/some.h +++ b/src/inc/til/some.h @@ -212,7 +212,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" std::wstring to_string() const { std::wstringstream wss; - wss << std::endl << L"Some contains " << size() << " of max size " << max_size() << ":" << std::endl; + wss << std::endl + << L"Some contains " << size() << " of max size " << max_size() << ":" << std::endl; wss << L"Elements:" << std::endl; for (auto& item : *this) diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index f5e1eec88bc..55aa2bbf4c2 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -410,7 +410,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { // Scroll the current offset and invalidate the revealed area _invalidMap.translate(til::point(*pcoordDelta), true); - + + // TODO: MIGRIE DO WE NEED THIS IF THE TRANSLATION ABOVE FILLS THE VACATED AREA? // Add the top/bottom of the window to the invalid area SMALL_RECT invalid = _lastViewport.ToOrigin().ToExclusive(); diff --git a/src/renderer/vt/math.cpp b/src/renderer/vt/math.cpp index 4198dcdad0c..a5aaa7129fe 100644 --- a/src/renderer/vt/math.cpp +++ b/src/renderer/vt/math.cpp @@ -78,12 +78,11 @@ bool VtEngine::_WillWriteSingleChar() const // Get the single point at which things are invalid. const auto invalidPoint = _invalidMap.runs().front().origin(); - // Either the next character to the right or the immediately previous // character should follow this code path // (The immediate previous character would suggest a backspace) - bool invalidIsNext = invalidPoint == til::point{ _lastText }; - bool invalidIsLast = invalidPoint == til::point{ _lastText.X - 1, _lastText.Y }; + bool invalidIsNext = invalidPoint == til::point{ _lastText }; + bool invalidIsLast = invalidPoint == til::point{ _lastText.X - 1, _lastText.Y }; return invalidIsNext || invalidIsLast; } diff --git a/src/renderer/vt/tracing.cpp b/src/renderer/vt/tracing.cpp index 74048bbf5ad..023a5453f80 100644 --- a/src/renderer/vt/tracing.cpp +++ b/src/renderer/vt/tracing.cpp @@ -136,8 +136,6 @@ void RenderTracing::TraceStartPaint(const bool quickReturn, #ifndef UNIT_TESTING if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) { - - const auto invalidatedStr = invalidMap.to_string(); const auto invalidated = invalidatedStr.c_str(); const auto lastViewStr = lastViewport.to_string(); diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index e54fdb10204..7ebb7a71a87 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -121,7 +121,7 @@ namespace Microsoft::Console::Render bool _lastWasBold; Microsoft::Console::Types::Viewport _lastViewport; - + til::bitmap _invalidMap; COORD _lastRealCursor; From 0651df1f711d62e1d480421eb96b079fa9acbe6a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 15:46:43 -0700 Subject: [PATCH 17/25] SA pass and code format. --- src/inc/til/rectangle.h | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index f167ac46786..41c476bd1eb 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -418,16 +418,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // ADD will translate (offset) the rectangle by the point. rectangle operator+(const point& point) const { - // Fetch the pieces of the rectangle. - auto l = left(); - auto r = right(); - auto t = top(); - auto b = bottom(); + ptrdiff_t l, t, r, b; - l += point.x(); - r += point.x(); - t += point.y(); - b += point.y(); + THROW_HR_IF(E_ABORT, !::base::CheckAdd(left(), point.x()).AssignIfValid(&l)); + THROW_HR_IF(E_ABORT, !::base::CheckAdd(top(), point.y()).AssignIfValid(&t)); + THROW_HR_IF(E_ABORT, !::base::CheckAdd(right(), point.x()).AssignIfValid(&r)); + THROW_HR_IF(E_ABORT, !::base::CheckAdd(bottom(), point.y()).AssignIfValid(&b)); return til::rectangle{ til::point{ l, t }, til::point{ r, b } }; } @@ -441,16 +437,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // SUB will translate (offset) the rectangle by the point. rectangle operator-(const point& point) const { - // Fetch the pieces of the rectangle. - auto l = left(); - auto r = right(); - auto t = top(); - auto b = bottom(); + ptrdiff_t l, t, r, b; - l -= point.x(); - r -= point.x(); - t -= point.y(); - b -= point.y(); + THROW_HR_IF(E_ABORT, !::base::CheckSub(left(), point.x()).AssignIfValid(&l)); + THROW_HR_IF(E_ABORT, !::base::CheckSub(top(), point.y()).AssignIfValid(&t)); + THROW_HR_IF(E_ABORT, !::base::CheckSub(right(), point.x()).AssignIfValid(&r)); + THROW_HR_IF(E_ABORT, !::base::CheckSub(bottom(), point.y()).AssignIfValid(&b)); return til::rectangle{ til::point{ l, t }, til::point{ r, b } }; } From cf06e979a175d6083de81992d5cc615e8df12883 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 16:02:53 -0700 Subject: [PATCH 18/25] inclusive/exclusive mismatch! *shakes fist at sky* --- src/renderer/vt/invalidate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp index 784006496bd..597f56b0f06 100644 --- a/src/renderer/vt/invalidate.cpp +++ b/src/renderer/vt/invalidate.cpp @@ -49,7 +49,7 @@ using namespace Microsoft::Console::Render; [[nodiscard]] HRESULT VtEngine::Invalidate(const SMALL_RECT* const psrRegion) noexcept try { - const til::rectangle rect{ *psrRegion }; + const til::rectangle rect{ Viewport::FromExclusive(*psrRegion).ToInclusive() }; _trace.TraceInvalidate(rect); _invalidMap.set(rect); return S_OK; From 59bfce1db972b3ef178e31771d21068a6383a348 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 16:22:32 -0700 Subject: [PATCH 19/25] Update simple tests to compensate for optimized invalidation rectangle. --- .../ConptyRoundtripTests.cpp | 15 ++++++--------- src/host/ut_host/ConptyOutputTests.cpp | 15 ++++++--------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index dae8a248f77..83364e95b7a 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -347,17 +347,14 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() expectedOutput.push_back("AAA"); expectedOutput.push_back("\r\n"); expectedOutput.push_back("BBB"); - expectedOutput.push_back("\r\n"); - // Here, we're going to emit 3 spaces. The region that got invalidated was a - // rectangle from 0,0 to 3,3, so the vt renderer will try to render the - // region in between BBB and CCC as well, because it got included in the - // rectangle Or() operation. - // This behavior should not be seen as binding - if a future optimization - // breaks this test, it wouldn't be the worst. - expectedOutput.push_back(" "); - expectedOutput.push_back("\r\n"); + // Jump down to the fourth line because emitting spaces didn't do anything + // and we will skip to emitting the CCC segment. + expectedOutput.push_back("\x1b[4;1H"); expectedOutput.push_back("CCC"); + // Cursor goes back on. + expectedOutput.push_back("\x1b[?25h"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); verifyData(termTb); diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index c24ec99fa15..07493ab4544 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -306,17 +306,14 @@ void ConptyOutputTests::WriteAFewSimpleLines() expectedOutput.push_back("AAA"); expectedOutput.push_back("\r\n"); expectedOutput.push_back("BBB"); - expectedOutput.push_back("\r\n"); - // Here, we're going to emit 3 spaces. The region that got invalidated was a - // rectangle from 0,0 to 3,3, so the vt renderer will try to render the - // region in between BBB and CCC as well, because it got included in the - // rectangle Or() operation. - // This behavior should not be seen as binding - if a future optimization - // breaks this test, it wouldn't be the worst. - expectedOutput.push_back(" "); - expectedOutput.push_back("\r\n"); + // Jump down to the fourth line because emitting spaces didn't do anything + // and we will skip to emitting the CCC segment. + expectedOutput.push_back("\x1b[4;1H"); expectedOutput.push_back("CCC"); + // Cursor goes back on. + expectedOutput.push_back("\x1b[?25h"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); } From a48919e683485da67a0f6d0a516834e62da6678c Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Thu, 19 Mar 2020 16:32:20 -0700 Subject: [PATCH 20/25] Remove testing for clear/paint of non-dirty regions (per new algorithm) --- src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 83364e95b7a..f861f882279 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -455,14 +455,10 @@ void ConptyRoundtripTests::TestAdvancedWrapping() expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)"); // Without line breaking, write the remaining 20 chars expectedOutput.push_back(R"(qrstuvwxyz{|}~!"#$%&)"); - // Clear the rest of row 1 - expectedOutput.push_back("\x1b[K"); // This is the hard line break expectedOutput.push_back("\r\n"); // Now write row 2 of the buffer expectedOutput.push_back(" 1234567890"); - // and clear everything after the text, because the buffer is empty. - expectedOutput.push_back("\x1b[K"); VERIFY_SUCCEEDED(renderer.PaintFrame()); verifyBuffer(termTb); From 54ce99ec56017409dccbe5b0e3967b3c7b2a43f7 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 20 Mar 2020 09:37:28 -0700 Subject: [PATCH 21/25] Fix the vt renderer tests to use the correct expected invalid rectangles (inclusive/exclusive bug.) Fix one of the tests to not be comparing to internal details based on a 0-size exclusive circumstance that only worked with or-rect. Fix importing of til for unit tests in the library includes so the WEX/TAEF templates show up appropriately (cascaded to changing renderervt.unittest lib compilation to use shared testing props so it works too.) --- src/common.build.tests.props | 2 +- src/host/ut_host/VtRendererTests.cpp | 64 +++++++++++++++------- src/inc/LibraryIncludes.h | 6 ++ src/renderer/vt/ut_lib/vt.unittest.vcxproj | 7 +-- 4 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/common.build.tests.props b/src/common.build.tests.props index 8a3527ff6e6..79410878836 100644 --- a/src/common.build.tests.props +++ b/src/common.build.tests.props @@ -2,7 +2,7 @@ - UNIT_TESTING;%(PreprocessorDefinitions) + INLINE_TEST_METHOD_MARKUP;UNIT_TESTING;%(PreprocessorDefinitions) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 3f1980aad75..a6ca4e34fa6 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -268,11 +268,11 @@ void VtRendererTest::Xterm256TestInvalidate() Log::Comment(NoThrowString().Format( L"Make sure that invalidating anything only invalidates that portion")); - SMALL_RECT invalid = { 1, 1, 1, 1 }; + SMALL_RECT invalid = { 1, 1, 2, 2 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { VERIFY_IS_TRUE(engine->_invalidMap.one()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( @@ -287,7 +287,7 @@ void VtRendererTest::Xterm256TestInvalidate() const auto runs = engine->_invalidMap.runs(); VERIFY_ARE_EQUAL(1u, runs.size()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, runs.front()); qExpectedInput.push_back("\x1b[H"); // Go Home qExpectedInput.push_back("\x1b[L"); // insert a line @@ -313,7 +313,7 @@ void VtRendererTest::Xterm256TestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); // We would expect a CUP here, but the cursor is already at the home position qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -329,7 +329,7 @@ void VtRendererTest::Xterm256TestInvalidate() const auto runs = engine->_invalidMap.runs(); VERIFY_ARE_EQUAL(1u, runs.size()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, runs.front()); qExpectedInput.push_back("\x1b[32;1H"); // Bottom of buffer qExpectedInput.push_back("\n"); // Scroll down once @@ -354,7 +354,7 @@ void VtRendererTest::Xterm256TestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); // We would expect a CUP here, but we're already at the bottom from the last call. qExpectedInput.push_back("\n\n\n"); // Scroll down three times @@ -384,7 +384,7 @@ void VtRendererTest::Xterm256TestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); qExpectedInput.push_back("\x1b[H"); // Go to home qExpectedInput.push_back("\x1b[3L"); // insert 3 lines @@ -412,8 +412,20 @@ void VtRendererTest::Xterm256TestInvalidate() invalidRect |= runs[i]; } - // verify the rect matches the entire view - VERIFY_ARE_EQUAL(til::rectangle{ view.ToInclusive() }, invalidRect); + // only the bottom line should be dirty. + // When we scrolled down, the bitmap looked like this: + // 1111 + // 0000 + // 0000 + // 0000 + // And then we scrolled up and the top line fell off and a bottom + // line was filled in like this: + // 0000 + // 0000 + // 0000 + // 1111 + const til::rectangle expected{ til::point{view.Left(), view.BottomInclusive()}, til::size{view.Width(), 1} }; + VERIFY_ARE_EQUAL(expected, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); }); @@ -777,11 +789,11 @@ void VtRendererTest::XtermTestInvalidate() Log::Comment(NoThrowString().Format( L"Make sure that invalidating anything only invalidates that portion")); - SMALL_RECT invalid = { 1, 1, 1, 1 }; + SMALL_RECT invalid = { 1, 1, 2, 2 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { VERIFY_IS_TRUE(engine->_invalidMap.one()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( @@ -796,7 +808,7 @@ void VtRendererTest::XtermTestInvalidate() const auto runs = engine->_invalidMap.runs(); VERIFY_ARE_EQUAL(1u, runs.size()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, runs.front()); qExpectedInput.push_back("\x1b[H"); // Go Home qExpectedInput.push_back("\x1b[L"); // insert a line @@ -821,7 +833,7 @@ void VtRendererTest::XtermTestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); // We would expect a CUP here, but the cursor is already at the home position qExpectedInput.push_back("\x1b[3L"); // insert 3 lines VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -837,7 +849,7 @@ void VtRendererTest::XtermTestInvalidate() const auto runs = engine->_invalidMap.runs(); VERIFY_ARE_EQUAL(1u, runs.size()); - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, runs.front()); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, runs.front()); qExpectedInput.push_back("\x1b[32;1H"); // Bottom of buffer qExpectedInput.push_back("\n"); // Scroll down once @@ -862,7 +874,7 @@ void VtRendererTest::XtermTestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); // We would expect a CUP here, but we're already at the bottom from the last call. qExpectedInput.push_back("\n\n\n"); // Scroll down three times @@ -892,7 +904,7 @@ void VtRendererTest::XtermTestInvalidate() } // verify the rect matches the invalid one. - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, invalidRect); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, invalidRect); qExpectedInput.push_back("\x1b[H"); // Go to home qExpectedInput.push_back("\x1b[3L"); // insert 3 lines @@ -920,8 +932,20 @@ void VtRendererTest::XtermTestInvalidate() invalidRect |= runs[i]; } - // verify the rect matches the entire view - VERIFY_ARE_EQUAL(til::rectangle{ view.ToInclusive() }, invalidRect); + // only the bottom line should be dirty. + // When we scrolled down, the bitmap looked like this: + // 1111 + // 0000 + // 0000 + // 0000 + // And then we scrolled up and the top line fell off and a bottom + // line was filled in like this: + // 0000 + // 0000 + // 0000 + // 1111 + const til::rectangle expected{ til::point{view.Left(), view.BottomInclusive()}, til::size{view.Width(), 1} }; + VERIFY_ARE_EQUAL(expected, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); }); @@ -1140,10 +1164,10 @@ void VtRendererTest::WinTelnetTestInvalidate() Log::Comment(NoThrowString().Format( L"Make sure that invalidating anything only invalidates that portion")); - SMALL_RECT invalid = { 1, 1, 1, 1 }; + SMALL_RECT invalid = { 1, 1, 2, 2 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); TestPaint(*engine, [&]() { - VERIFY_ARE_EQUAL(til::rectangle{ invalid }, *(engine->_invalidMap.begin())); + VERIFY_ARE_EQUAL(til::rectangle{ Viewport::FromExclusive(invalid).ToInclusive() }, *(engine->_invalidMap.begin())); }); Log::Comment(NoThrowString().Format( diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 2002fa48fb0..92b572075e3 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -85,6 +85,12 @@ // WRL #include +// WEX/TAEF testing +// Include before TIL if we're unit testing so it can light up WEX/TAEF template extensions +#ifdef UNIT_TESTING +#include +#endif + // TIL - Terminal Implementation Library #ifndef BLOCK_TIL // Certain projects may want to include TIL manually to gain superpowers #include "til.h" diff --git a/src/renderer/vt/ut_lib/vt.unittest.vcxproj b/src/renderer/vt/ut_lib/vt.unittest.vcxproj index b2f71422a17..686f185ac0a 100644 --- a/src/renderer/vt/ut_lib/vt.unittest.vcxproj +++ b/src/renderer/vt/ut_lib/vt.unittest.vcxproj @@ -10,13 +10,8 @@ - - - INLINE_TEST_METHOD_MARKUP;UNIT_TESTING;%(PreprocessorDefinitions) - - - + From 8647ae0f5c8a5ac39c80c3dfd50c5888b10ec755 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 20 Mar 2020 09:37:55 -0700 Subject: [PATCH 22/25] Format pass. --- src/host/ut_host/VtRendererTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index a6ca4e34fa6..4be9db4b85a 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -424,7 +424,7 @@ void VtRendererTest::Xterm256TestInvalidate() // 0000 // 0000 // 1111 - const til::rectangle expected{ til::point{view.Left(), view.BottomInclusive()}, til::size{view.Width(), 1} }; + const til::rectangle expected{ til::point{ view.Left(), view.BottomInclusive() }, til::size{ view.Width(), 1 } }; VERIFY_ARE_EQUAL(expected, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); @@ -944,7 +944,7 @@ void VtRendererTest::XtermTestInvalidate() // 0000 // 0000 // 1111 - const til::rectangle expected{ til::point{view.Left(), view.BottomInclusive()}, til::size{view.Width(), 1} }; + const til::rectangle expected{ til::point{ view.Left(), view.BottomInclusive() }, til::size{ view.Width(), 1 } }; VERIFY_ARE_EQUAL(expected, invalidRect); VERIFY_SUCCEEDED(engine->ScrollFrame()); From 0fc48b90b8eadfd42479151afdb8887b8e3c1b9a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 20 Mar 2020 15:05:48 -0700 Subject: [PATCH 23/25] PR feedback. Restoring non-optional dirty. Removing some leftover comments. Taking out the MUL for scaling since it's unused and confusing. --- .../ConptyRoundtripTests.cpp | 4 -- src/inc/til.h | 2 + src/inc/til/bitmap.h | 38 ++++--------------- src/inc/til/operators.h | 2 - src/inc/til/rectangle.h | 23 ----------- src/renderer/vt/XtermEngine.cpp | 14 ------- src/til/ut_til/RectangleTests.cpp | 20 ---------- src/til/ut_til/sources | 6 +++ 8 files changed, 16 insertions(+), 93 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index f861f882279..981be1c4b20 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -530,8 +530,6 @@ void ConptyRoundtripTests::TestExactWrappingWithoutSpaces() expectedOutput.push_back("\r\n"); // Now write row 2 of the buffer expectedOutput.push_back("1234567890"); - // and clear everything after the text, because the buffer is empty. - expectedOutput.push_back("\x1b[K"); VERIFY_SUCCEEDED(renderer.PaintFrame()); verifyBuffer(termTb); @@ -594,8 +592,6 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() expectedOutput.push_back("\r\n"); // Now write row 2 of the buffer expectedOutput.push_back(" 1234567890"); - // and clear everything after the text, because the buffer is empty. - expectedOutput.push_back("\x1b[K"); VERIFY_SUCCEEDED(renderer.PaintFrame()); verifyBuffer(termTb); diff --git a/src/inc/til.h b/src/inc/til.h index 289a8c52efb..5204284a004 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -3,6 +3,8 @@ #pragma once +#define _TIL_INLINEPREFIX __declspec(noinline) inline + #include "til/at.h" #include "til/color.h" #include "til/some.h" diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index fa58ae9e7a3..6cda4d31512 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -149,13 +149,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(sz.area(), fill), - _dirty(), + _dirty(fill ? sz : til::rectangle{ sz }), _runs{} { - if (fill) - { - _dirty = til::rectangle{ sz }; - } } constexpr bool operator==(const bitmap& other) const noexcept @@ -190,14 +186,10 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // If there's only one square dirty, quick save it off and be done. if (one()) { - // We're const for the sake of actually manipulating the dirty state. - // But we remove const for updating the cache. - _runs.emplace({ _dirty.value() }); + _runs.emplace({ _dirty }); } else { - // We're const for the sake of actually manipulating the dirty state. - // But we remove const for updating the cache. _runs.emplace(begin(), end()); } } @@ -279,14 +271,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" til::at(_bits, _rc.index_of(pt)) = true; - if (_dirty.has_value()) - { - _dirty.value() |= til::rectangle{ pt }; - } - else - { - _dirty = til::rectangle{ pt }; - } + _dirty |= til::rectangle{ pt }; } void set(const til::rectangle rc) @@ -299,14 +284,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" til::at(_bits, _rc.index_of(pt)) = true; } - if (_dirty.has_value()) - { - _dirty.value() |= rc; - } - else - { - _dirty = rc; - } + _dirty |= rc; } void set_all() @@ -387,7 +365,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" bool one() const { - return _dirty.has_value() && _dirty.value().size() == til::size{ 1, 1 }; + return _dirty.size() == til::size{ 1, 1 }; } constexpr bool any() const noexcept @@ -397,12 +375,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool none() const noexcept { - return !_dirty.has_value(); + return _dirty.empty(); } constexpr bool all() const noexcept { - return _dirty.has_value() && _dirty.value() == _rc; + return _dirty == _rc; } std::wstring to_string() const @@ -421,7 +399,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: - std::optional _dirty; + til::rectangle _dirty; til::size _sz; til::rectangle _rc; std::vector _bits; diff --git a/src/inc/til/operators.h b/src/inc/til/operators.h index a9a19d4bf80..109cd3e3576 100644 --- a/src/inc/til/operators.h +++ b/src/inc/til/operators.h @@ -7,8 +7,6 @@ #include "size.h" #include "bitmap.h" -#define _TIL_INLINEPREFIX __declspec(noinline) inline - namespace til // Terminal Implementation Library. Also: "Today I Learned" { // Operators go here when they involve two headers that can't/don't include each other. diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 41c476bd1eb..19f684f589f 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -636,29 +636,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return *this; } - // MUL will scale the entire rectangle by the size L/R * WIDTH and T/B * HEIGHT. - rectangle operator*(const size& size) const - { - ptrdiff_t l; - THROW_HR_IF(E_ABORT, !base::CheckMul(left(), size.width()).AssignIfValid(&l)); - - ptrdiff_t t; - THROW_HR_IF(E_ABORT, !base::CheckMul(top(), size.height()).AssignIfValid(&t)); - - ptrdiff_t r; - THROW_HR_IF(E_ABORT, !base::CheckMul(right(), size.width()).AssignIfValid(&r)); - - ptrdiff_t b; - THROW_HR_IF(E_ABORT, !base::CheckMul(bottom(), size.height()).AssignIfValid(&b)); - - return til::rectangle{ l, t, r, b }; - } - - rectangle& operator*=(const size& size) - { - *this = *this * size; - return *this; - } #pragma endregion constexpr ptrdiff_t top() const noexcept diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 55aa2bbf4c2..8c0420e5224 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -411,20 +411,6 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // Scroll the current offset and invalidate the revealed area _invalidMap.translate(til::point(*pcoordDelta), true); - // TODO: MIGRIE DO WE NEED THIS IF THE TRANSLATION ABOVE FILLS THE VACATED AREA? - // Add the top/bottom of the window to the invalid area - SMALL_RECT invalid = _lastViewport.ToOrigin().ToExclusive(); - - if (dy > 0) - { - invalid.Bottom = dy; - } - else if (dy < 0) - { - invalid.Top = invalid.Bottom + dy; - } - _invalidMap.set(til::rectangle(Viewport::FromExclusive(invalid).ToInclusive())); - COORD invalidScrollNew; RETURN_IF_FAILED(ShortAdd(_scrollDelta.X, dx, &invalidScrollNew.X)); RETURN_IF_FAILED(ShortAdd(_scrollDelta.Y, dy, &invalidScrollNew.Y)); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 59fbf048ccc..82aa5a69a75 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -797,26 +797,6 @@ class RectangleTests } } - TEST_METHOD(MultiplicationSize) - { - const til::rectangle start{ 10, 20, 30, 40 }; - const til::size size{ 3, 7 }; - const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; - const auto actual = start * size; - - VERIFY_ARE_EQUAL(expected, actual); - } - - TEST_METHOD(MultiplicationSizeInplace) - { - til::rectangle start{ 10, 20, 30, 40 }; - const til::size size{ 3, 7 }; - const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; - start *= size; - - VERIFY_ARE_EQUAL(expected, start); - } - TEST_METHOD(Top) { const til::rectangle rc{ 5, 10, 15, 20 }; diff --git a/src/til/ut_til/sources b/src/til/ut_til/sources index 2e7861cdfb5..48a90247708 100644 --- a/src/til/ut_til/sources +++ b/src/til/ut_til/sources @@ -14,8 +14,14 @@ DLLDEF = SOURCES = \ $(SOURCES) \ + BitmapTests.cpp \ ColorTests.cpp \ + OperatorTests.cpp \ + PointTests.cpp \ + RectangleTests.cpp \ + SizeTests.cpp \ SomeTests.cpp \ + u8u16convertTests.cpp \ DefaultResource.rc \ INCLUDES = \ From 7a5a3d378432553ab3f2e16c6d76dce4dfcdab36 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 20 Mar 2020 16:11:58 -0700 Subject: [PATCH 24/25] tabs to spaces --- src/til/ut_til/sources | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/til/ut_til/sources b/src/til/ut_til/sources index 48a90247708..af602729cd6 100644 --- a/src/til/ut_til/sources +++ b/src/til/ut_til/sources @@ -14,14 +14,14 @@ DLLDEF = SOURCES = \ $(SOURCES) \ - BitmapTests.cpp \ + BitmapTests.cpp \ ColorTests.cpp \ - OperatorTests.cpp \ - PointTests.cpp \ - RectangleTests.cpp \ - SizeTests.cpp \ + OperatorTests.cpp \ + PointTests.cpp \ + RectangleTests.cpp \ + SizeTests.cpp \ SomeTests.cpp \ - u8u16convertTests.cpp \ + u8u16convertTests.cpp \ DefaultResource.rc \ INCLUDES = \ From 91c11d9ef73b79ee960a05ab7a2df45a7f5f3f1b Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 20 Mar 2020 16:16:01 -0700 Subject: [PATCH 25/25] fix the til bitmap and tests for removing the optional... I swear I compiled this before the last push and it didn't fail. --- src/inc/til/bitmap.h | 2 +- src/til/ut_til/BitmapTests.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 6cda4d31512..2070ee7a643 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -149,7 +149,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(sz.area(), fill), - _dirty(fill ? sz : til::rectangle{ sz }), + _dirty(fill ? sz : til::rectangle{}), _runs{} { } diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 6b494e4a9e0..34005e259f0 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -27,7 +27,7 @@ class BitmapTests // Union up all the dirty rectangles into one big one. if (bitsOn.empty()) { - VERIFY_ARE_EQUAL(std::nullopt, map._dirty); + VERIFY_ARE_EQUAL(til::rectangle{}, map._dirty); } else { @@ -71,7 +71,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -86,7 +86,7 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -114,7 +114,7 @@ class BitmapTests // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. VERIFY_IS_TRUE(bitmap._bits.cend() == std::find(bitmap._bits.cbegin(), bitmap._bits.cend(), true)); - VERIFY_ARE_EQUAL(std::nullopt, bitmap._dirty); + VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); } else {