Skip to content

Commit

Permalink
[81_11] Fix the bug causing crashes when dragging tabs. (#2147)
Browse files Browse the repository at this point in the history
<!-- Thank you for your contribution! -->
## How to test your changes?
Just drag it. I believe it will be fine this time.
  • Loading branch information
Minzihao authored Oct 10, 2024
1 parent 9fea7fd commit bc4803e
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 66 deletions.
111 changes: 50 additions & 61 deletions src/Plugins/Qt/QTMTabPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
*/
url g_mostRecentlyClosedTab;

url g_mostRecentlyDraggedTab;

/******************************************************************************
* QTMTabPage
******************************************************************************/
Expand Down Expand Up @@ -82,35 +84,36 @@ QTMTabPage::resizeEvent (QResizeEvent* e) {
m_closeBtn->setGeometry (x, y, w, h);
}

// void
// QTMTabPage::mousePressEvent (QMouseEvent* e) {
// if (e->button () == Qt::LeftButton) {
// m_dragStartPos= e->pos ();
// }
// QToolButton::mousePressEvent (e);
// }
//
// void
// QTMTabPage::mouseMoveEvent (QMouseEvent* e) {
// if (!(e->buttons () & Qt::LeftButton)) return QToolButton::mouseMoveEvent
// (e); if ((e->pos () - m_dragStartPos).manhattanLength () < 15) {
// // avoid treating small movement(more like a click) as dragging
// return QToolButton::mouseMoveEvent (e);
// }
// e->accept ();
//
// QPixmap pixmap (size ());
// render (&pixmap);
//
// QDrag* drag= new QDrag (this);
// // align pixmap to the topLeft of the cursor
// drag->setHotSpot (QPoint (0, 0));
// drag->setMimeData (new QMimeData ()); // Qt requires
// drag->setPixmap (pixmap);
// drag->exec (Qt::MoveAction);
//
// setDown (false); // to avoid keeping the pressed state
// }
void
QTMTabPage::mousePressEvent (QMouseEvent* e) {
if (e->button () == Qt::LeftButton) {
m_dragStartPos= e->pos ();
}
QToolButton::mousePressEvent (e);
}

void
QTMTabPage::mouseMoveEvent (QMouseEvent* e) {
if (!(e->buttons () & Qt::LeftButton)) return QToolButton::mouseMoveEvent (e);
if ((e->pos () - m_dragStartPos).manhattanLength () < 15) {
// avoid treating small movement(more like a click) as dragging
return QToolButton::mouseMoveEvent (e);
}
e->accept ();

QPixmap pixmap (size ());
render (&pixmap);
setDown (false); // to avoid keeping the pressed state

g_mostRecentlyDraggedTab= m_bufferUrl;

QDrag* drag=
new QDrag (parent ()); // don't point to `this`, it will cause crash
drag->setHotSpot (QPoint (0, 0)); // align pixmap to the topLeft of the cursor
drag->setMimeData (new QMimeData ()); // Qt requires
drag->setPixmap (pixmap);
drag->exec (Qt::MoveAction);
}

void
QTMTabPage::setupStyle () {
Expand Down Expand Up @@ -167,17 +170,9 @@ QTMTabPageContainer::~QTMTabPageContainer () { removeAllTabPages (); }

void
QTMTabPageContainer::replaceTabPages (QList<QAction*>* p_src) {
/* why tryLock()? see dragEnterEvent(), but you absolutely can NOT call the
* lock() method here, which will cause DEADLOCK! Because it's locked
* in dragEnterEvent() (running on MAIN thread), so you can't call lock()
* again on MAIN thread before it's unlocked. */
if (!m_updateMutex.tryLock ()) return;

removeAllTabPages (); // remove old tabs
extractTabPages (p_src); // extract new tabs
arrangeTabPages ();

m_updateMutex.unlock ();
}

void
Expand Down Expand Up @@ -271,22 +266,22 @@ QTMTabPageContainer::mapToPointing (QDropEvent* e, QPoint& p_indicatorPos) {

void
QTMTabPageContainer::dragEnterEvent (QDragEnterEvent* e) {
QTMTabPage* source= qobject_cast<QTMTabPage*> (e->source ());
if (source) {
/* Tab pages are updated periodically by scheme(?), even if there are no
* changes. So sometimes(not always) when we dragging a tab page, all the
* existing tab pages have been replaced. As a result, we're holding an
* invalid tab page pointer which causes a "segmentation fault" error.
* To avoid this, we need to use a mutex. */
m_updateMutex.lock (); // <-
m_draggingTab= source;
int index= -1;
for (int i= 0; i < m_tabPageList.size (); ++i) {
if (m_tabPageList[i]->m_bufferUrl == g_mostRecentlyDraggedTab) {
index= i;
break;
}
}
if (index != -1) {
m_draggingTabIndex= index;
e->acceptProposedAction ();
}
}

void
QTMTabPageContainer::dragMoveEvent (QDragMoveEvent* e) {
if (m_draggingTab) {
if (m_draggingTabIndex != -1) {
e->acceptProposedAction ();
QPoint pos;
mapToPointing (e, pos);
Expand All @@ -299,37 +294,31 @@ QTMTabPageContainer::dragMoveEvent (QDragMoveEvent* e) {

void
QTMTabPageContainer::dropEvent (QDropEvent* e) {
if (m_draggingTab) {
if (m_draggingTabIndex != -1) {
e->acceptProposedAction ();

QPoint _; // dummy argument
int pointingIndex= mapToPointing (e, _);
int oldIndex = m_tabPageList.indexOf (m_draggingTab);
QPoint _; // dummy argument
int pointingIndex= mapToPointing (e, _);
QTMTabPage* draggingTab = m_tabPageList[m_draggingTabIndex];
int oldIndex = m_draggingTabIndex;
int newIndex= pointingIndex > oldIndex ? pointingIndex - 1 : pointingIndex;

// update tab page positions immediately
if (pointingIndex != oldIndex) {
m_tabPageList.removeAt (oldIndex);
m_tabPageList.insert (newIndex, m_draggingTab);
m_tabPageList.insert (newIndex, draggingTab);
}
arrangeTabPages ();

object url (m_draggingTab->m_bufferUrl);
m_draggingTab= nullptr;
m_updateMutex.unlock ();

object url (draggingTab->m_bufferUrl);
call ("move-buffer-to-index", url, object (newIndex));
}
m_indicator->hide ();
}

void
QTMTabPageContainer::dragLeaveEvent (QDragLeaveEvent* e) {
if (m_draggingTab) {
e->accept ();
m_draggingTab= nullptr;
m_updateMutex.unlock ();
}
e->accept ();
m_indicator->hide ();
}

Expand Down
9 changes: 4 additions & 5 deletions src/Plugins/Qt/QTMTabPage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class QTMTabPage : public QToolButton {

protected:
virtual void resizeEvent (QResizeEvent* e) override;
// virtual void mousePressEvent (QMouseEvent* e) override;
// virtual void mouseMoveEvent (QMouseEvent* e) override;
virtual void mousePressEvent (QMouseEvent* e) override;
virtual void mouseMoveEvent (QMouseEvent* e) override;

private:
void setupStyle ();
Expand All @@ -69,10 +69,9 @@ In order to:
2. Support drag-and-drop to sort tab page
*/
class QTMTabPageContainer : public QWidget {
QMutex m_updateMutex;
QList<QTMTabPage*> m_tabPageList;
int m_rowHeight = 0;
QTMTabPage* m_draggingTab= nullptr;
int m_rowHeight = 0;
int m_draggingTabIndex= -1;
QFrame* m_indicator;

public:
Expand Down

0 comments on commit bc4803e

Please sign in to comment.