Skip to content

Commit

Permalink
Replace til::some with til::small_vector (microsoft#16952)
Browse files Browse the repository at this point in the history
`til::small_vector` had a bug: Its internal backing buffer didn't
prevent default initialization! Wrapping it in an `union` fixed that.

`til::some` had the same issue, but thinking about it I realized we
don't need both classes to exist, so I removed `til::some` since
`til::small_vector` is more flexible.

Checking the assembly, I noticed that `til::small_vector` with the
`union` fix produced a more compact result. I also noticed that in
combination with function calls and inlining the bit-wise ANDs in
the point/size/rect boolean operators produced poor-ish results.
Since their impact on performance is negligible to begin with I
simplified that code slightly.

Finally, I noticed that the boolean operator for `til::point`
was incorrect since it checked for `>0` instead of `>=0`.
Luckily nothing seemed to have used that operator yet.
(= No inbox regression.)
  • Loading branch information
lhecker authored Mar 29, 2024
1 parent 75dea24 commit 3cc82a5
Show file tree
Hide file tree
Showing 21 changed files with 77 additions and 707 deletions.
2 changes: 1 addition & 1 deletion src/inc/til/point.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

constexpr explicit operator bool() const noexcept
{
return (x > 0) & (y > 0);
return x >= 0 && y >= 0;
}

constexpr bool operator<(const point other) const noexcept
Expand Down
18 changes: 7 additions & 11 deletions src/inc/til/rect.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
#pragma once

#include "bit.h"
#include "some.h"
#include "math.h"
#include "size.h"
#include "point.h"
#include "operators.h"
#include "small_vector.h"

namespace til // Terminal Implementation Library. Also: "Today I Learned"
{
Expand All @@ -31,8 +31,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

explicit constexpr operator bool() const noexcept
{
return (left >= 0) & (top >= 0) &
(right >= left) & (bottom >= top);
return left >= 0 && top >= 0 && right >= left && bottom >= top;
}
};

Expand Down Expand Up @@ -204,8 +203,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

explicit constexpr operator bool() const noexcept
{
return (left >= 0) & (top >= 0) &
(right > left) & (bottom > top);
return left >= 0 && top >= 0 && right > left && bottom > top;
}

constexpr const_iterator begin() const
Expand Down Expand Up @@ -294,9 +292,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
}

// - = subtract
constexpr some<rect, 4> operator-(const rect& other) const
small_vector<rect, 4> operator-(const rect& other) const
{
some<rect, 4> result;
small_vector<rect, 4> result;

// We could have up to four rectangles describing the area resulting when you take removeMe out of main.
// Find the intersection of the two so we know which bits of removeMe are actually applicable
Expand Down Expand Up @@ -566,14 +564,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

constexpr bool contains(point pt) const noexcept
{
return (pt.x >= left) & (pt.x < right) &
(pt.y >= top) & (pt.y < bottom);
return pt.x >= left && pt.x < right && pt.y >= top && pt.y < bottom;
}

constexpr bool contains(const rect& rc) const noexcept
{
return (rc.left >= left) & (rc.top >= top) &
(rc.right <= right) & (rc.bottom <= bottom);
return rc.left >= left && rc.top >= top && rc.right <= right && rc.bottom <= bottom;
}

template<typename T = CoordType>
Expand Down
2 changes: 0 additions & 2 deletions src/inc/til/rle.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

#pragma once

#include "small_vector.h"

#ifdef UNIT_TESTING
class RunLengthEncodingTests;
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/inc/til/size.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

constexpr explicit operator bool() const noexcept
{
return (width > 0) & (height > 0);
return width > 0 && height > 0;
}

constexpr size operator+(const size other) const
Expand Down
7 changes: 5 additions & 2 deletions src/inc/til/small_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ namespace til
reference emplace_back(Args&&... args)
{
const auto new_size = _ensure_fits(1);
const auto it = new (_data + _size) T(std::forward<Args>(args)...);
const auto it = std::construct_at(_data + _size, std::forward<Args>(args)...);
_size = new_size;
return *it;
}
Expand Down Expand Up @@ -930,7 +930,10 @@ namespace til
T* _data;
size_t _capacity;
size_t _size;
T _buffer[N];
union
{
T _buffer[N];
};
};
}

Expand Down
267 changes: 0 additions & 267 deletions src/inc/til/some.h

This file was deleted.

1 change: 0 additions & 1 deletion src/renderer/atlas/BackendD3D.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include <stb_rect_pack.h>
#include <til/flat_set.h>
#include <til/small_vector.h>

#include "Backend.h"

Expand Down
2 changes: 0 additions & 2 deletions src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#include "precomp.h"
#include "gdirenderer.hpp"

#include <til/small_vector.h>

#include "../inc/unicode.hpp"

#pragma hdrstop
Expand Down
Loading

0 comments on commit 3cc82a5

Please sign in to comment.