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

WIP: Streamlines #20

Closed
wants to merge 22 commits into from
Closed

Conversation

PeterPetrik
Copy link
Owner

... todo for official pr ....

src/core/mesh/qgsmeshtracerenderer.cpp Show resolved Hide resolved
@@ -0,0 +1,6 @@
#include "qgsmeshtracerenderer.h"

QgsMeshTraceRenderer::QgsMeshTraceRenderer()
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= default

src/core/mesh/qgsmeshtracerenderer.h Show resolved Hide resolved
* \note not available in Python bindings
* \since QGIS 3.12
*/
class QgsMeshVectorValueInterpolatorFromNode: public QgsMeshVectorValueInterpolator
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use terminology consistenty. Face, Vertex, Volume (3D). No nodes, no elements...

@@ -0,0 +1,123 @@
/***************************************************************************
testqgsmeshlayer.cpp
--------------------
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong header

Copy link
Owner Author

@PeterPetrik PeterPetrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job, keep it going!

point );
}

QgsMeshVectorValueInterpolator::QgsMeshVectorValueInterpolator( QgsTriangularMesh triangularMesh,
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably you want to pass references, not values?

QgsMeshDataBlock scalarActiveFaceFlagValues ):
mTriangularMesh( triangularMesh ),
mDatasetValues( datasetVectorValues ),
mActiveFaceFlagValue( scalarActiveFaceFlagValues )
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mActiveFaceFlagValues (missing s)


}

QgsMeshVectorValueInterpolator::~QgsMeshVectorValueInterpolator() {}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QgsMeshVectorValueInterpolator::~QgsMeshVectorValueInterpolator() = default


QgsMeshVectorValueInterpolator::~QgsMeshVectorValueInterpolator() {}

void QgsMeshVectorValueInterpolator::updateFace( const QgsPointXY &point )
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if you need to first check if the point is in triangle. Isn't it enough to just check if interpolateVectorFromVerticesData is not NaN?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are maybe right! But it will depend which of interpolateVectorFromVerticesData or isInTriangleFace is faster. If interpolateVectorFromVerticesData do the job as fast, no need to have the isInTriangleFace function.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that how it is done elsewhere

  1. from spatial index on triangular mesh find just triangles that are in your current bounding box/extent
  2. on those selected triangles do interpolateVectorFromVerticesData to find if inside/ouside na data interpolated

return mFieldResolution;
}

QgsPointXY QgsMeshTraceField::positionToMapCoordinnate( const QPoint &pixelPosition, const QgsPointXY &positionInPixel )
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo "coordinate"

bool mValid;

QgsMeshVectorValueInterpolator *mVectorValueInterpolator;
double mVmax;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialize

QgsMapToPixel mMapToFieldPixel;
QSize mFieldSize;
QPoint mFieldTopLeftInDeviceCoordinates;
bool mValid;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialize

* \note not available in Python bindings
* \since QGIS 3.12
*/
class QgsMeshTraceParticule
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it is not "particle" in english

delete mTraceField;
}

void draw()
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementations to cpp...

* NB : tirangle surface = Isleft / 2
* * \since QGIS 3.12
*/
CORE_EXPORT double IsLeft2D( const QgsPoint &p1, const QgsPoint &p2, const QgsPoint &p );
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to be exported?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

@vcloarec vcloarec force-pushed the streamlines branch 3 times, most recently from 5d0f0c4 to 736f143 Compare November 19, 2019 04:30
@PeterPetrik
Copy link
Owner Author

  • please remove file .TODO.kate-swp
  • please rebase against latest master and squash to 1 or 2 commits

Copy link
Owner Author

@PeterPetrik PeterPetrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks very good... if you squash the commits to 1 commit, rebase and fix the issues in the review I think we should create official PR in QGIS and close this one

@@ -59,6 +59,27 @@ class APP_EXPORT QgsMeshRendererVectorSettingsWidget : public QWidget, private U
//! Mesh rendering settings changed
void widgetChanged();

private slots:
void onSymbologyChanged( int currentIndex )
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move implementation to cpp file

@@ -69,6 +90,12 @@ class APP_EXPORT QgsMeshRendererVectorSettingsWidget : public QWidget, private U

QgsMeshLayer *mMeshLayer = nullptr; //not owned
int mActiveDatasetGroup = -1;

QList<QWidget *> mArrowSettingsWidget;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would group the arrow settings widget in one new parent widget in ui file. same with streamlines widgets. and then just hide the whole group.

@@ -152,7 +154,7 @@ void QgsMeshLayerRenderer::copyVectorDatasetValues( QgsMeshLayer *layer )
const int datasetGroupCount = layer->dataProvider()->datasetGroupCount();
QgsMeshLayerRendererCache *cache = layer->rendererCache();
if ( ( cache->mDatasetGroupsCount == datasetGroupCount ) &&
( cache->mActiveVectorDatasetIndex == datasetIndex ) )
( cache->mActiveVectorDatasetIndex == datasetIndex ) ) //scalar weight data set can have a different datasetIndex
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this comment still actual?

@@ -33,6 +33,7 @@ class QgsMeshLayer;
#include "qgsmeshlayer.h"
#include "qgssymbol.h"
#include "qgsmeshdataprovider.h"
#include "qgsmeshtracerenderer.h" //TODO : dispach this file in two files : one with renderer the other one with fields
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove/implement todo comment?

mSeedingDensity = seedingDensity;
}


Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra lines

{
if ( ! QgsMeshUtils::isInTriangleFace( point, mFaceCache, mTriangularMesh.vertices() ) )
{
/*the first version of this method uses QgsGeometry::contain to check if the point is in triangles,
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably developer comment, best to remove it for production version and also rename the function you want to use .remove the other one

return;
}

QgsPointXY interestZoneTopLeft = deviceMapToPixel.transform( QgsPointXY( interestZoneExtent.xMinimum(), interestZoneExtent.yMaximum() ) );
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this implemented?

//! Dtor
~QgsMeshVectorRenderer();
~QgsMeshVectorArrowRenderer() override {}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move implementation to cpp. use =default

@@ -85,6 +85,10 @@ class CORE_EXPORT QgsTriangularMesh
*/
int faceIndexForPoint( const QgsPointXY &point ) const ;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, maybe there are advantages to use GEOS for some context ? Is the first version is used elsewhere ? My QtCreator doesn't find another use...

* Result: > 0 if on the left
* = 0 if on the line
* < 0 if on the right
* NB : tirangle surface = Isleft / 2
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: triangle

@PeterPetrik
Copy link
Owner Author

closing this, best to open official PR!

PeterPetrik pushed a commit that referenced this pull request Aug 25, 2020
These references are to objects belonging to another thread, which
can cause crashes if the object on the main thread is deleted while
the handler is still active

Refs a asan report:

==677416==ERROR: AddressSanitizer: heap-use-after-free on address 0x6070029bc278 at pc 0x7f95719ccc45 bp 0x7f94bdd7a310 sp 0x7f94bdd7a300
READ of size 4 at 0x6070029bc278 thread T36 (Thread (pooled))
    #0 0x7f95719ccc44 in QgsLine3DSymbol::extrusionHeight() const /home/nyall/dev/qgis-asan/src/3d/symbols/qgsline3dsymbol.h:78
    #1 0x7f95674a7c01 in QgsBufferedLine3DSymbolHandler::processFeature(QgsFeature&, Qgs3DRenderContext const&) /home/nyall/dev/qgis-asan/src/3d/symbols/qgsline3dsymbol_p.cpp:126
    #2 0x7f9567457045 in operator() /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.cpp:97
    #3 0x7f95674597fd in runFunctor /usr/include/qt5/QtConcurrent/qtconcurrentstoredfunctioncall.h:70
    #4 0x7f956744acf7 in QtConcurrent::RunFunctionTask<void>::run() (/home/nyall/dev/build-QGIS-asan/output/lib/libqgis_3d.so.3.15.0+0x2adcf7)
    #5 0x7f956a275d59  (/usr/lib64/libQt5Core.so.5+0xc9d59)
    #6 0x7f956a27268f  (/usr/lib64/libQt5Core.so.5+0xc668f)
    #7 0x7f956a193431 in start_thread (/usr/lib64/libpthread.so.0+0x9431)
    #8 0x7f955ad59912 in __GI___clone (/usr/lib64/libc.so.6+0x101912)

0x6070029bc278 is located 56 bytes inside of 72-byte region [0x6070029bc240,0x6070029bc288)
freed by thread T0 here:
    #0 0x7f9572b86b87 in operator delete(void*) (/usr/lib64/libasan.so.6+0xb2b87)
    #1 0x7f956749c00b in QgsLine3DSymbol::~QgsLine3DSymbol() /home/nyall/dev/qgis-asan/src/3d/symbols/qgsline3dsymbol.cpp:30
    #2 0x7f95719664ca in std::default_delete<QgsAbstract3DSymbol>::operator()(QgsAbstract3DSymbol*) const /usr/include/c++/10/bits/unique_ptr.h:85
    #3 0x7f95674428ce in std::unique_ptr<QgsAbstract3DSymbol, std::default_delete<QgsAbstract3DSymbol> >::~unique_ptr() /usr/include/c++/10/bits/unique_ptr.h:361
    #4 0x7f9567459eb6 in QgsVectorLayerChunkLoaderFactory::~QgsVectorLayerChunkLoaderFactory() /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.h:53
    #5 0x7f9567459edd in QgsVectorLayerChunkLoaderFactory::~QgsVectorLayerChunkLoaderFactory() /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.h:53
    #6 0x7f956746e4db in QgsChunkedEntity::~QgsChunkedEntity() /home/nyall/dev/qgis-asan/src/3d/chunks/qgschunkedentity_p.cpp:122
    #7 0x7f9567459358 in QgsVectorLayerChunkedEntity::~QgsVectorLayerChunkedEntity() /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.cpp:164
    #8 0x7f9567459373 in QgsVectorLayerChunkedEntity::~QgsVectorLayerChunkedEntity() /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.cpp:168
    #9 0x7f956a42e960 in QObject::event(QEvent*) (/usr/lib64/libQt5Core.so.5+0x282960)
    #10 0x7f956ada2062 in QApplicationPrivate::notify_helper(QObject*, QEvent*) (/usr/lib64/libQt5Widgets.so.5+0x172062)

previously allocated by thread T0 here:
    #0 0x7f9572b86067 in operator new(unsigned long) (/usr/lib64/libasan.so.6+0xb2067)
    #1 0x7f95674a3018 in qgis::_Unique_if<QgsLine3DSymbol>::_Single_object qgis::make_unique<QgsLine3DSymbol>() /home/nyall/dev/qgis-asan/src/core/qgis.h:425
    #2 0x7f956749c0b9 in QgsLine3DSymbol::clone() const /home/nyall/dev/qgis-asan/src/3d/symbols/qgsline3dsymbol.cpp:34
    #3 0x7f9567458b5f in QgsVectorLayerChunkLoaderFactory::QgsVectorLayerChunkLoaderFactory(Qgs3DMapSettings const&, QgsVectorLayer*, QgsAbstract3DSymbol*, int) /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.cpp:141
    #4 0x7f9567458d88 in QgsVectorLayerChunkedEntity::QgsVectorLayerChunkedEntity(QgsVectorLayer*, double, double, QgsVectorLayer3DTilingSettings const&, QgsAbstract3DSymbol*, Qgs3DMapSettings const&) /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayerchunkloader_p.cpp:159
    #5 0x7f956745558f in QgsVectorLayer3DRenderer::createEntity(Qgs3DMapSettings const&) const /home/nyall/dev/qgis-asan/src/3d/qgsvectorlayer3drenderer.cpp:76
    #6 0x7f95673a71c0 in Qgs3DMapScene::addLayerEntity(QgsMapLayer*) /home/nyall/dev/qgis-asan/src/3d/qgs3dmapscene.cpp:693
    #7 0x7f95673a5523 in Qgs3DMapScene::onLayerRenderer3DChanged() /home/nyall/dev/qgis-asan/src/3d/qgs3dmapscene.cpp:598
    #8 0x7f95673c6c33 in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (Qgs3DMapScene::*)()>::call(void (Qgs3DMapScene::*)(), Qgs3DMapScene*, void**) (/home/nyall/dev/build-QGIS-asan/output/lib/libqgis_3d.so.3.15.0+0x229c33)
    #9 0x7f95673c5468 in void QtPrivate::FunctionPointer<void (Qgs3DMapScene::*)()>::call<QtPrivate::List<>, void>(void (Qgs3DMapScene::*)(), Qgs3DMapScene*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:185
    #10 0x7f95673c1748 in QtPrivate::QSlotObject<void (Qgs3DMapScene::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) /usr/include/qt5/QtCore/qobjectdefs_impl.h:418
    #11 0x7f956a435f75  (/usr/lib64/libQt5Core.so.5+0x289f75)
    #12 0x7f956252106c in QgsMapLayer::renderer3DChanged() src/core/qgis_core_autogen/EWIEGA46WW/moc_qgsmaplayer.cpp:702
    #13 0x7f9563917a12 in QgsMapLayer::setRenderer3D(QgsAbstract3DRenderer*) /home/nyall/dev/qgis-asan/src/core/qgsmaplayer.cpp:1813
    #14 0x7f95719aaf52 in QgsApp3DSymbolWidgetWithPreview::updatePreview(QgsAbstract3DSymbol*) /home/nyall/dev/qgis-asan/src/app/3d/qgsapp3dsymbolwidget.cpp:243
    #15 0x7f95719a97af in operator() /home/nyall/dev/qgis-asan/src/app/3d/qgsapp3dsymbolwidget.cpp:175
    #16 0x7f95719ac2dd in call /usr/include/qt5/QtCore/qobjectdefs_impl.h:146
    #17 0x7f95719ac24f in call<QtPrivate::List<>, void> /usr/include/qt5/QtCore/qobjectdefs_impl.h:256
    #18 0x7f95719ac21e in impl /usr/include/qt5/QtCore/qobjectdefs_impl.h:443
    #19 0x7f956a435f75  (/usr/lib64/libQt5Core.so.5+0x289f75)
    #20 0x7f95708d15ec in QgsApp3DSymbolWidget::widgetChanged() src/app/qgis_app_autogen/6LADBHSVD5/moc_qgsapp3dsymbolwidget.cpp:144
    #21 0x7f95719ae1dd in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (QgsApp3DSymbolWidget::*)()>::call(void (QgsApp3DSymbolWidget::*)(), QgsApp3DSymbolWidget*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:152
    #22 0x7f95719add9c in void QtPrivate::FunctionPointer<void (QgsApp3DSymbolWidget::*)()>::call<QtPrivate::List<>, void>(void (QgsApp3DSymbolWidget::*)(), QgsApp3DSymbolWidget*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:185
    #23 0x7f95719adbfa in QtPrivate::QSlotObject<void (QgsApp3DSymbolWidget::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) /usr/include/qt5/QtCore/qobjectdefs_impl.h:418
    #24 0x7f956a435f75  (/usr/lib64/libQt5Core.so.5+0x289f75)
    #25 0x7f956c91c63e in Qgs3DSymbolWidget::changed() src/gui/qgis_gui_autogen/EWIEGA46WW/moc_qgs3dsymbolwidget.cpp:131
    #26 0x7f95719ce2ad in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (Qgs3DSymbolWidget::*)()>::call(void (Qgs3DSymbolWidget::*)(), Qgs3DSymbolWidget*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:152
    #27 0x7f95719ce014 in void QtPrivate::FunctionPointer<void (Qgs3DSymbolWidget::*)()>::call<QtPrivate::List<>, void>(void (Qgs3DSymbolWidget::*)(), Qgs3DSymbolWidget*, void**) /usr/include/qt5/QtCore/qobjectdefs_impl.h:185
    #28 0x7f95719cdbfc in QtPrivate::QSlotObject<void (Qgs3DSymbolWidget::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) /usr/include/qt5/QtCore/qobjectdefs_impl.h:418
    #29 0x7f956a435f75  (/usr/lib64/libQt5Core.so.5+0x289f75)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants