Skip to content
Permalink
Browse files

Address some of the reviews

  • Loading branch information
NEDJIMAbelgacem committed Nov 16, 2020
1 parent 270e64a commit 8faa5eee43cf58300bf0e2162b8e4265e44ad4ca
@@ -43,9 +43,12 @@ Returns point cloud layer associated with the renderer
virtual QgsPointCloudLayer3DRenderer *clone() const /Factory/;


void setSymbol( QgsPointCloud3DSymbol *symbol );
void setSymbol( QgsPointCloud3DSymbol *symbol /Transfer/ );
%Docstring
Sets 3D symbol associated with the renderer
Sets the 3D ``symbol`` associated with the renderer.
Ownership of ``symbol`` is transferred to the renderer.

.. seealso:: :py:func:`symbol`
%End
const QgsPointCloud3DSymbol *symbol() const;
%Docstring
@@ -36,7 +36,6 @@
#include "qgsline3dsymbol.h"
#include "qgspoint3dsymbol.h"
#include "qgspolygon3dsymbol.h"
//#include "qgspointcloud3dsymbol.h"

#include <Qt3DExtras/QPhongMaterial>

@@ -26,6 +26,7 @@
#include "qgs3dsymbolregistry.h"
#include "qgspointcloud3dsymbol.h"

#include "qgis.h"

QgsPointCloudLayer3DRendererMetadata::QgsPointCloudLayer3DRendererMetadata()
: Qgs3DRendererAbstractMetadata( QStringLiteral( "pointcloud" ) )
@@ -79,12 +80,12 @@ Qt3DCore::QEntity *QgsPointCloudLayer3DRenderer::createEntity( const Qgs3DMapSet
if ( !pcl || !pcl->dataProvider() || !pcl->dataProvider()->index() )
return nullptr;

return new QgsPointCloudLayerChunkedEntity( pcl->dataProvider()->index(), map, mSymbol );
return new QgsPointCloudLayerChunkedEntity( pcl->dataProvider()->index(), map, mSymbol.get() );
}

void QgsPointCloudLayer3DRenderer::setSymbol( QgsPointCloud3DSymbol *symbol )
{
mSymbol = dynamic_cast<QgsPointCloud3DSymbol *>( symbol->clone() );
mSymbol.reset( symbol );
}

void QgsPointCloudLayer3DRenderer::writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const
@@ -109,7 +110,8 @@ void QgsPointCloudLayer3DRenderer::readXml( const QDomElement &elem, const QgsRe
mLayerRef = QgsMapLayerRef( elem.attribute( QStringLiteral( "layer" ) ) );

QDomElement elemSymbol = elem.firstChildElement( QStringLiteral( "symbol" ) );
if ( !mSymbol ) mSymbol = new QgsPointCloud3DSymbol;
if ( !mSymbol )
mSymbol = qgis::make_unique< QgsPointCloud3DSymbol >();
mSymbol->readXml( elemSymbol, context );
}

@@ -70,18 +70,22 @@ class _3D_EXPORT QgsPointCloudLayer3DRenderer : public QgsAbstract3DRenderer
QgsPointCloudLayer3DRenderer *clone() const override SIP_FACTORY;
Qt3DCore::QEntity *createEntity( const Qgs3DMapSettings &map ) const override SIP_SKIP;

//! Sets 3D symbol associated with the renderer
void setSymbol( QgsPointCloud3DSymbol *symbol );
/**
* Sets the 3D \a symbol associated with the renderer.
* Ownership of \a symbol is transferred to the renderer.
* \see symbol()
*/
void setSymbol( QgsPointCloud3DSymbol *symbol SIP_TRANSFER );
//! Returns 3D symbol associated with the renderer
const QgsPointCloud3DSymbol *symbol() const { return mSymbol; }
const QgsPointCloud3DSymbol *symbol() const { return mSymbol.get(); }

void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
void resolveReferences( const QgsProject &project ) override;

private:
QgsMapLayerRef mLayerRef; //!< Layer used to extract mesh data from
QgsPointCloud3DSymbol *mSymbol = nullptr;
std::unique_ptr< QgsPointCloud3DSymbol > mSymbol;

private:
#ifdef SIP_RUN
@@ -292,7 +292,7 @@ QgsChunkLoader *QgsPointCloudLayerChunkLoaderFactory::createChunkLoader( QgsChun
{
QgsChunkNodeId id = node->tileId();
Q_ASSERT( mPointCloudIndex->hasNode( IndexedPointCloudNode( id.d, id.x, id.y, id.z ) ) );
return new QgsPointCloudLayerChunkLoader( this, node, mSymbol );
return new QgsPointCloudLayerChunkLoader( this, node, dynamic_cast<QgsPointCloud3DSymbol *>( mSymbol->clone() ) );
}

QgsAABB nodeBoundsToAABB( QgsPointCloudDataBounds nodeBounds, QgsVector3D offset, QgsVector3D scale, const Qgs3DMapSettings &map );
@@ -139,7 +139,11 @@ class QgsPointCloudLayerChunkLoaderFactory : public QgsChunkLoaderFactory
class QgsPointCloudLayerChunkLoader : public QgsChunkLoader
{
public:
//! Constructs the loader

/**
* Constructs the loader
* QgsPointCloudLayerChunkLoader takes ownership over symbol
*/
QgsPointCloudLayerChunkLoader( const QgsPointCloudLayerChunkLoaderFactory *factory, QgsChunkNode *node, QgsPointCloud3DSymbol *symbol );
~QgsPointCloudLayerChunkLoader() override;

@@ -15,7 +15,6 @@

#include "qgspointcloud3dsymbol.h"

// TODO: For some reason whwn I define function on a .cpp file they don't get included in the qgis_app target

QgsPointCloud3DSymbol::QgsPointCloud3DSymbol()
: QgsAbstract3DSymbol()
@@ -19,52 +19,17 @@
#include "qgspointcloud3dsymbol.h"
#include "qgspointcloudlayer3drenderer.h"

QgsPointCloud3DSymbolWidget::QgsPointCloud3DSymbolWidget( QgsPointCloudLayer *layer, QWidget *parent )
QgsPointCloud3DSymbolWidget::QgsPointCloud3DSymbolWidget( QgsPointCloud3DSymbol *symbol, QWidget *parent )
: QWidget( parent )
, mLayer( layer )
// , mLayer( layer )
{
this->setupUi( this );

if ( layer )
{
QgsPointCloudLayer3DRenderer *renderer = dynamic_cast<QgsPointCloudLayer3DRenderer *>( layer->renderer3D() );
QgsPointCloud3DSymbol *symbol;
if ( renderer != nullptr )
{
symbol = const_cast<QgsPointCloud3DSymbol *>( renderer->symbol() );
setSymbol( symbol );
}
else
{
symbol = new QgsPointCloud3DSymbol;
setSymbol( symbol );
delete symbol;
}
}
if ( symbol )
setSymbol( symbol );

connect( mPointSizeSpinBox, qgis::overload<double>::of( &QDoubleSpinBox::valueChanged ), this, &QgsPointCloud3DSymbolWidget::changed );
}

void QgsPointCloud3DSymbolWidget::setLayer( QgsPointCloudLayer *layer )
{
mLayer = layer;
if ( layer )
{
QgsPointCloudLayer3DRenderer *renderer = dynamic_cast<QgsPointCloudLayer3DRenderer *>( layer->renderer3D() );
QgsPointCloud3DSymbol *symbol;
if ( renderer != nullptr )
{
symbol = const_cast<QgsPointCloud3DSymbol *>( renderer->symbol() );
setSymbol( symbol );
}
else
{
symbol = new QgsPointCloud3DSymbol;
setSymbol( symbol );
delete symbol;
}
}
}

void QgsPointCloud3DSymbolWidget::setSymbol( QgsPointCloud3DSymbol *symbol )
{
@@ -73,7 +38,6 @@ void QgsPointCloud3DSymbolWidget::setSymbol( QgsPointCloud3DSymbol *symbol )

QgsPointCloud3DSymbol *QgsPointCloud3DSymbolWidget::symbol() const
{
// TODO: fix memory leak
QgsPointCloud3DSymbol *symb = new QgsPointCloud3DSymbol;
symb->setPointSize( mPointSizeSpinBox->value() );
return symb;
@@ -26,20 +26,14 @@ class QgsPointCloud3DSymbolWidget : public QWidget, private Ui::QgsPointCloud3DS
Q_OBJECT

public:
explicit QgsPointCloud3DSymbolWidget( QgsPointCloudLayer *layer, QWidget *parent = nullptr );
explicit QgsPointCloud3DSymbolWidget( QgsPointCloud3DSymbol *symbol, QWidget *parent = nullptr );

void setLayer( QgsPointCloudLayer *pointCloudLayer );

QgsPointCloudLayer *pointCloudLayer() const { return mLayer; }
void setSymbol( QgsPointCloud3DSymbol *symbol );

QgsPointCloud3DSymbol *symbol() const;

signals:
void changed();

private:
QgsPointCloudLayer *mLayer = nullptr;
};

#endif // QGSPOINTCLOUD3DSYMBOLWIDGET_H
@@ -35,7 +35,7 @@ QgsPointCloudLayer3DRendererWidget::QgsPointCloudLayer3DRendererWidget( QgsPoint
mChkEnabled = new QCheckBox( tr( "Enable 3D Renderer" ), this );
layout->addWidget( mChkEnabled );

mWidgetPointCloudSymbol = new QgsPointCloud3DSymbolWidget( layer, this );
mWidgetPointCloudSymbol = new QgsPointCloud3DSymbolWidget( nullptr, this );
layout->addWidget( mWidgetPointCloudSymbol );

connect( mChkEnabled, &QCheckBox::clicked, this, &QgsPointCloudLayer3DRendererWidget::onEnabledClicked );
@@ -44,21 +44,18 @@ QgsPointCloudLayer3DRendererWidget::QgsPointCloudLayer3DRendererWidget( QgsPoint

void QgsPointCloudLayer3DRendererWidget::setRenderer( const QgsPointCloudLayer3DRenderer *renderer )
{
mRenderer.reset( renderer ? renderer->clone() : nullptr );
if ( renderer != nullptr )
mWidgetPointCloudSymbol->setSymbol( const_cast<QgsPointCloud3DSymbol *>( renderer->symbol() ) );
whileBlocking( mChkEnabled )->setChecked( renderer ? renderer->symbol()->isEnabled() : false );
}

QgsPointCloudLayer3DRenderer *QgsPointCloudLayer3DRendererWidget::renderer()
{
// TODO: use unique_ptr for point cloud symbol
QgsPointCloudLayer3DRenderer *renderer = new QgsPointCloudLayer3DRenderer;
QgsPointCloud3DSymbol *sym = mWidgetPointCloudSymbol->symbol();
mRenderer.reset( new QgsPointCloudLayer3DRenderer );
mRenderer->setSymbol( sym );
delete sym;
mRenderer->setLayer( qobject_cast<QgsPointCloudLayer *>( mLayer ) );
return mRenderer.get();
renderer->setSymbol( sym );
renderer->setLayer( qobject_cast<QgsPointCloudLayer *>( mLayer ) );
return renderer;
}

void QgsPointCloudLayer3DRendererWidget::apply()
@@ -81,9 +78,6 @@ void QgsPointCloudLayer3DRendererWidget::onEnabledClicked()

void QgsPointCloudLayer3DRendererWidget::syncToLayer( QgsMapLayer *layer )
{
mLayer = layer ;
QgsPointCloudLayer *pointCloudLayer = qobject_cast<QgsPointCloudLayer *>( layer );
mWidgetPointCloudSymbol->setLayer( pointCloudLayer );
QgsAbstract3DRenderer *r = layer->renderer3D();
if ( r && r->type() == QLatin1String( "pointcloud" ) )
{
@@ -21,6 +21,7 @@
#include "qgsmaplayerconfigwidget.h"
#include "qgspointcloudlayer3drenderer.h"
#include "qgsmaplayerconfigwidgetfactory.h"
#include "qgspointcloud3dsymbol.h"

class QCheckBox;

@@ -51,7 +52,6 @@ class QgsPointCloudLayer3DRendererWidget : public QgsMapLayerConfigWidget
private:
QCheckBox *mChkEnabled = nullptr;
QgsPointCloud3DSymbolWidget *mWidgetPointCloudSymbol = nullptr;
std::unique_ptr<QgsPointCloudLayer3DRenderer> mRenderer;
};

class QgsPointCloudLayer3DRendererWidgetFactory : public QObject, public QgsMapLayerConfigWidgetFactory
@@ -683,7 +683,7 @@ void QgsLayerStylingWidget::updateCurrentWidgetLayer()
#ifdef HAVE_3D
if ( !mPointCloud3DWidget )
{
mPointCloud3DWidget = new QgsPointCloudLayer3DRendererWidget( nullptr, mMapCanvas, mWidgetStack );
mPointCloud3DWidget = new QgsPointCloudLayer3DRendererWidget( pcLayer, mMapCanvas, mWidgetStack );
mPointCloud3DWidget->setDockMode( true );
connect( mPointCloud3DWidget, &QgsPointCloudLayer3DRendererWidget::widgetChanged, this, &QgsLayerStylingWidget::autoApply );
}

0 comments on commit 8faa5ee

Please sign in to comment.
You can’t perform that action at this time.