Are we sure this gets 0 initialized always? I know we used to have some weird OS-side bugs where memory wouldn't get 0 initialized. (apologies for needing a refresher on basic language concepts)

Yes, with _cPosition a til::point now, it doesn't accept a single constructor parameter anymore - it needs either 0 (_cPosition{}) or 2 (_cPosition{ 0, 0 }). But if you write the former, you can drop it from the constructor list entirely, since it's already constructed like that by default anyways.

To expand on that:
The old COORD _cPosition needed a manual initialization unlike til::point, because that type has no custom constructor which initializes X/Y to 0, nor does it use C++11's default member initializers (short X = 0;) like til::point does, so the default value for its members is "uninitialized" (which is UB).

The reason the existing code said _cPosition{ 0 } and not _cPosition{} is probably a carry-over from the C code, because C structs don't have default constructors - _cPosition{} is not valid C.
In C, if you pass less arguments than the struct has members, then the remaining fields will be set to 0. RECT{ 0 } is equivalent to RECT{ 0, 0, 0, 0 } and RECT{ 1 } is equivalent to RECT{ 1, 0, 0, 0 }. In C++, the compiler-generated constructor acts equivalent to C's with the addition of the default constructor {} which acts equivalent to { 0 }.

Since til::point has a custom constructor it gets no compiler-generated one and thus it can't be used with C-style constructor arguments anymore ({ 0 }).

til::point

This lost the & - intentional?

Yeah. Technically it's beneficial to pass primitive structs like that by value at all times, because x64 calling conventions / ABIs pass them via registers. Any x64 ABI will pass the 8 byte large til::point within a single register (RCX on Windows, RDI everywhere else) which means that it's not just super fast, but also that no stack needs to be allocated, etc. This might not matter for functions that are only called from time to time, but some functions we got are called thousands of times per second. This is why I pass til::point and til::size by value throughout the project - it's just consistently very very performant.

Now the problem is that the Windows x64 ABI has a very unfortunate, very important flaw as compared to the SYSV ABI used by every single other x64 OS in the world and it makes me very very sad every single time I'm reminded of this. 🥲
...It can't pass structs larger than 8 byte via registers. With the SYSV ABI a 16 byte struct simply gets passed in the first two 8 byte large registers (RDI/RSI), but on Windows such arguments are passed as "hidden references/pointers" - the argument is still passed via a register (RCX), but the register now contains a pointer to the struct on the caller's stack. The callee will then copy it on their stack to create a by-value copy (via SSE's movups/movaps). Sometimes the optimizer can optimize the copy away, but usually it'll fail at doing so. The difference is a 4-5 fold increase in runtime cost for function calls, just by merely having a single by-value 16-byte struct as an argument.
This is why I pass til::rect and til::inclusive_rect consistently by-reference, because it prevents the VS 2022 (v143) compiler from creating such redundant, very costly stack copies on the callee side.
(I've been cooperating with DevDiv/Intel on optimizing these redundant stack copies away in the most common cases and it should hopefully land in a future VS 2022 update soonish. It reduces the overhead from 4-5x down to around 2x which is then about as good as passing it by-reference.)

Wow that's incredibly thorough. We should have this saved somewhere (niksa.md maybe) as a reference that til::point&size should always be by value and rect always by ref.

Thanks!

Yes, please do write this up as an addendum to Niksa.md

this one seems wrong, yea?

If no other issues turn up I might just leave it as it is, because I'm going to fix all those members in a follow-up anyways.

if I could admit: i think optional captures the essence of this API better than "magic -1" does

I understand where you're coming from, but this method exists only to implement ITextRangeProvider::GetText which says:

The maximum length of the string to return. Use -1 if no limit is required.

I feel like this is a direct translation of this API. Additionally I personally think that negative lengths are very common in C code (especially if cast to unsigned integers).

this isn't exactly a coordinate either...

The popup.Height() is in character cells and so I made it return a til::CoordType.
This change makes it so that the compiler doesn't complain that we should promote integers first before we add/subtract/multiply/divide them.

Curious, using the constructor via function call syntax. You replaced similar thing with point{}; should we here?

The VERIFY_ARE_EQUAL macro blows up if you use curly brackets inside it. Not kidding. IIRC the ({ 0, 0 }) doesn't work because the constructor is explicit and doesn't take a single parameter list argument (unlike structs).

This seems like a very silly way to do a = rand(); b = a;

Double check this code. I saw that INSIDE the renderer they're exclusive now, but via RenderData and OUTSIDE the renderer they're inclusive. That's fine, but look at line R1284 -- we bump up the inclusive SR to be exclusive, and then we construct a til::rect out of it making it exclusive again...

I, too, am surprised!