Skip to content

Commit

Permalink
improve tab dragging, more robust to tab count changing while dragging (
Browse files Browse the repository at this point in the history
fixes #4520)
  • Loading branch information
kjk committed Sep 10, 2024
1 parent a412fb0 commit b23df72
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 114 deletions.
1 change: 1 addition & 0 deletions src/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ Cmd* enum (e.g. CmdOpen) and a human-readable name (not used yet).
V(CmdDebugStartStressTest, "Debug: Start Stress Test") \
V(CmdDebugTogglePredictiveRender, "Debug: Toggle Predictive Rendering") \
V(CmdDebugToggleRtl, "Debug: Toggle Rtl") \
V(CmdDebugDelayCloseWindow, "Debug: Delay Close Window") \
V(CmdNone, "Do nothing")

// order of CreateAnnot* must be the same as enum AnnotationType
Expand Down
15 changes: 15 additions & 0 deletions src/SumatraPDF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5097,6 +5097,16 @@ static void SetAnnotCreateArgs(AnnotCreateArgs& args, CustomCommand* cmd) {
}
}

static void CloseWindowDelayedAsync(MainWindow* win) {
if (IsGUIThread(FALSE)) {
CloseWindow(win, false, false);
return;
}
Sleep(4 * 1000);
auto fn = MkFunc0(CloseWindowDelayedAsync, win);
uitask::Post(fn, nullptr);
}

static LRESULT FrameOnCommand(MainWindow* win, HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
int cmdId = LOWORD(wp);

Expand Down Expand Up @@ -5318,6 +5328,11 @@ static LRESULT FrameOnCommand(MainWindow* win, HWND hwnd, UINT msg, WPARAM wp, L
}
} break;

case CmdDebugDelayCloseWindow: {
auto fn = MkFunc0(CloseWindowDelayedAsync, win);
RunAsync(fn, "DelayedCloseWindow");
} break;

case CmdExit:
OnMenuExit();
break;
Expand Down
158 changes: 44 additions & 114 deletions src/wingui/WinGui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

Kind kindWnd = "wnd";

constexpr bool gLogTabs = false;

#define WIN_MESSAGES(V) \
V(WM_CREATE) \
V(WM_DESTROY) \
Expand Down Expand Up @@ -3296,6 +3294,26 @@ TabsCtrl::TabsCtrl() {
kind = kindTabs;
}

// must be called after LayoutTabs()
static void TabsCtrlUpdateAfterChangingTabsCount(TabsCtrl* tabs) {
HWND hwnd = tabs->hwnd;
if (GetCapture() == hwnd) {
ReleaseCapture();
}
tabs->tabBeingClosed = -1;
Point mousePos = HwndGetCursorPos(hwnd);
auto tabState = tabs->TabStateFromMousePosition(mousePos);
bool canClose = tabState.tabInfo && tabState.tabInfo->canClose;
bool overClose = tabState.overClose && canClose;
int tabUnderMouse = tabState.tabIdx;
tabs->tabHighlighted = tabUnderMouse;
tabs->tabHighlightedClose = overClose ? tabUnderMouse : -1;
if (tabs->draggingTab) {
tabs->draggingTab = false;
ImageList_EndDrag();
}
}

TabsCtrl::~TabsCtrl() {
}

Expand Down Expand Up @@ -3371,6 +3389,7 @@ static void UpdateAfterDrag(TabsCtrl* tabsCtrl, int tab1, int tab2) {
}
tabsCtrl->SetSelected(newSelected);
tabsCtrl->LayoutTabs();
TabsCtrlUpdateAfterChangingTabsCount(tabsCtrl);
}

LRESULT TabsCtrl::OnNotifyReflect(WPARAM wp, LPARAM lp) {
Expand All @@ -3385,27 +3404,17 @@ LRESULT TabsCtrl::OnNotifyReflect(WPARAM wp, LPARAM lp) {

case TTN_GETDISPINFOA:
case TTN_GETDISPINFOW:
if (gLogTabs) {
logfa("TabsCtrl::OnNotifyReflect: TTN_GETDISPINFO\n");
}
break;
}
return 0;
}

// used to do less logging of WM_MOUSEMOVE
static int nWmMouseMoveCount = 0;

LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
// TCITEMW* tcs = nullptr;

Point mousePos = {GET_X_LPARAM(lp), GET_Y_LPARAM(lp)};
if (WM_MOUSELEAVE == msg) {
POINT p;
GetCursorPos(&p);
ScreenToClient(hwnd, &p);
mousePos.x = p.x;
mousePos.y = p.y;
mousePos = HwndGetCursorPos(hwnd);
}

TabsCtrl::MouseState tabState;
Expand Down Expand Up @@ -3456,28 +3465,15 @@ LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
break;

case WM_MOUSELEAVE:
if (gLogTabs) {
logfa("TabsCtrl::WndProc: WM_MOUSELEAVE, tabUnderMouse: %d, tabHighlited: %d\n", tabUnderMouse,
tabHighlighted);
}
if (tabHighlighted != tabUnderMouse) {
tabHighlighted = tabUnderMouse;
logf("tab: WM_MOUSELEAVE: tabHighlighted = tabUnderMouse: %d\n", tabHighlighted);
HwndScheduleRepaint(hwnd);
}
nWmMouseMoveCount = 0;
break;

case WM_MOUSEMOVE: {
TrackMouseLeave(hwnd);
bool isDragging = (GetCapture() == hwnd);
if (nWmMouseMoveCount == 0 || isDragging) {
if (gLogTabs) {
logfa("TabsCtrl::WndProc: WM_MOUSEMOVE: tabUnderMouse: %d, tabHighlited: %d, isDragging: %d\n",
tabUnderMouse, tabHighlighted, (int)isDragging);
}
}
nWmMouseMoveCount++;
int hl = tabHighlighted;
if (isDragging && tabUnderMouse == -1) {
// move the tab out: draw it as a image and drag around the screen
Expand All @@ -3503,16 +3499,7 @@ LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
if (isDragging && hl != -1) {
// send notification if the highlighted tab is dragged over another
if (!GetTab(tabUnderMouse)->isPinned) {
if (gLogTabs) {
logfa(
"TabsCtrl::WndProc: WM_MOUSEMOVE: before TriggerTabDragged: hl=%d, tabUnderMouse=%d\n",
hl, tabUnderMouse);
}
TriggerTabDragged(this, hl, tabUnderMouse);
if (gLogTabs) {
logfa("TabsCtrl::WndProc: WM_MOUSEMOVE: before UpdateAfterDrag: hl=%d, tabUnderMouse=%d\n",
hl, tabUnderMouse);
}
UpdateAfterDrag(this, hl, tabUnderMouse);
}
} else {
Expand All @@ -3528,16 +3515,14 @@ LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
// logfa("inX=%d, hl=%d, xHl=%d, xHighlighted=%d\n", (int)inX, hl, xHl, tab->xHighlighted);
if (tabHighlightedClose != xHl) {
// logfa("before invalidate, xHl=%d, xHighlited=%d\n", xHl, tab->xHighlighted);
HwndScheduleRepaint(hwnd);
tabHighlightedClose = xHl;
HwndScheduleRepaint(hwnd);
}
return 0;
}

case WM_LBUTTONDOWN: {
nWmMouseMoveCount = 0;
tabHighlighted = tabUnderMouse;
logf("tab: WM_LBUTTONDOWN: tabHighlighted = tabUnderMouse: %d\n", tabHighlighted);
if (overClose) {
HwndScheduleRepaint(hwnd);
tabBeingClosed = tabUnderMouse;
Expand All @@ -3558,23 +3543,10 @@ LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
SetCapture(hwnd);
}
}
if (gLogTabs || (tabHighlighted == -1)) {
logfa(
"TabsCtrl::WndProc: WM_LBUTTONDOWN, tabUnderMouse: %d, tabHighlited: %d, tabBeingClosed: %d, "
"overClose: %d\n",
tabUnderMouse, tabHighlighted, tabBeingClosed, (int)overClose);
}
return 0;
}

case WM_LBUTTONUP: {
nWmMouseMoveCount = 0;
if (gLogTabs) {
logfa(
"TabsCtrl::WndProc: WM_LBUTTONUP, tabUnderMouse: %d, tabHighlited: %d, tabBeingClosed: %d, "
"overClose: %d\n",
tabUnderMouse, tabHighlighted, tabBeingClosed, (int)overClose);
}
if (tabBeingClosed != -1 && tabUnderMouse == tabBeingClosed && overClose) {
// send notification that the tab is closed
TriggerTabClosed(this, tabBeingClosed);
Expand All @@ -3588,41 +3560,31 @@ LRESULT TabsCtrl::WndProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) {
// we don't always get WM_MOUSEMOVE before WM_LBUTTONUP so
// update tabHighlighted
tabHighlighted = tabUnderMouse;
logf("tab: WM_LBUTTONUP: tabHighlighted = tabUnderMouse: %d\n", tabHighlighted);

if (draggingTab) {
draggingTab = false;
ImageList_EndDrag();
int selectedTab = GetSelected();
if (gLogTabs) {
logfa("TabsCtrl::WndProc: WM_LBUTTONUP, selectedTab: %d tabUnderMouse: %d\n", selectedTab,
tabUnderMouse);
}
if (tabUnderMouse < 0) {
// migrate to new/different window
POINT p(mousePos.x, mousePos.y);
ClientToScreen(hwnd, &p);
Point scPoint(p.x, p.y);
TriggerTabMigration(this, selectedTab, scPoint);
return 0;
}
if (tabUnderMouse != selectedTab && !GetTab(tabUnderMouse)->isPinned) {
TriggerTabDragged(this, selectedTab, tabUnderMouse);
UpdateAfterDrag(this, selectedTab, tabUnderMouse);
}
if (!draggingTab) {
return 0;
}
draggingTab = false;
ImageList_EndDrag();
int selectedTab = GetSelected();
if (tabUnderMouse < 0) {
// migrate to new/different window
POINT p(mousePos.x, mousePos.y);
ClientToScreen(hwnd, &p);
Point scPoint(p.x, p.y);
TriggerTabMigration(this, selectedTab, scPoint);
return 0;
}
if (tabUnderMouse != selectedTab && !GetTab(tabUnderMouse)->isPinned) {
TriggerTabDragged(this, selectedTab, tabUnderMouse);
UpdateAfterDrag(this, selectedTab, tabUnderMouse);
}
return 0;
}

case WM_MBUTTONDOWN: {
// middle-clicking unconditionally closes the tab
if (gLogTabs) {
logfa(
"TabsCtrl::WndProc: WM_MBUTTONDOWN, tabUnderMouse: %d, tabHighlited: %d, tabBeingClosed: %d, "
"overClose: %d\n",
tabUnderMouse, tabHighlighted, tabBeingClosed, (int)overClose);
}
nWmMouseMoveCount = 0;

tabBeingClosed = tabUnderMouse;
if (tabBeingClosed < 0 || !canClose) {
return 0;
Expand Down Expand Up @@ -3726,38 +3688,9 @@ int TabsCtrl::InsertTab(int idx, TabInfo* tab) {
return res;
}
tabs.InsertAt(idx, tab);

#if 0 // WTF was I trying to do here?
if (idx == 0) {
SetSelected(0);
} else {
int selectedTab = GetSelected();
if (idx <= selectedTab) {
SetSelected(selectedTab + 1);
}
}
#else
SetSelected(idx);
#endif
#if 0
tabBeingClosed = -1;
// TODO: this is probabably incorrect
tabHighlighted = -1;
#else
if (tabBeingClosed != -1) {
if (idx <= tabBeingClosed) {
tabBeingClosed++;
}
}
if (tabHighlighted != -1) {
if (idx <= tabHighlighted) {
logf("tab: TabsCtrl::InsertTab %d: tabHighlighted: %d (was %d)\n", idx, tabHighlighted + 1, tabHighlighted);
tabHighlighted++;
}
}
#endif
tabHighlightedClose = -1;
LayoutTabs();
TabsCtrlUpdateAfterChangingTabsCount(this);
return idx;
}

Expand All @@ -3778,14 +3711,14 @@ UINT_PTR TabsCtrl::RemoveTab(int idx) {
UINT_PTR userData = tab->userData;
tabs.RemoveAt(idx);
delete tab;
tabBeingClosed = -1;
int selectedTab = GetSelected();
if (idx < selectedTab) {
SetSelected(selectedTab - 1);
} else if (idx == selectedTab) {
SetSelected(0);
}
LayoutTabs();
TabsCtrlUpdateAfterChangingTabsCount(this);
return userData;
}

Expand All @@ -3799,13 +3732,10 @@ void TabsCtrl::SwapTabs(int idx1, int idx2) {
// Note: the caller should take care of deleting userData
void TabsCtrl::RemoveAllTabs() {
TabCtrl_DeleteAllItems(hwnd);
tabHighlighted = -1;
tabBeingClosed = -1;
tabHighlightedClose = -1;
DeleteVecMembers(tabs);
tabs.Reset();
LayoutTabs();
logf("tab: TabsCtrl::RemoveAllTabs: tabHighlighted: %d\n", -1);
TabsCtrlUpdateAfterChangingTabsCount(this);
}

TabInfo* TabsCtrl::GetTab(int idx) {
Expand Down

0 comments on commit b23df72

Please sign in to comment.