Skip to content
Permalink
Browse files

Address comments from Nyall's code review

  • Loading branch information
wonder-sk committed Sep 30, 2017
1 parent 041704e commit 518dd1769945a2dcaae04e16b89474d41899527a
Showing with 96 additions and 141 deletions.
  1. +2 −2 python/core/3d/qgs3drendererregistry.sip
  2. +2 −2 python/core/3d/qgsabstract3drenderer.sip
  3. +0 −6 python/core/qgsmaplayer.sip
  4. +0 −1 src/3d/CMakeLists.txt
  5. +0 −7 src/3d/chunks/qgschunkloader_p.cpp
  6. +2 −2 src/3d/chunks/qgschunkloader_p.h
  7. +0 −4 src/3d/chunks/qgschunkqueuejob_p.cpp
  8. +0 −2 src/3d/chunks/qgschunkqueuejob_p.h
  9. +4 −5 src/3d/qgs3dutils.cpp
  10. +3 −1 src/3d/qgstilingscheme.h
  11. +0 −4 src/3d/qgsvectorlayer3drenderer.cpp
  12. +4 −4 src/3d/qgsvectorlayer3drenderer.h
  13. +3 −0 src/3d/shaders/light.inc.frag
  14. +3 −2 src/3d/symbols/qgsabstract3dsymbol.h
  15. +14 −14 src/3d/symbols/qgsline3dsymbol.cpp
  16. +1 −1 src/3d/symbols/qgsline3dsymbol.h
  17. +1 −1 src/3d/symbols/qgsline3dsymbol_p.cpp
  18. +8 −8 src/3d/symbols/qgspoint3dsymbol.cpp
  19. +1 −1 src/3d/symbols/qgspoint3dsymbol.h
  20. +4 −0 src/3d/symbols/qgspoint3dsymbol_p.cpp
  21. +12 −12 src/3d/symbols/qgspolygon3dsymbol.cpp
  22. +1 −1 src/3d/symbols/qgspolygon3dsymbol.h
  23. +7 −7 src/3d/symbols/qgspolygon3dsymbol_p.cpp
  24. +2 −2 src/3d/terrain/qgsdemterraingenerator.h
  25. +4 −8 src/3d/terrain/qgsdemterraintilegeometry_p.cpp
  26. +1 −1 src/3d/terrain/qgsdemterraintilegeometry_p.h
  27. +0 −4 src/3d/terrain/qgsdemterraintileloader_p.cpp
  28. +0 −4 src/3d/terrain/qgsflatterraingenerator.cpp
  29. +3 −3 src/3d/terrain/qgsflatterraingenerator.h
  30. +0 −4 src/3d/terrain/qgsterrainentity_p.cpp
  31. +5 −5 src/3d/terrain/qgsterraingenerator.h
  32. +5 −5 src/3d/terrain/quantizedmeshgeometry.h
  33. +1 −1 src/3d/terrain/quantizedmeshterraingenerator.h
  34. +0 −8 src/core/3d/qgs3drendererregistry.cpp
  35. +2 −2 src/core/3d/qgs3drendererregistry.h
  36. +0 −4 src/core/3d/qgsabstract3drenderer.cpp
  37. +1 −1 src/core/3d/qgsabstract3drenderer.h
  38. +0 −2 src/core/qgsmaplayer.h
@@ -1,7 +1,7 @@
/************************************************************************
* This file has been generated automatically from *
* *
* src/core/./3d/qgs3drendererregistry.h *
* src/core/3d/qgs3drendererregistry.h *
* *
* Do not edit manually ! Edit header and run scripts/sipify.pl again *
************************************************************************/
@@ -90,7 +90,7 @@ class Qgs3DRendererRegistry
/************************************************************************
* This file has been generated automatically from *
* *
* src/core/./3d/qgs3drendererregistry.h *
* src/core/3d/qgs3drendererregistry.h *
* *
* Do not edit manually ! Edit header and run scripts/sipify.pl again *
************************************************************************/
@@ -1,7 +1,7 @@
/************************************************************************
* This file has been generated automatically from *
* *
* src/core/./3d/qgsabstract3drenderer.h *
* src/core/3d/qgsabstract3drenderer.h *
* *
* Do not edit manually ! Edit header and run scripts/sipify.pl again *
************************************************************************/
@@ -59,7 +59,7 @@ Resolves references to other objects - second phase of loading - after readXml()
/************************************************************************
* This file has been generated automatically from *
* *
* src/core/./3d/qgsabstract3drenderer.h *
* src/core/3d/qgsabstract3drenderer.h *
* *
* Do not edit manually ! Edit header and run scripts/sipify.pl again *
************************************************************************/
@@ -763,18 +763,12 @@ Return pointer to layer's undo stack
void setRenderer3D( QgsAbstract3DRenderer *renderer /Transfer/ );
%Docstring
Sets 3D renderer for the layer. Takes ownership of the renderer.
.. note::

not available in Python bindings
.. versionadded:: 3.0
%End

QgsAbstract3DRenderer *renderer3D() const;
%Docstring
Returns 3D renderer associated with the layer. May be null.
.. note::

not available in Python bindings
.. versionadded:: 3.0
:rtype: QgsAbstract3DRenderer
%End
@@ -59,7 +59,6 @@ SET(QGIS_3D_MOC_HDRS

terrain/qgsdemterraintileloader_p.h
terrain/qgsterrainentity_p.h
terrain/qgsterraingenerator.h
terrain/qgsterraintexturegenerator_p.h
terrain/qgsterraintextureimage_p.h
terrain/qgsterraintileentity_p.h
@@ -2,12 +2,5 @@

///@cond PRIVATE

QgsChunkLoader::~QgsChunkLoader()
{
}

QgsChunkLoaderFactory::~QgsChunkLoaderFactory()
{
}

/// @endcond
@@ -28,7 +28,7 @@ class QgsChunkLoader : public QgsChunkQueueJob
{
}

virtual ~QgsChunkLoader();
virtual ~QgsChunkLoader() = default;

//! Run in main thread to use loaded data.
//! Returns entity attached to the given parent entity in disabled state
@@ -44,7 +44,7 @@ class QgsChunkLoader : public QgsChunkQueueJob
class QgsChunkLoaderFactory
{
public:
virtual ~QgsChunkLoaderFactory();
virtual ~QgsChunkLoaderFactory() = default;

//! Creates loader for the given chunk node. Ownership of the returned is passed to the caller.
virtual QgsChunkLoader *createChunkLoader( QgsChunkNode *node ) const = 0;
@@ -2,10 +2,6 @@

///@cond PRIVATE

QgsChunkQueueJob::~QgsChunkQueueJob()
{
}

void QgsChunkQueueJob::cancel()
{
// TODO: what to do...
@@ -43,8 +43,6 @@ class QgsChunkQueueJob : public QObject
{
}

virtual ~QgsChunkQueueJob();

//! Returns chunk node of this job
QgsChunkNode *chunk() { return mNode; }

@@ -108,7 +108,7 @@ bool Qgs3DUtils::clampAltitudes( QgsPolygonV2 *polygon, AltitudeClamping altClam
centroid = polygon->centroid();

QgsCurve *curve = const_cast<QgsCurve *>( polygon->exteriorRing() );
QgsLineString *lineString = dynamic_cast<QgsLineString *>( curve );
QgsLineString *lineString = qgsgeometry_cast<QgsLineString *>( curve );
if ( !lineString )
return false;

@@ -117,7 +117,7 @@ bool Qgs3DUtils::clampAltitudes( QgsPolygonV2 *polygon, AltitudeClamping altClam
for ( int i = 0; i < polygon->numInteriorRings(); ++i )
{
QgsCurve *curve = const_cast<QgsCurve *>( polygon->interiorRing( i ) );
QgsLineString *lineString = dynamic_cast<QgsLineString *>( curve );
QgsLineString *lineString = qgsgeometry_cast<QgsLineString *>( curve );
if ( !lineString )
return false;

@@ -156,10 +156,9 @@ QList<QVector3D> Qgs3DUtils::positions( const Qgs3DMapSettings &map, QgsVectorLa
if ( f.geometry().isNull() )
continue;

QgsAbstractGeometry *g = f.geometry().geometry();
if ( QgsWkbTypes::flatType( g->wkbType() ) == QgsWkbTypes::Point )
const QgsAbstractGeometry *g = f.geometry().geometry();
if ( const QgsPoint *pt = qgsgeometry_cast< const QgsPoint *>( g ) )
{
QgsPoint *pt = static_cast<QgsPoint *>( g );
// TODO: use Z coordinates if the point is 3D
float h = map.terrainGenerator()->heightAt( pt->x(), pt->y(), map ) * map.terrainVerticalScale();
positions.append( QVector3D( pt->x() - map.originX(), h, -( pt->y() - map.originY() ) ) );
@@ -1,6 +1,8 @@
#ifndef QGSTILINGSCHEME_H
#define QGSTILINGSCHEME_H

#include "qgis_3d.h"

#include <qgscoordinatereferencesystem.h>
#include <qgspointxy.h>

@@ -11,7 +13,7 @@ class QgsRectangle;
* The origin (tile [0,0]) is in bottom-left corner.
* \since QGIS 3.0
*/
class QgsTilingScheme
class _3D_EXPORT QgsTilingScheme
{
public:
//! Creates invalid tiling scheme
@@ -32,10 +32,6 @@ QgsVectorLayer3DRenderer::QgsVectorLayer3DRenderer( QgsAbstract3DSymbol *s )
{
}

QgsVectorLayer3DRenderer::~QgsVectorLayer3DRenderer()
{
}

QgsVectorLayer3DRenderer *QgsVectorLayer3DRenderer::clone() const
{
QgsVectorLayer3DRenderer *r = new QgsVectorLayer3DRenderer( mSymbol ? mSymbol->clone() : nullptr );
@@ -28,7 +28,7 @@ class _3D_EXPORT QgsVectorLayer3DRendererMetadata : public Qgs3DRendererAbstract
QgsVectorLayer3DRendererMetadata();

//! Creates an instance of a 3D renderer based on a DOM element with renderer configuration
virtual QgsAbstract3DRenderer *createRenderer( QDomElement &elem, const QgsReadWriteContext &context ) override;
virtual QgsAbstract3DRenderer *createRenderer( QDomElement &elem, const QgsReadWriteContext &context ) override SIP_FACTORY;
};


@@ -42,7 +42,7 @@ class _3D_EXPORT QgsVectorLayer3DRenderer : public QgsAbstract3DRenderer
public:
//! Takes ownership of the symbol object
explicit QgsVectorLayer3DRenderer( QgsAbstract3DSymbol *s SIP_TRANSFER = nullptr );
~QgsVectorLayer3DRenderer();
~QgsVectorLayer3DRenderer() = default;

//! Sets vector layer associated with the renderer
void setLayer( QgsVectorLayer *layer );
@@ -55,8 +55,8 @@ class _3D_EXPORT QgsVectorLayer3DRenderer : public QgsAbstract3DRenderer
const QgsAbstract3DSymbol *symbol() const;

QString type() const override { return "vector"; }
QgsVectorLayer3DRenderer *clone() const override;
Qt3DCore::QEntity *createEntity( const Qgs3DMapSettings &map ) const override;
QgsVectorLayer3DRenderer *clone() const override SIP_FACTORY;
Qt3DCore::QEntity *createEntity( const Qgs3DMapSettings &map ) const override SIP_FACTORY;

void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
@@ -1,3 +1,6 @@

// copy of light.inc.frag from qt3d extras

const int MAX_LIGHTS = 8;
const int TYPE_POINT = 0;
const int TYPE_DIRECTIONAL = 1;
@@ -2,6 +2,7 @@
#define QGSABSTRACT3DSYMBOL_H

#include "qgis_3d.h"
#include "qgis_sip.h"

class QDomElement;
class QString;
@@ -20,12 +21,12 @@ class QgsReadWriteContext;
class _3D_EXPORT QgsAbstract3DSymbol
{
public:
virtual ~QgsAbstract3DSymbol() {}
virtual ~QgsAbstract3DSymbol() = default;

//! Returns identifier of symbol type. Each 3D symbol implementation should return a different type.
virtual QString type() const = 0;
//! Returns a new instance of the symbol with the same settings
virtual QgsAbstract3DSymbol *clone() const = 0;
virtual QgsAbstract3DSymbol *clone() const = 0 SIP_FACTORY;

//! Writes symbol configuration to the given DOM element
virtual void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const = 0;
@@ -21,15 +21,15 @@ void QgsLine3DSymbol::writeXml( QDomElement &elem, const QgsReadWriteContext &co

QDomDocument doc = elem.ownerDocument();

QDomElement elemDataProperties = doc.createElement( "data" );
elemDataProperties.setAttribute( "alt-clamping", Qgs3DUtils::altClampingToString( mAltClamping ) );
elemDataProperties.setAttribute( "alt-binding", Qgs3DUtils::altBindingToString( mAltBinding ) );
elemDataProperties.setAttribute( "height", mHeight );
elemDataProperties.setAttribute( "extrusion-height", mExtrusionHeight );
elemDataProperties.setAttribute( "width", mWidth );
QDomElement elemDataProperties = doc.createElement( QStringLiteral( "data" ) );
elemDataProperties.setAttribute( QStringLiteral( "alt-clamping" ), Qgs3DUtils::altClampingToString( mAltClamping ) );
elemDataProperties.setAttribute( QStringLiteral( "alt-binding" ), Qgs3DUtils::altBindingToString( mAltBinding ) );
elemDataProperties.setAttribute( QStringLiteral( "height" ), mHeight );
elemDataProperties.setAttribute( QStringLiteral( "extrusion-height" ), mExtrusionHeight );
elemDataProperties.setAttribute( QStringLiteral( "width" ), mWidth );
elem.appendChild( elemDataProperties );

QDomElement elemMaterial = doc.createElement( "material" );
QDomElement elemMaterial = doc.createElement( QStringLiteral( "material" ) );
mMaterial.writeXml( elemMaterial );
elem.appendChild( elemMaterial );
}
@@ -38,13 +38,13 @@ void QgsLine3DSymbol::readXml( const QDomElement &elem, const QgsReadWriteContex
{
Q_UNUSED( context );

QDomElement elemDataProperties = elem.firstChildElement( "data" );
mAltClamping = Qgs3DUtils::altClampingFromString( elemDataProperties.attribute( "alt-clamping" ) );
mAltBinding = Qgs3DUtils::altBindingFromString( elemDataProperties.attribute( "alt-binding" ) );
mHeight = elemDataProperties.attribute( "height" ).toFloat();
mExtrusionHeight = elemDataProperties.attribute( "extrusion-height" ).toFloat();
mWidth = elemDataProperties.attribute( "width" ).toFloat();
QDomElement elemDataProperties = elem.firstChildElement( QStringLiteral( "data" ) );
mAltClamping = Qgs3DUtils::altClampingFromString( elemDataProperties.attribute( QStringLiteral( "alt-clamping" ) ) );
mAltBinding = Qgs3DUtils::altBindingFromString( elemDataProperties.attribute( QStringLiteral( "alt-binding" ) ) );
mHeight = elemDataProperties.attribute( QStringLiteral( "height" ) ).toFloat();
mExtrusionHeight = elemDataProperties.attribute( QStringLiteral( "extrusion-height" ) ).toFloat();
mWidth = elemDataProperties.attribute( QStringLiteral( "width" ) ).toFloat();

QDomElement elemMaterial = elem.firstChildElement( "material" );
QDomElement elemMaterial = elem.firstChildElement( QStringLiteral( "material" ) );
mMaterial.readXml( elemMaterial );
}
@@ -18,7 +18,7 @@ class _3D_EXPORT QgsLine3DSymbol : public QgsAbstract3DSymbol
QgsLine3DSymbol();

QString type() const override { return "line"; }
QgsAbstract3DSymbol *clone() const override;
QgsAbstract3DSymbol *clone() const override SIP_FACTORY;

void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
@@ -100,7 +100,7 @@ Qt3DRender::QGeometryRenderer *QgsLine3DSymbolEntityNode::renderer( const Qgs3DM
if ( QgsWkbTypes::isCurvedType( geom.geometry()->wkbType() ) )
geom = QgsGeometry( geom.geometry()->segmentize() );

QgsAbstractGeometry *g = geom.geometry();
const QgsAbstractGeometry *g = geom.geometry();

QgsGeos engine( g );
QgsAbstractGeometry *buffered = engine.buffer( symbol.width() / 2., nSegments, endCapStyle, joinStyle, mitreLimit ); // factory
@@ -17,31 +17,31 @@ void QgsPoint3DSymbol::writeXml( QDomElement &elem, const QgsReadWriteContext &c
{
QDomDocument doc = elem.ownerDocument();

QDomElement elemMaterial = doc.createElement( "material" );
QDomElement elemMaterial = doc.createElement( QStringLiteral( "material" ) );
mMaterial.writeXml( elemMaterial );
elem.appendChild( elemMaterial );

QVariantMap shapePropertiesCopy( mShapeProperties );
shapePropertiesCopy["model"] = QVariant( context.pathResolver().writePath( shapePropertiesCopy["model"].toString() ) );

QDomElement elemShapeProperties = doc.createElement( "shape-properties" );
QDomElement elemShapeProperties = doc.createElement( QStringLiteral( "shape-properties" ) );
elemShapeProperties.appendChild( QgsXmlUtils::writeVariant( shapePropertiesCopy, doc ) );
elem.appendChild( elemShapeProperties );

QDomElement elemTransform = doc.createElement( "transform" );
elemTransform.setAttribute( "matrix", Qgs3DUtils::matrix4x4toString( mTransform ) );
QDomElement elemTransform = doc.createElement( QStringLiteral( "transform" ) );
elemTransform.setAttribute( QStringLiteral( "matrix" ), Qgs3DUtils::matrix4x4toString( mTransform ) );
elem.appendChild( elemTransform );
}

void QgsPoint3DSymbol::readXml( const QDomElement &elem, const QgsReadWriteContext &context )
{
QDomElement elemMaterial = elem.firstChildElement( "material" );
QDomElement elemMaterial = elem.firstChildElement( QStringLiteral( "material" ) );
mMaterial.readXml( elemMaterial );

QDomElement elemShapeProperties = elem.firstChildElement( "shape-properties" );
QDomElement elemShapeProperties = elem.firstChildElement( QStringLiteral( "shape-properties" ) );
mShapeProperties = QgsXmlUtils::readVariant( elemShapeProperties.firstChildElement() ).toMap();
mShapeProperties["model"] = QVariant( context.pathResolver().readPath( mShapeProperties["model"].toString() ) );

QDomElement elemTransform = elem.firstChildElement( "transform" );
mTransform = Qgs3DUtils::stringToMatrix4x4( elemTransform.attribute( "matrix" ) );
QDomElement elemTransform = elem.firstChildElement( QStringLiteral( "transform" ) );
mTransform = Qgs3DUtils::stringToMatrix4x4( elemTransform.attribute( QStringLiteral( "matrix" ) ) );
}
@@ -19,7 +19,7 @@ class _3D_EXPORT QgsPoint3DSymbol : public QgsAbstract3DSymbol
QgsPoint3DSymbol();

QString type() const override { return "point"; }
QgsAbstract3DSymbol *clone() const override;
QgsAbstract3DSymbol *clone() const override SIP_FACTORY;

void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
@@ -138,6 +138,7 @@ void QgsPoint3DSymbolInstancedEntityFactory::addEntityForSelectedPoints( const Q
QgsFeatureRequest req;
req.setDestinationCrs( map.crs() );
req.setFilterFids( layer->selectedFeatureIds() );
req.setSubsetOfAttributes( QgsAttributeList() );

// build the entity
QgsPoint3DSymbolInstancedEntityNode *entity = new QgsPoint3DSymbolInstancedEntityNode( map, layer, symbol, req );
@@ -153,6 +154,7 @@ void QgsPoint3DSymbolInstancedEntityFactory::addEntityForNotSelectedPoints( cons
// build the feature request to select features
QgsFeatureRequest req;
req.setDestinationCrs( map.crs() );
req.setSubsetOfAttributes( QgsAttributeList() );

QgsFeatureIds notSelected = layer->allFeatureIds();
notSelected.subtract( layer->selectedFeatureIds() );
@@ -297,6 +299,7 @@ void QgsPoint3DSymbolModelEntityFactory::addEntitiesForSelectedPoints( const Qgs
{
QgsFeatureRequest req;
req.setDestinationCrs( map.crs() );
req.setSubsetOfAttributes( QgsAttributeList() );
req.setFilterFids( layer->selectedFeatureIds() );

addMeshEntities( map, layer, req, symbol, parent, true );
@@ -309,6 +312,7 @@ void QgsPoint3DSymbolModelEntityFactory::addEntitiesForNotSelectedPoints( const
// build the feature request to select features
QgsFeatureRequest req;
req.setDestinationCrs( map.crs() );
req.setSubsetOfAttributes( QgsAttributeList() );
QgsFeatureIds notSelected = layer->allFeatureIds();
notSelected.subtract( layer->selectedFeatureIds() );
req.setFilterFids( notSelected );

0 comments on commit 518dd17

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