Cleanup diffuse texture API

Seperate the calls for whether a user has enabled diffuse texture from API for determining whether diffuse texture should be used during rendering. And don't try to use diffuse textures when the texture path hasn't yet been set, which causes a flood of Qt warnings...
qgis · Jul 10, 2020 · eff171a · eff171a
1 parent 402ce03
commit eff171a
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 25 deletions.
diff --git a/python/3d/auto_generated/qgsphongmaterialsettings.sip.in b/python/3d/auto_generated/qgsphongmaterialsettings.sip.in
@@ -46,13 +46,47 @@ Returns specular color component
 %Docstring
 Returns shininess of the surface
 %End
-    bool isUsingDiffuseTexture() const;
+
+    bool diffuseTextureEnabled() const;
+%Docstring
+Returns whether the diffuse texture is used.
+
+.. note::
+
+   Diffuse textures will only be used at render time if :py:func:`~QgsPhongMaterialSettings.diffuseTextureEnabled` is ``True``
+   and a :py:func:`~QgsPhongMaterialSettings.texturePath` is non-empty.
+
+.. seealso:: :py:func:`shouldUseDiffuseTexture`
+
+.. seealso:: :py:func:`setDiffuseTextureEnabled`
+
+.. seealso:: :py:func:`texturePath`
+%End
+
+    bool shouldUseDiffuseTexture() const;
 %Docstring
-Returns whether the diffuse texture is used
+Returns whether the diffuse texture should be used during rendering.
+
+Diffuse textures will only be used at render time if :py:func:`~QgsPhongMaterialSettings.diffuseTextureEnabled` is ``True``
+and a :py:func:`~QgsPhongMaterialSettings.texturePath` is non-empty.
+
+.. seealso:: :py:func:`diffuseTextureEnabled`
+
+.. seealso:: :py:func:`texturePath`
 %End
+
     QString texturePath() const;
 %Docstring
-Returns the diffuse texture path
+Returns the diffuse texture path.
+
+.. note::
+
+   Diffuse textures will only be used at render time if :py:func:`~QgsPhongMaterialSettings.diffuseTextureEnabled` is ``True``
+   and a :py:func:`~QgsPhongMaterialSettings.texturePath` is non-empty.
+
+.. seealso:: :py:func:`setTexturePath`
+
+.. seealso:: :py:func:`diffuseTextureEnabled`
 %End
 
     float textureScale() const;
@@ -83,13 +117,33 @@ Sets specular color component
 %Docstring
 Sets shininess of the surface
 %End
-    void setUseTexture( bool used );
+
+    void setDiffuseTextureEnabled( bool used );
 %Docstring
-Sets whether the diffuse texture will be used
+Sets whether the diffuse texture is enabled.
+
+.. note::
+
+   Diffuse textures will only be used at render time if :py:func:`~QgsPhongMaterialSettings.diffuseTextureEnabled` is ``True``
+   and a :py:func:`~QgsPhongMaterialSettings.texturePath` is non-empty.
+
+.. seealso:: :py:func:`diffuseTextureEnabled`
+
+.. seealso:: :py:func:`setTexturePath`
 %End
-    void setTexturePath( QString texturePath );
+
+    void setTexturePath( const QString &path );
 %Docstring
-Sets the path of the texture
+Sets the ``path`` of the texture.
+
+.. note::
+
+   Diffuse textures will only be used at render time if :py:func:`~QgsPhongMaterialSettings.diffuseTextureEnabled` is ``True``
+   and a :py:func:`~QgsPhongMaterialSettings.texturePath` is non-empty.
+
+.. seealso:: :py:func:`texturePath`
+
+.. seealso:: :py:func:`setDiffuseTextureEnabled`
 %End
 
     void setTextureScale( float scale );

diff --git a/src/3d/qgsphongmaterialsettings.cpp b/src/3d/qgsphongmaterialsettings.cpp
@@ -24,7 +24,7 @@ void QgsPhongMaterialSettings::readXml( const QDomElement &elem )
   mDiffuse = QgsSymbolLayerUtils::decodeColor( elem.attribute( QStringLiteral( "diffuse" ) ) );
   mSpecular = QgsSymbolLayerUtils::decodeColor( elem.attribute( QStringLiteral( "specular" ) ) );
   mShininess = elem.attribute( QStringLiteral( "shininess" ) ).toFloat();
-  mIsUsingDiffuseTexture = elem.attribute( QStringLiteral( "is_using_diffuse_texture" ), QStringLiteral( "0" ) ).toInt();
+  mDiffuseTextureEnabled = elem.attribute( QStringLiteral( "is_using_diffuse_texture" ), QStringLiteral( "0" ) ).toInt();
   mTexturePath = elem.attribute( QStringLiteral( "diffuse_texture_path" ), QString() );
   mTextureScale = elem.attribute( QStringLiteral( "texture_scale" ), QString( "1.0" ) ).toFloat();
   mTextureRotation = elem.attribute( QStringLiteral( "texture-rotation" ), QString( "0.0" ) ).toFloat();
@@ -36,7 +36,7 @@ void QgsPhongMaterialSettings::writeXml( QDomElement &elem ) const
   elem.setAttribute( QStringLiteral( "diffuse" ), QgsSymbolLayerUtils::encodeColor( mDiffuse ) );
   elem.setAttribute( QStringLiteral( "specular" ), QgsSymbolLayerUtils::encodeColor( mSpecular ) );
   elem.setAttribute( QStringLiteral( "shininess" ), mShininess );
-  elem.setAttribute( QStringLiteral( "is_using_diffuse_texture" ), mIsUsingDiffuseTexture );
+  elem.setAttribute( QStringLiteral( "is_using_diffuse_texture" ), mDiffuseTextureEnabled );
   elem.setAttribute( QStringLiteral( "diffuse_texture_path" ), mTexturePath );
   elem.setAttribute( QStringLiteral( "texture_scale" ), mTextureScale );
   elem.setAttribute( QStringLiteral( "texture-rotation" ), mTextureRotation );

diff --git a/src/3d/qgsphongmaterialsettings.h b/src/3d/qgsphongmaterialsettings.h
@@ -45,9 +45,39 @@ class _3D_EXPORT QgsPhongMaterialSettings
     QColor specular() const { return mSpecular; }
     //! Returns shininess of the surface
     float shininess() const { return mShininess; }
-    //! Returns whether the diffuse texture is used
-    bool isUsingDiffuseTexture() const { return mIsUsingDiffuseTexture; }
-    //! Returns the diffuse texture path
+
+    /**
+     * Returns whether the diffuse texture is used.
+     *
+     * \note Diffuse textures will only be used at render time if diffuseTextureEnabled() is TRUE
+     * and a texturePath() is non-empty.
+     *
+     * \see shouldUseDiffuseTexture()
+     * \see setDiffuseTextureEnabled()
+     * \see texturePath()
+     */
+    bool diffuseTextureEnabled() const { return mDiffuseTextureEnabled; }
+
+    /**
+     * Returns whether the diffuse texture should be used during rendering.
+     *
+     * Diffuse textures will only be used at render time if diffuseTextureEnabled() is TRUE
+     * and a texturePath() is non-empty.
+     *
+     * \see diffuseTextureEnabled()
+     * \see texturePath()
+     */
+    bool shouldUseDiffuseTexture() const { return mDiffuseTextureEnabled && !mTexturePath.isEmpty(); }
+
+    /**
+     * Returns the diffuse texture path.
+     *
+     * \note Diffuse textures will only be used at render time if diffuseTextureEnabled() is TRUE
+     * and a texturePath() is non-empty.
+     *
+     * \see setTexturePath()
+     * \see diffuseTextureEnabled()
+     */
     QString texturePath() const { return mTexturePath; }
 
     /**
@@ -68,10 +98,28 @@ class _3D_EXPORT QgsPhongMaterialSettings
     void setSpecular( const QColor &specular ) { mSpecular = specular; }
     //! Sets shininess of the surface
     void setShininess( float shininess ) { mShininess = shininess; }
-    //! Sets whether the diffuse texture will be used
-    void setUseTexture( bool used ) { mIsUsingDiffuseTexture = used; }
-    //! Sets the path of the texture
-    void setTexturePath( QString texturePath ) { mTexturePath = texturePath; }
+
+    /**
+     * Sets whether the diffuse texture is enabled.
+     *
+     * \note Diffuse textures will only be used at render time if diffuseTextureEnabled() is TRUE
+     * and a texturePath() is non-empty.
+     *
+     * \see diffuseTextureEnabled()
+     * \see setTexturePath()
+     */
+    void setDiffuseTextureEnabled( bool used ) { mDiffuseTextureEnabled = used; }
+
+    /**
+     * Sets the \a path of the texture.
+     *
+     * \note Diffuse textures will only be used at render time if diffuseTextureEnabled() is TRUE
+     * and a texturePath() is non-empty.
+     *
+     * \see texturePath()
+     * \see setDiffuseTextureEnabled()
+     */
+    void setTexturePath( const QString &path ) { mTexturePath = path; }
 
     /**
      * Sets the texture scale
@@ -94,7 +142,7 @@ class _3D_EXPORT QgsPhongMaterialSettings
              mDiffuse == other.mDiffuse &&
              mSpecular == other.mSpecular &&
              mShininess == other.mShininess &&
-             mIsUsingDiffuseTexture == other.mIsUsingDiffuseTexture &&
+             mDiffuseTextureEnabled == other.mDiffuseTextureEnabled &&
              mTexturePath == other.mTexturePath &&
              mTextureScale == other.mTextureScale &&
              mTextureRotation == other.mTextureRotation;
@@ -105,7 +153,7 @@ class _3D_EXPORT QgsPhongMaterialSettings
     QColor mDiffuse{ QColor::fromRgbF( 0.7f, 0.7f, 0.7f, 1.0f ) };
     QColor mSpecular{ QColor::fromRgbF( 1.0f, 1.0f, 1.0f, 1.0f ) };
     float mShininess = 0.0f;
-    bool mIsUsingDiffuseTexture{ false };
+    bool mDiffuseTextureEnabled{ false };
     QString mTexturePath;
     float mTextureScale{ 1.0f };
     float mTextureRotation{ 0.0f };

diff --git a/src/3d/symbols/qgsmesh3dsymbol_p.cpp b/src/3d/symbols/qgsmesh3dsymbol_p.cpp
@@ -137,7 +137,7 @@ Qt3DRender::QGeometryRenderer *QgsMesh3DSymbolEntityNode::renderer( const Qgs3DM
   // Polygons from mesh are already triangles, but
   // call QgsTessellatedPolygonGeometry to
   // use symbol settings for back faces, normals, etc
-  mGeometry = new QgsTessellatedPolygonGeometry( true, false, symbol.addBackFaces(), symbol.material().isUsingDiffuseTexture() );
+  mGeometry = new QgsTessellatedPolygonGeometry( true, false, symbol.addBackFaces(), symbol.material().shouldUseDiffuseTexture() );
   QList<float> extrusionHeightPerPolygon;
   mGeometry->setPolygons( polygons, fids, origin, 0.0, extrusionHeightPerPolygon );
 

diff --git a/src/3d/symbols/qgspolygon3dsymbol_p.cpp b/src/3d/symbols/qgspolygon3dsymbol_p.cpp
@@ -86,8 +86,8 @@ bool QgsPolygon3DSymbolHandler::prepare( const Qgs3DRenderContext &context, QSet
   outEdges.withAdjacency = true;
   outEdges.init( mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), mSymbol.height(), &context.map() );
 
-  outNormal.tessellator.reset( new QgsTessellator( context.map().origin().x(), context.map().origin().y(), true, mSymbol.invertNormals(), mSymbol.addBackFaces(), false, mSymbol.material().isUsingDiffuseTexture(), mSymbol.renderedFacade(), mSymbol.material().textureRotation() ) );
-  outSelected.tessellator.reset( new QgsTessellator( context.map().origin().x(), context.map().origin().y(), true, mSymbol.invertNormals(), mSymbol.addBackFaces(), false, mSymbol.material().isUsingDiffuseTexture(), mSymbol.renderedFacade(), mSymbol.material().textureRotation() ) );
+  outNormal.tessellator.reset( new QgsTessellator( context.map().origin().x(), context.map().origin().y(), true, mSymbol.invertNormals(), mSymbol.addBackFaces(), false, mSymbol.material().shouldUseDiffuseTexture(), mSymbol.renderedFacade(), mSymbol.material().textureRotation() ) );
+  outSelected.tessellator.reset( new QgsTessellator( context.map().origin().x(), context.map().origin().y(), true, mSymbol.invertNormals(), mSymbol.addBackFaces(), false, mSymbol.material().shouldUseDiffuseTexture(), mSymbol.renderedFacade(), mSymbol.material().textureRotation() ) );
 
   QSet<QString> attrs = mSymbol.dataDefinedProperties().referencedFields( context.expressionContext() );
   attributeNames.unite( attrs );
@@ -231,7 +231,7 @@ void QgsPolygon3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const Qgs
   QByteArray data( ( const char * )out.tessellator->data().constData(), out.tessellator->data().count() * sizeof( float ) );
   int nVerts = data.count() / out.tessellator->stride();
 
-  QgsTessellatedPolygonGeometry *geometry = new QgsTessellatedPolygonGeometry( true, mSymbol.invertNormals(), mSymbol.addBackFaces(), mSymbol.material().isUsingDiffuseTexture() );
+  QgsTessellatedPolygonGeometry *geometry = new QgsTessellatedPolygonGeometry( true, mSymbol.invertNormals(), mSymbol.addBackFaces(), mSymbol.material().shouldUseDiffuseTexture() );
   geometry->setData( data, nVerts, out.triangleIndexFids, out.triangleIndexStartingIndices );
 
   Qt3DRender::QGeometryRenderer *renderer = new Qt3DRender::QGeometryRenderer;
@@ -281,7 +281,7 @@ static void applyCullingMode( Qgs3DTypes::CullingMode cullingMode, Qt3DRender::Q
 Qt3DRender::QMaterial *QgsPolygon3DSymbolHandler::material( const QgsPolygon3DSymbol &symbol, bool isSelected, const Qgs3DRenderContext &context ) const
 {
   Qt3DRender::QMaterial *retMaterial = nullptr;
-  if ( symbol.material().isUsingDiffuseTexture() )
+  if ( symbol.material().shouldUseDiffuseTexture() )
   {
     QString textureFilePath = symbol.material().texturePath();
     Qt3DExtras::QDiffuseMapMaterial *material = new Qt3DExtras::QDiffuseMapMaterial;

diff --git a/src/app/3d/qgsphongmaterialwidget.cpp b/src/app/3d/qgsphongmaterialwidget.cpp
@@ -58,7 +58,7 @@ void QgsPhongMaterialWidget::setMaterial( const QgsPhongMaterialSettings &materi
   btnAmbient->setColor( material.ambient() );
   btnSpecular->setColor( material.specular() );
   spinShininess->setValue( material.shininess() );
-  useDiffuseCheckBox->setCheckState( material.isUsingDiffuseTexture() ? Qt::CheckState::Checked : Qt::CheckState::Unchecked );
+  useDiffuseCheckBox->setCheckState( material.diffuseTextureEnabled() ? Qt::CheckState::Checked : Qt::CheckState::Unchecked );
   textureFile->setFilePath( material.texturePath() );
   textureScaleSpinBox->setValue( material.textureScale() );
   textureRotationSpinBox->setValue( material.textureRotation() );
@@ -71,7 +71,7 @@ QgsPhongMaterialSettings QgsPhongMaterialWidget::material() const
   m.setAmbient( btnAmbient->color() );
   m.setSpecular( btnSpecular->color() );
   m.setShininess( spinShininess->value() );
-  m.useTexture( useDiffuseCheckBox->checkState() == Qt::CheckState::Checked );
+  m.setDiffuseTextureEnabled( useDiffuseCheckBox->checkState() == Qt::CheckState::Checked );
   m.setTexturePath( textureFile->filePath() );
   m.setTextureScale( textureScaleSpinBox->value() );
   m.setTextureRotation( textureRotationSpinBox->value() );