[3d][ui] Remove cryptic label-less checkbox for overriding point

model symbol material, and replace by a dedicated "Embedded Textures" entry in the material selection combo box This brings consistency in how materials are set for model symbols and avoids a confusing ui behavior with a disabled material widget and no immediate clues on how to enable these settings.
qgis · Nov 13, 2020 · 17a4b07 · 17a4b07
1 parent c214428
commit 17a4b07
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 46 deletions.
diff --git a/src/3d/symbols/qgspoint3dsymbol_p.cpp b/src/3d/symbols/qgspoint3dsymbol_p.cpp
@@ -413,7 +413,9 @@ void QgsModelPoint3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const
   }
   else
   {
-    if ( mSymbol->shapeProperties()[QStringLiteral( "overwriteMaterial" )].toBool() )
+    //  "overwriteMaterial" is a legacy setting indicating that non-embedded material should be used
+    if ( mSymbol->shapeProperties()[QStringLiteral( "overwriteMaterial" )].toBool()
+         || ( mSymbol->material() && mSymbol->material()->type() != QLatin1String( "null" ) ) )
     {
       addMeshEntities( context.map(), out.positions, mSymbol.get(), parent, false );
     }

diff --git a/src/app/3d/qgsmaterialwidget.cpp b/src/app/3d/qgsmaterialwidget.cpp
@@ -88,6 +88,12 @@ QgsAbstractMaterialSettings *QgsMaterialWidget::settings()
   return mCurrentSettings->clone();
 }
 
+void QgsMaterialWidget::setType( const QString &type )
+{
+  mMaterialTypeComboBox->setCurrentIndex( mMaterialTypeComboBox->findData( type ) );
+  materialTypeChanged();
+}
+
 void QgsMaterialWidget::materialTypeChanged()
 {
   std::unique_ptr< QgsAbstractMaterialSettings > currentSettings( settings() );

diff --git a/src/app/3d/qgsmaterialwidget.h b/src/app/3d/qgsmaterialwidget.h
@@ -41,6 +41,8 @@ class QgsMaterialWidget : public QWidget, private Ui::MaterialWidgetBase
     void setSettings( const QgsAbstractMaterialSettings *settings, QgsVectorLayer *layer );
     QgsAbstractMaterialSettings *settings();
 
+    void setType( const QString &type );
+
   signals:
 
     void changed();

diff --git a/src/app/3d/qgspoint3dsymbolwidget.cpp b/src/app/3d/qgspoint3dsymbolwidget.cpp
@@ -74,35 +74,19 @@ QgsPoint3DSymbolWidget::QgsPoint3DSymbolWidget( QWidget *parent )
   for ( QDoubleSpinBox *spinBox : constSpinWidgets )
     connect( spinBox, static_cast<void ( QDoubleSpinBox::* )( double )>( &QDoubleSpinBox::valueChanged ), this, &QgsPoint3DSymbolWidget::changed );
   connect( lineEditModel, &QgsAbstractFileContentSourceLineEdit::sourceChanged, this, &QgsPoint3DSymbolWidget::changed );
-  connect( cbOverwriteMaterial, static_cast<void ( QCheckBox::* )( int )>( &QCheckBox::stateChanged ), this, &QgsPoint3DSymbolWidget::onOverwriteMaterialChecked );
   connect( widgetMaterial, &QgsMaterialWidget::changed, this, &QgsPoint3DSymbolWidget::changed );
   connect( btnChangeSymbol, static_cast<void ( QgsSymbolButton::* )( )>( &QgsSymbolButton::changed ), this, &QgsPoint3DSymbolWidget::changed );
 
   // Sync between billboard height and TY
   connect( spinBillboardHeight, static_cast<void ( QDoubleSpinBox::* )( double )>( &QDoubleSpinBox::valueChanged ), spinTY,  &QDoubleSpinBox::setValue );
   connect( spinTY, static_cast<void ( QDoubleSpinBox::* )( double )>( &QDoubleSpinBox::valueChanged ), spinBillboardHeight,  &QDoubleSpinBox::setValue );
-
-  widgetMaterial->setTechnique( QgsMaterialSettingsRenderingTechnique::InstancedPoints );
 }
 
 Qgs3DSymbolWidget *QgsPoint3DSymbolWidget::create( QgsVectorLayer * )
 {
   return new QgsPoint3DSymbolWidget();
 }
 
-void QgsPoint3DSymbolWidget::onOverwriteMaterialChecked( int state )
-{
-  if ( state == Qt::Checked )
-  {
-    widgetMaterial->setEnabled( true );
-  }
-  else
-  {
-    widgetMaterial->setEnabled( false );
-  }
-  emit changed();
-}
-
 void QgsPoint3DSymbolWidget::setSymbol( const QgsAbstract3DSymbol *symbol, QgsVectorLayer *layer )
 {
   const QgsPoint3DSymbol *pointSymbol = dynamic_cast< const QgsPoint3DSymbol *>( symbol );
@@ -114,8 +98,8 @@ void QgsPoint3DSymbolWidget::setSymbol( const QgsAbstract3DSymbol *symbol, QgsVe
   QVariantMap vm = pointSymbol->shapeProperties();
   int index = cboShape->findData( pointSymbol->shape() );
   cboShape->setCurrentIndex( index != -1 ? index : 1 );  // use cylinder by default if shape is not set
-  widgetMaterial->setEnabled( true );
   QgsMaterialSettingsRenderingTechnique technique = QgsMaterialSettingsRenderingTechnique::InstancedPoints;
+  bool forceNullMaterial = false;
   switch ( cboShape->currentIndex() )
   {
     case 0:  // sphere
@@ -143,9 +127,11 @@ void QgsPoint3DSymbolWidget::setSymbol( const QgsAbstract3DSymbol *symbol, QgsVe
     case 6:  // 3d model
     {
       lineEditModel->setSource( vm[QStringLiteral( "model" )].toString() );
-      bool overwriteMaterial = vm[QStringLiteral( "overwriteMaterial" )].toBool();
+      // "overwriteMaterial" is a legacy setting indicating that non-null material should be used
-      widgetMaterial->setEnabled( overwriteMaterial );
+      forceNullMaterial = ( vm.contains( QStringLiteral( "overwriteMaterial" ) ) && !vm[QStringLiteral( "overwriteMaterial" )].toBool() )
-      cbOverwriteMaterial->setChecked( overwriteMaterial );
+                          || !pointSymbol->material()
+                          || pointSymbol->material()->type() == QLatin1String( "null" );
+      technique = QgsMaterialSettingsRenderingTechnique::TrianglesFromModel;
       break;
     }
     case 7:  // billboard
@@ -160,6 +146,11 @@ void QgsPoint3DSymbolWidget::setSymbol( const QgsAbstract3DSymbol *symbol, QgsVe
   widgetMaterial->setSettings( pointSymbol->material(), layer );
   widgetMaterial->setTechnique( technique );
 
+  if ( forceNullMaterial )
+  {
+    widgetMaterial->setType( QStringLiteral( "null" ) );
+  }
+
   // decompose the transform matrix
   // assuming the last row has values [0 0 0 1]
   // see https://math.stackexchange.com/questions/237369/given-this-transformation-matrix-how-do-i-decompose-it-into-translation-rotati
@@ -220,7 +211,6 @@ QgsAbstract3DSymbol *QgsPoint3DSymbolWidget::symbol()
       break;
     case 6:  // 3d model
       vm[QStringLiteral( "model" )] = lineEditModel->source();
-      vm[QStringLiteral( "overwriteMaterial" )] = cbOverwriteMaterial->isChecked();
       break;
     case 7:  // billboard
       sym->setBillboardSymbol( btnChangeSymbol->clonedSymbol<QgsMarkerSymbol>() );
@@ -258,10 +248,9 @@ void QgsPoint3DSymbolWidget::onShapeChanged()
              << labelTopRadius << spinTopRadius
              << labelBottomRadius << spinBottomRadius
              << labelLength << spinLength
-             << labelModel << lineEditModel << cbOverwriteMaterial
+             << labelModel << lineEditModel
              << labelBillboardHeight << spinBillboardHeight << labelBillboardSymbol << btnChangeSymbol;
 
-  widgetMaterial->setEnabled( true );
   materialsGroupBox->show();
   transformationWidget->show();
   QList<QWidget *> activeWidgets;
@@ -287,8 +276,8 @@ void QgsPoint3DSymbolWidget::onShapeChanged()
       activeWidgets << labelRadius << spinRadius << labelMinorRadius << spinMinorRadius;
       break;
     case 6:  // 3d model
-      activeWidgets << labelModel << lineEditModel << cbOverwriteMaterial;
+      activeWidgets << labelModel << lineEditModel;
-      widgetMaterial->setEnabled( cbOverwriteMaterial->isChecked() );
+      technique = QgsMaterialSettingsRenderingTechnique::TrianglesFromModel;
       break;
     case 7:  // billboard
       activeWidgets << labelBillboardHeight << spinBillboardHeight << labelBillboardSymbol << btnChangeSymbol;
@@ -301,6 +290,12 @@ void QgsPoint3DSymbolWidget::onShapeChanged()
 
   widgetMaterial->setTechnique( technique );
 
+  if ( cboShape->currentIndex() == 6 )
+  {
+    // going from different shape -> model resets the material to the null type
+    widgetMaterial->setType( QStringLiteral( "null" ) );
+  }
+
   const auto constAllWidgets = allWidgets;
   for ( QWidget *w : constAllWidgets )
   {

diff --git a/src/app/3d/qgspoint3dsymbolwidget.h b/src/app/3d/qgspoint3dsymbolwidget.h
@@ -37,7 +37,6 @@ class QgsPoint3DSymbolWidget : public Qgs3DSymbolWidget, private Ui::Point3DSymb
 
   private slots:
     void onShapeChanged();
-    void onOverwriteMaterialChecked( int state );
 };
 
 #endif // QGSPOINT3DSYMBOLWIDGET_H
diff --git a/src/ui/3d/point3dsymbolwidget.ui b/src/ui/3d/point3dsymbolwidget.ui
@@ -14,7 +14,16 @@
    <string notr="true">Form</string>
   </property>
   <layout class="QFormLayout" name="formLayout">
-   <property name="margin">
+   <property name="leftMargin">
+    <number>0</number>
+   </property>
+   <property name="topMargin">
+    <number>0</number>
+   </property>
+   <property name="rightMargin">
+    <number>0</number>
+   </property>
+   <property name="bottomMargin">
     <number>0</number>
    </property>
    <item row="2" column="0" colspan="2">
@@ -182,7 +191,16 @@
       <string>Shading</string>
      </property>
      <layout class="QGridLayout" name="shadingGroup">
-      <property name="margin">
+      <property name="leftMargin">
+       <number>0</number>
+      </property>
+      <property name="topMargin">
+       <number>0</number>
+      </property>
+      <property name="rightMargin">
+       <number>0</number>
+      </property>
+      <property name="bottomMargin">
        <number>0</number>
       </property>
       <item row="0" column="0">
@@ -238,17 +256,7 @@
      <item row="7" column="1">
       <layout class="QHBoxLayout" name="horizontalLayout">
        <item>
-        <widget class="Qgs3DModelSourceLineEdit" name="lineEditModel"/>
+        <widget class="Qgs3DModelSourceLineEdit" name="lineEditModel" native="true"/>
-       </item>
-       <item>
-        <widget class="QCheckBox" name="cbOverwriteMaterial">
-         <property name="toolTip">
-          <string>Overwrite model material</string>
-         </property>
-         <property name="text">
-          <string/>
-         </property>
-        </widget>
        </item>
       </layout>
      </item>
@@ -404,6 +412,11 @@
   </layout>
  </widget>
  <customwidgets>
+  <customwidget>
+   <class>QgsDoubleSpinBox</class>
+   <extends>QDoubleSpinBox</extends>
+   <header>qgsdoublespinbox.h</header>
+  </customwidget>
   <customwidget>
    <class>Qgs3DModelSourceLineEdit</class>
    <extends>QWidget</extends>
@@ -416,11 +429,6 @@
    <header>qgsmaterialwidget.h</header>
    <container>1</container>
   </customwidget>
-  <customwidget>
-   <class>QgsDoubleSpinBox</class>
-   <extends>QDoubleSpinBox</extends>
-   <header>qgsdoublespinbox.h</header>
-  </customwidget>
   <customwidget>
    <class>QgsSymbolButton</class>
    <extends>QToolButton</extends>
@@ -442,7 +450,6 @@
   <tabstop>spinSize</tabstop>
   <tabstop>spinLength</tabstop>
   <tabstop>lineEditModel</tabstop>
-  <tabstop>cbOverwriteMaterial</tabstop>
   <tabstop>spinBillboardHeight</tabstop>
   <tabstop>btnChangeSymbol</tabstop>
   <tabstop>cboAltClamping</tabstop>