Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spatial Bookmark Manager crash #56493

Closed
2 tasks
yljxhh opened this issue Feb 23, 2024 · 2 comments · Fixed by #56498
Closed
2 tasks

Spatial Bookmark Manager crash #56493

yljxhh opened this issue Feb 23, 2024 · 2 comments · Fixed by #56498
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption

Comments

@yljxhh
Copy link

yljxhh commented Feb 23, 2024

What is the bug or the crash?

A crash happened when using Spatial Bookmark Manager.

Steps to reproduce the issue

User Feedback

1 open Spatial Bookmark Manager,
2 then,add 6 bookmarks,
3 after that, delete the 4 and 5 bookmarks at the same time,
4 then, add a new bookmark.
5 oops, may cause a crash!

Report Details

Python Stack Trace

Windows fatal exception: access violation

Current thread 0x00006738 (most recent call first):
<no Python frame>

Stack Trace


QSortFilterProxyModel::parent :
QSortFilterProxyModel::mapToSource :
QSortFilterProxyModel::flags :
QModelIndex::flags :
QAbstractItemView::focusInEvent :
QWidget::event :
QFrame::event :
QAbstractScrollArea::event :
QApplicationPrivate::notify_helper :
QApplication::notify :
QgsApplication::notify :
QCoreApplication::notifyInternal2 :
QApplicationPrivate::setFocusWidget :
QWidget::setFocus :
QApplication::setActiveWindow :
QApplicationPrivate::notifyActiveWindowChange :
QGuiApplicationPrivate::processActivatedEvent :
QWindowSystemInterface::sendWindowSystemEvents :
QEventDispatcherWin32::processEvents :
qt_plugin_query_metadata :
QEventLoop::exec :
QCoreApplication::exec :
main :
BaseThreadInitThunk :
RtlUserThreadStart :

QGIS Info
QGIS Version: 3.28.4-Firenze
QGIS code revision: fd0fb72
Compiled against Qt: 5.15.3
Running against Qt: 5.15.3
Compiled against GDAL: 3.6.2
Running against GDAL: 3.6.2

System Info
CPU Type: x86_64
Kernel Type: winnt
Kernel Version: 10.0.22621

Versions

QGIS Info
QGIS Version: 3.28.4-Firenze
QGIS code revision: fd0fb72
Compiled against Qt: 5.15.3
Running against Qt: 5.15.3
Compiled against GDAL: 3.6.2
Running against GDAL: 3.6.2

System Info
CPU Type: x86_64
Kernel Type: winnt
Kernel Version: 10.0.22621

Supported QGIS version

  • I'm running a supported QGIS version according to the roadmap.

New profile

Additional context

No response

@yljxhh yljxhh added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Feb 23, 2024
@agiudiceandrea
Copy link
Contributor

While I cannot replicate the Crash on my system, I can see that there is an issue removing multiple items using the Spatial Bookmark Manager:

bookmarks.mp4

Ping @elpaso.

@elpaso
Copy link
Contributor

elpaso commented Feb 23, 2024

All I can see is a warning QSortFilterProxyModel: inconsistent changes reported by source model, and I can fix it with:

diff --git a/src/core/qgsbookmarkmodel.cpp b/src/core/qgsbookmarkmodel.cpp
index e1b2dabd5c8..5e833d3e601 100644
--- a/src/core/qgsbookmarkmodel.cpp
+++ b/src/core/qgsbookmarkmodel.cpp
@@ -222,11 +222,9 @@ bool QgsBookmarkManagerModel::setData( const QModelIndex &index, const QVariant
   return false;
 }
 
-bool QgsBookmarkManagerModel::insertRows( int, int count, const QModelIndex &parent )
+bool QgsBookmarkManagerModel::insertRows( int, int count, const QModelIndex & )
 {
   // append
-  const int oldCount = mManager->bookmarks().count();
-  beginInsertRows( parent, oldCount, oldCount + count );
   bool result = true;
   for ( int i = 0; i < count; ++i )
   {
@@ -236,14 +234,11 @@ bool QgsBookmarkManagerModel::insertRows( int, int count, const QModelIndex &par
     mBlocked = false;
     result &= res;
   }
-  endInsertRows();
   return result;
 }
 
-bool QgsBookmarkManagerModel::removeRows( int row, int count, const QModelIndex &parent )
+bool QgsBookmarkManagerModel::removeRows( int row, int count, const QModelIndex & )
 {
-  beginRemoveRows( parent, row, row + count );
-
   const QList< QgsBookmark > appBookmarks = mManager->bookmarks();
   const QList< QgsBookmark > projectBookmarks = mProjectManager->bookmarks();
   for ( int r = row + count - 1; r >= row; --r )
@@ -253,7 +248,6 @@ bool QgsBookmarkManagerModel::removeRows( int row, int count, const QModelIndex
     else
       mManager->removeBookmark( appBookmarks.at( r ).id() );
   }
-  endRemoveRows();
   return true;
 }

elpaso added a commit to elpaso/QGIS that referenced this issue Feb 23, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix qgis#56493
nyalldawson pushed a commit that referenced this issue Feb 25, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
qgis-bot pushed a commit that referenced this issue Feb 25, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
qgis-bot pushed a commit that referenced this issue Feb 25, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
nyalldawson pushed a commit that referenced this issue Feb 26, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
nyalldawson pushed a commit that referenced this issue Feb 26, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
nyalldawson pushed a commit that referenced this issue Mar 27, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
nyalldawson pushed a commit that referenced this issue Mar 27, 2024
rows inserted/added was fired twice (once by the manager and once by the
model), confusing the proxy model.

Also count was wrong by 1 in beginInsertRows( parent, oldCount, oldCount + count )
and beginRemoveRows, but now these two calls have been removed completely.

Fix #56493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants