Repository: microsoft/terminal (language: cpp)

Issue:
"invalid bounds arguments passed to std::clamp" when resizing window to zero height
### Windows Terminal version

1.12.10733.0

### Windows build number

10.0.22000

### Other Software

_No response_

### Steps to reproduce

Launch OpenConsole.exe compiled in debug mode, drag bottom of the window to the very top.



### Expected Behavior

No assertion

### Actual Behavior

Assertion from from STL `"invalid bounds arguments passed to std::clamp"` on the line
```cpp
clampMe.Top = std::clamp(clampMe.Top, Top(), BottomInclusive());
```
in `Viewport Viewport::Clamp`
<details>
<summary>Call stack</summary>
<pre>
 	ucrtbased.dll!00007ff9248a7fbc()	Unknown
 	ucrtbased.dll!00007ff9248a7ef4()	Unknown
 	ucrtbased.dll!00007ff9248a7d90()	Unknown
>	OpenConsole.exe!std::clamp<short,std::less<void>>(const short & _Val, const short & _Min_val, const short & _Max_val, std::less<void> _Pred) Line 10188	C++
 	OpenConsole.exe!std::clamp<short>(const short & _Val, const short & _Min_val, const short & _Max_val) Line 10208	C++
 	OpenConsole.exe!Microsoft::Console::Types::Viewport::Clamp(const Microsoft::Console::Types::Viewport & other) Line 245	C++
 	OpenConsole.exe!Microsoft::Console::Render::Renderer::TriggerRedrawCursor(const _COORD * const pcoord) Line 284	C++
 	OpenConsole.exe!TextBuffer::TriggerRedrawCursor(const _COORD position) Line 992	C++
 	OpenConsole.exe!Cursor::_RedrawCursorAlways() Line 190	C++
 	OpenConsole.exe!Cursor::SetIsOn(const bool fIsOn) Line 103	C++
 	OpenConsole.exe!Microsoft::Console::CursorBlinker::TimerRoutine(SCREEN_INFORMATION & ScreenInfo) Line 153	C++
 	OpenConsole.exe!CursorTimerRoutineWrapper(_TP_CALLBACK_INSTANCE * __formal, void * __formal, _TP_TIMER * __formal) Line 24	C++
 	ntdll.dll!TppTimerpExecuteCallback()	Unknown
 	ntdll.dll!TppWorkerThread()	Unknown
 	kernel32.dll!00007ff9feee54e0()	Unknown
 	ntdll.dll!RtlUserThreadStart()	Unknown
</pre>
</details>

Looks like it happens because `SCREEN_INFORMATION::_InternalSetViewportSize` increases viewport's `Top` in `srNewViewport.Top -= DeltaY;` (`DeltaY` is negative) line without any check to prevent `Top` becoming more than `Bottom` which sadly can happen. Clamp cannot work when `Top` is more than `Bottom`


Discussion / hints:
For my own reference, which commit are you building off of? I just wanna know how far back we'll have to service this fix 😅 
I'm building recent `main` (`v1.10.1933.0-605-g41ef5554f`) and haven't tried to figure out when exactly it appeared unfortunately.
uh, https://github.com/microsoft/terminal/releases/tag/v1.10.1933.0 is like, 10 months old... If this goes that far back, well, we're in some serious trouble

edit: just guesses
* https://github.com/microsoft/terminal/blame/41ef5554f5cb946c4a48e2b8db791265c7217fce/src/renderer/base/renderer.cpp#L266 - #8664 added the Clamp call to `Renderer::TriggerRedrawCursor`. There wasn't one there before that.

The more I think about, the more I think that call to `view.Clamp` is fundamentally wrong anyway. The intention was to clip the cursor rect to the viewport, so you'd only be drawing the portion of the cursor that was visible, but that's not what `Viewport::Clamp` does. If the cursor is completely off screen, it's basically going to pull it back into view, forcing the renderer to redraw an area of the screen that doesn't actually need refreshing. That's probably not the end of the world, but it's not right.

For the same reason, I don't think this bounds error is the end of the world either. In a release build, the clamp is just going to fail to do anything, but that's no worse than the off screen clamping. It's just invalidating an area of the screen that doesn't need redrawing.

So while we definitely should be fixing this, I don't think you need to panic about backporting the fix, unless I'm missing something.
Oh oh oh maybe this is why #12739 

Maybe we're clamping the cursor, drawing it there, and then not invalidating it at the clamped place. Idk. Just a thought while fixing this. EDIT: I was wrong. that one still repros.

Fix patch:
diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp
index 7ceb0840b8c..2424515b991 100644
--- a/src/renderer/base/renderer.cpp
+++ b/src/renderer/base/renderer.cpp
@@ -281,9 +281,7 @@ void Renderer::TriggerRedrawCursor(const COORD* const pcoord)
         // The region is clamped within the viewport boundaries and we only
         // trigger a redraw if the region is not empty.
         auto view = _pData->GetViewport();
-        cursorView = view.Clamp(cursorView);
-
-        if (cursorView.IsValid())
+        if (view.IsInBounds(cursorView))
         {
             const auto updateRect = view.ConvertToOrigin(cursorView).ToExclusive();
             FOREACH_ENGINE(pEngine)


Test patch: