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

normalizeForVisualization() and other methods applying on a ProjectedCRS: do not mess the derivingConversion object of the original object (fixes #1736) #1746

Merged
merged 1 commit into from Nov 25, 2019

Conversation

@rouault
Copy link
Member

rouault commented Nov 25, 2019

normalizeForVisualization(), promoteTo3D(), demoteTo2D(), alterGeodeticCRS(),
alterCSLinearUnit() and alterParametersLinearUnit() all used the object
returned by derivingConversionRef() to create a new ProjectedCRS. While doing
so, this caused the derivingConversion of the original object to have its
targetCRS set to the object returned by normalizeForVisualization() and similar.
If that object died, then the weak pointer would be reset, and the original
ProjectedCRS() has now its derivingConversionRef()->targetCRS() nullptr

So bottom line is use derivingConversion() for anything that is not pure
reading !!!

This is confirmed to be the fix for the QGIS scenario in
qgis/QGIS#30569 (comment)

In QGIS use case, the issue arised when using a projected CRS with a non-GIS
friendly axis (that is where normalizeForVisualization() created a new projectedCRS)

…CRS: do not mess the derivingConversion object of the original object (fixes #1736)

normalizeForVisualization(), promoteTo3D(), demoteTo2D(), alterGeodeticCRS(),
alterCSLinearUnit() and alterParametersLinearUnit() all used the object
returned by derivingConversionRef() to create a new ProjectedCRS. While doing
so, this caused the derivingConversion of the original object to have its
targetCRS set to the object returned by normalizeForVisualization() and similar.
If that object died, then the weak pointer would be reset, and the original
ProjectedCRS() has now its derivingConversionRef()->targetCRS() nullptr

So bottom line is use derivingConversion() for anything that is not pure
reading !!!

This is confirmed to be the fix for the QGIS scenario in
qgis/QGIS#30569 (comment)

In QGIS use case, the issue arised when using a projected CRS with a non-GIS
friendly axis (that is where normalizeForVisualization() created a new projectedCRS)
@rouault rouault force-pushed the rouault:fix_1736 branch from df9f9e4 to 685a644 Nov 25, 2019
@nyalldawson

This comment has been minimized.

Copy link
Contributor

nyalldawson commented Nov 25, 2019

Confirming that this fixes the issue for me too -- fantastic work @rouault !!

@rouault rouault merged commit bc5148f into OSGeo:master Nov 25, 2019
4 checks passed
4 checks passed
FreeBSD Task Summary
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.002%) to 85.907%
Details
@rouault

This comment has been minimized.

Copy link
Member Author

rouault commented Nov 25, 2019

Cherry-picked in 6.2 branch as bce4b15

@kbevers kbevers added this to the 6.3.0 milestone Nov 25, 2019
@rouault

This comment has been minimized.

Copy link
Member Author

rouault commented Nov 25, 2019

@kbevers Jürgen Fischer announced on https://lists.osgeo.org/pipermail/qgis-developer/2019-November/059411.html that he had cherry-picked the fix to generate a 6.2.1 patched OSGeo4W package. So I guess we can probably just follow the original plan of a 6.3.0 release beginning of January. Speaking about that, PROJ master uses 7.0.0 numbering currently. Should it be changed to 6.3.0 or will you do so when you create the 6.3 branch from master ?

@kbevers

This comment has been minimized.

Copy link
Member

kbevers commented Nov 25, 2019

@rouault Yes, I just saw Jürgens email. I agree with the 6.3.0 as the next release.

As for version number in master, yeah that should probably be changed. Feel free to do it yourself, otherwise I'll just do it when I prepare the release in about a months time.

@happydeb

This comment has been minimized.

Copy link

happydeb commented Nov 26, 2019

@nyalldawson @rouault @gioman So you all have made quite a bit of progress, I have reproduced some failures quite unintentionally since I hadn't followed the progress on this thread till now. I use Windows 10 64x. It started with adding multiple layers simultaneously...

Using ESRI 104019 (ITRF2014) (World - not Australia) GCRS as a Project Canvas CRS because I need to use projected data that will not work accurately with psuedo mercators (all tiled web maps Bing Google etc), and 3.10 is crashing with a projected canvas, i.e. with the Bing/Google Base map projected to match my data.

Adding GCRS layer worked fine on close and reopen.
Adding a different GRS layer from a data repsitory had a bug that I thought warranted closing QGIS to see if it would reopen. The problem happened when I added two layers simultaneously of different CGCRS (counties and roads) and only one showed up in the TOC but both rendered in the map Canvass (and in the overview insets in the layout)?!

Reopening I chose the same GCRS 104019 world. QGIS crashed. Here is the error log.

Stack Trace

osgeo::proj::datum::Datum::conventionalRS :
osgeo::proj::datum::Datum::conventionalRS :
osgeo::proj::datum::Datum::conventionalRS :
OGRSpatialReference::~OGRSpatialReference :
OGRSpatialReference::operator= :
OGRGeomFieldDefn::~OGRGeomFieldDefn :
SHPWriteObject :
OGRFeatureDefn::~OGRFeatureDefn :
OGRFeatureDefn::~OGRFeatureDefn :
SHPWriteObject :
DBFWriteTuple :
OGREditableLayer::Translate :
QgsOgrProviderUtils::GDALCloseWrapper qgsogrprovider.cpp:4129
QgsConnectionPoolGroup::onConnectionExpired qgsconnectionpool.h:227
QMetaObject::activate :
QTimer::timerEvent :
QObject::event :
QApplicationPrivate::notify_helper :
QApplication::notify :
QgsApplication::notify qgsapplication.cpp:418
QCoreApplication::notifyInternal2 :
QEventDispatcherWin32Private::sendTimerEvent :
QEventDispatcherWin32::processEvents :
UserCallWinProcCheckWow :
DispatchMessageWorker :
QEventDispatcherWin32::processEvents :
qt_plugin_query_metadata :
QEventLoop::exec :
QCoreApplication::exec :
main main.cpp:1598
BaseThreadInitThunk :
RtlUserThreadStart :

QGIS Info
QGIS Version: 3.10.0-A Coru�a
QGIS code revision: 6ffa89eb3e
Compiled against Qt: 5.11.2
Running against Qt: 5.11.2
Compiled against GDAL: 3.0.2
Running against GDAL: 3.0.2

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

AND THE FOLLOWING HAPPENED BEFORE THE PROJECT REFERENCED ABOVE CRASHED QGIS. I DON'T KNOW IF ITS A SEPARATE OR RELATED ISSUE. TRYING TO RELOAD A SAVED LAYOUT 3.11 CANT FIND THEM, WHEN I MANUALLY INPUT THE LOCATION IT LOADS QGIS 3.46 (THE VERSION THE LAYOUT WAS MADE IN) THE LESSON IS DON'T TRY TO TRANSPORT ANY PART OF AN OLD PROJECT TO 3.1?

THE SPECIFIED FOLDER NOW SHOWS UP IN THE DEFAULT LAYOUT MGR. A CLOSE AND REOPEN AFTER THIS ISSUE WORKED (CHOOSING THE ESRI104019 (ITRF2014) BUT CRASHED ON THE MULTIPLE LAYERS ISSUE.

An error has occurred while executing Python code:

RuntimeError: wrapped C/C++ object of type ContourGeneratorAlgorithm has been deleted
Traceback (most recent call last):
File "C:/Users/happy/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\contour\ContourGeneratorProvider.py", line 55, in loadAlgorithms
self.addAlgorithm( alg )
RuntimeError: wrapped C/C++ object of type ContourGeneratorAlgorithm has been deleted

Python version: 3.7.0 (v3.7.0:1bf9cc5093, Jun 27 2018, 04:59:51) [MSC v.1914 64 bit (AMD64)]
QGIS version: 3.10.0-A Coruña A Coruña, 6ffa89eb3e

Python Path:
C:/Users/happy/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins\contour
C:/PROGRA1/QGIS31.10/apps/qgis/./python
C:/Users/happy/AppData/Roaming/QGIS/QGIS3\profiles\default/python
C:/Users/happy/AppData/Roaming/QGIS/QGIS3\profiles\default/python/plugins
C:/PROGRA1/QGIS31.10/apps/qgis/./python/plugins
C:\Program Files\QGIS 3.10\bin\python37.zip
C:\PROGRA1\QGIS31.10\apps\Python37\DLLs
C:\PROGRA1\QGIS31.10\apps\Python37\lib
C:\Program Files\QGIS 3.10\bin
C:\PROGRA1\QGIS31.10\apps\Python37
C:\PROGRA1\QGIS31.10\apps\Python37\lib\site-packages
C:\PROGRA1\QGIS31.10\apps\Python37\lib\site-packages\win32
C:\PROGRA1\QGIS31.10\apps\Python37\lib\site-packages\win32\lib
C:\PROGRA1\QGIS31.10\apps\Python37\lib\site-packages\Pythonwin
C:/Users/happy/AppData/Roaming/QGIS/QGIS3\profiles\default/python
C:/Users/happy/Documents/KEC/20191009DeLuzWD/QGIS

CAN I IMPLEMENT ANY OF YOUR FIXES; WOULD IT HELP? OR SHOULD I JUST DO WORK AROUNDS?

@nyalldawson

This comment has been minimized.

Copy link
Contributor

nyalldawson commented Nov 26, 2019

@happydeb just wait till 3.10.1 is released at the end of the week -- it will include this fix.

RuntimeError: wrapped C/C++ object of type ContourGeneratorAlgorithm has been deleted

This is a bug in the "contour" plugin, you'll need to report this one with the authors of that plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.