Suggestions pop up of the Japanese MS IME got pushed down after multiple commands #13769

rabianr · 2022-08-18T04:30:04Z

Windows Terminal version

1.15.2282.0

Windows build number

10.0.19044.1889

Other Software

No response

Steps to reproduce

Entering multiple commands or just press Enter multiple times.
Japanese MS IME's suggestions pop up got pushed down gradually.

Expected Behavior

this doesn't happen on v1.13.10983.0

what's being typed is shown in the input area

Actual Behavior

This happens on v1.14.1432.0 and above.

v1.15.2282.0

v1.14.1432.0

also, what's being typed isn't shown in the input area

zadjii-msft · 2022-08-18T11:13:16Z

oh no, we must have lost a DIP<->Pixel conversion somewhere in #13677 or #13678.

DHowett · 2022-08-18T19:11:36Z

We may also (or instead) have mixed up a viewport -> buffer conversion.

DHowett · 2022-08-18T23:24:37Z

I'm bisecting. :)

git bisect start v1.14.1432.0 29e97b618 src/cascadia
Bisecting: 111 revisions left to test after this (roughly 7 steps)

DHowett · 2022-08-18T23:46:12Z

I may not be able to finish bisecting just yet. Here's the current bisect log:

# bad: [39c71d642179403d52b04a03de7014820ab85657] Debounce window state changes caused by the PTY (#13147)
# good: [29e97b61823297b414c41fc70725569a2acf32cf] Update readme to account for minversion bump (#12332)
git bisect start 'v1.14.1432.0' '29e97b618' 'src/cascadia'
# good: [73f02024d29cf558e42633f93e707631c306bf6f] AtlasEngine: Fix grayscale blending shader (#12734)
git bisect good 73f02024d29cf558e42633f93e707631c306bf6f
# good: [30383ccc2210e3e7d62323421748c6eedf4b5fdd] Revert "Add support for rich code navigation in GitHub (#12855)" (#12909)
git bisect good 30383ccc2210e3e7d62323421748c6eedf4b5fdd

DHowett · 2022-08-19T00:25:43Z

This is weird, but bisect pointed to f4e0d9f (#13024)

It did remove a copy of Terminal::GetCursorPosition that returned the position adjusted by the viewport offset from TerminalApi.cpp.

We may need to introduce something like that again -- or propagate knowledge of the viewport offset up to ControlCore?

(/cc @j4james)

DHowett · 2022-08-19T00:27:29Z

Oh my god, I think we were just getting unimaginably lucky the whole time?

src\cascadia\TerminalCore\TerminalApi.cpp
53:til::point Terminal::GetCursorPosition()

src\cascadia\TerminalCore\terminalrenderdata.cpp
41:COORD Terminal::GetCursorPosition() const noexcept

There were two, and one of them was pulled when we were const-qualified.

James' PR removed the first one.

They had different return values...!

j4james · 2022-08-19T00:49:19Z

Sorry about that. I'll try and have a look on the weekend if someone else hasn't fixed it by then.

zadjii-msft · 2022-08-19T11:07:44Z

In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore. This has been widely regarded as a good move. Now, you might rightly be wondering: why didn't compilation immediately fail? Well. It turns out that there were _two_ copies of `GetCursorPosition`. One for `const Terminal`, and one for `Terminal`. This is important. `Terminal::GetCursorPosition()` returned the cursor position relative to the viewport. `Terminal::GetCursorPosition() const`, however, returns the cursor position in absolute. We removed the non-`const` one. Fortunately, thanks to the lookup rules for `const`-qualified members, this didn't matter. Code that called `GetCursorPosition()` still called `GetCursorPosition()`, and everything was fine. Except that part about the relative coordinates. That was not fine. The TSF control is the _only_ consumer of `ControlCore.CursorPosition`, and that was the _only_ consumer of relative-`GetCursorPosition()`. This commit restores equilibrium by introducing a new `GetViewportRelativeCursorPosition()` member to `Terminal` and switching over the only consumer of relative cursor position to use it. Closes #13769.

DHowett · 2022-08-19T22:16:26Z

@j4james no worries, I've submitted PR #13785 for it.

In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore. This has been widely regarded as a good move. Now, you might rightly be wondering: why didn't compilation immediately fail? Well. It turns out that there were _two_ copies of `GetCursorPosition`. One for `const Terminal`, and one for `Terminal`. This is important. `Terminal::GetCursorPosition()` returned the cursor position relative to the viewport. `Terminal::GetCursorPosition() const`, however, returns the cursor position in absolute. We removed the non-`const` one. Fortunately, thanks to the lookup rules for `const`-qualified members, this didn't matter. Code that called `GetCursorPosition()` still called `GetCursorPosition()`, and everything was fine. Except that part about the relative coordinates. That was not fine. The TSF control is the _only_ consumer of `ControlCore.CursorPosition`, and that was the _only_ consumer of relative-`GetCursorPosition()`. This commit restores equilibrium by introducing a new `GetViewportRelativeCursorPosition()` member to `Terminal` and switching over the only consumer of relative cursor position to use it. Closes #13769.

In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore. This has been widely regarded as a good move. Now, you might rightly be wondering: why didn't compilation immediately fail? Well. It turns out that there were _two_ copies of `GetCursorPosition`. One for `const Terminal`, and one for `Terminal`. This is important. `Terminal::GetCursorPosition()` returned the cursor position relative to the viewport. `Terminal::GetCursorPosition() const`, however, returns the cursor position in absolute. We removed the non-`const` one. Fortunately, thanks to the lookup rules for `const`-qualified members, this didn't matter. Code that called `GetCursorPosition()` still called `GetCursorPosition()`, and everything was fine. Except that part about the relative coordinates. That was not fine. The TSF control is the _only_ consumer of `ControlCore.CursorPosition`, and that was the _only_ consumer of relative-`GetCursorPosition()`. This commit restores equilibrium by introducing a new `GetViewportRelativeCursorPosition()` member to `Terminal` and switching over the only consumer of relative cursor position to use it. Closes #13769. Backport of #13785.

In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore. This has been widely regarded as a good move. Now, you might rightly be wondering: why didn't compilation immediately fail? Well. It turns out that there were _two_ copies of `GetCursorPosition`. One for `const Terminal`, and one for `Terminal`. This is important. `Terminal::GetCursorPosition()` returned the cursor position relative to the viewport. `Terminal::GetCursorPosition() const`, however, returns the cursor position in absolute. We removed the non-`const` one. Fortunately, thanks to the lookup rules for `const`-qualified members, this didn't matter. Code that called `GetCursorPosition()` still called `GetCursorPosition()`, and everything was fine. Except that part about the relative coordinates. That was not fine. The TSF control is the _only_ consumer of `ControlCore.CursorPosition`, and that was the _only_ consumer of relative-`GetCursorPosition()`. This commit restores equilibrium by introducing a new `GetViewportRelativeCursorPosition()` member to `Terminal` and switching over the only consumer of relative cursor position to use it. Closes #13769.

In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore. This has been widely regarded as a good move. Now, you might rightly be wondering: why didn't compilation immediately fail? Well. It turns out that there were _two_ copies of `GetCursorPosition`. One for `const Terminal`, and one for `Terminal`. This is important. `Terminal::GetCursorPosition()` returned the cursor position relative to the viewport. `Terminal::GetCursorPosition() const`, however, returns the cursor position in absolute. We removed the non-`const` one. Fortunately, thanks to the lookup rules for `const`-qualified members, this didn't matter. Code that called `GetCursorPosition()` still called `GetCursorPosition()`, and everything was fine. Except that part about the relative coordinates. That was not fine. The TSF control is the _only_ consumer of `ControlCore.CursorPosition`, and that was the _only_ consumer of relative-`GetCursorPosition()`. This commit restores equilibrium by introducing a new `GetViewportRelativeCursorPosition()` member to `Terminal` and switching over the only consumer of relative cursor position to use it. Closes #13769. (cherry picked from commit 2dedc9a) Service-Card-Id: 85131461 Service-Version: 1.15

ghost · 2022-09-13T17:02:32Z

🎉This issue was addressed in #13785, which has now been successfully released as Windows Terminal v1.15.252.:tada:

Handy links:

ghost · 2022-09-13T17:04:35Z

🎉This issue was addressed in #13785, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

rabianr added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Aug 18, 2022

ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 18, 2022

zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. Priority-1 A description (P1) labels Aug 18, 2022

ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 18, 2022

zadjii-msft added this to the Terminal v1.16 milestone Aug 18, 2022

DHowett self-assigned this Aug 18, 2022

zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 19, 2022

DHowett mentioned this issue Aug 19, 2022

Use the viewport-relative cursor pos for CCore.CursorPosition #13785

Merged

ghost added the In-PR This issue has a related PR label Aug 19, 2022

DHowett mentioned this issue Aug 19, 2022

[1.14] Use the viewport-rel. cursor pos for CursorPosition #13786

Merged

DHowett closed this as completed in #13785 Aug 22, 2022

ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Aug 22, 2022

j4james mentioned this issue Aug 23, 2022

korean input buffer is not shown (buffer position is wierd) after some commands #13820

Closed

j4james mentioned this issue Sep 3, 2022

When the input line reaches the bottom of the window, the character string of the IME conversion candidate in the input line is not displayed in the input line. #13918

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggestions pop up of the Japanese MS IME got pushed down after multiple commands #13769

Suggestions pop up of the Japanese MS IME got pushed down after multiple commands #13769

rabianr commented Aug 18, 2022 •

edited

Loading

zadjii-msft commented Aug 18, 2022

DHowett commented Aug 18, 2022

DHowett commented Aug 18, 2022 •

edited

Loading

DHowett commented Aug 18, 2022

DHowett commented Aug 19, 2022

DHowett commented Aug 19, 2022 •

edited

Loading

j4james commented Aug 19, 2022

zadjii-msft commented Aug 19, 2022

DHowett commented Aug 19, 2022

ghost commented Sep 13, 2022

ghost commented Sep 13, 2022

Suggestions pop up of the Japanese MS IME got pushed down after multiple commands #13769

Suggestions pop up of the Japanese MS IME got pushed down after multiple commands #13769

Comments

rabianr commented Aug 18, 2022 • edited Loading

Windows Terminal version

Windows build number

Other Software

Steps to reproduce

Expected Behavior

Actual Behavior

zadjii-msft commented Aug 18, 2022

DHowett commented Aug 18, 2022

DHowett commented Aug 18, 2022 • edited Loading

DHowett commented Aug 18, 2022

DHowett commented Aug 19, 2022

DHowett commented Aug 19, 2022 • edited Loading

j4james commented Aug 19, 2022

zadjii-msft commented Aug 19, 2022

DHowett commented Aug 19, 2022

ghost commented Sep 13, 2022

ghost commented Sep 13, 2022

rabianr commented Aug 18, 2022 •

edited

Loading

DHowett commented Aug 18, 2022 •

edited

Loading

DHowett commented Aug 19, 2022 •

edited

Loading