Skip to content

Commit

Permalink
qgsabstractmaterialsettings: Handle selection in addParametersToEffect
Browse files Browse the repository at this point in the history 
`addParametersToEffect()` method is used to set the material color
parameters if the material has been created without calling
`toMaterial()`. In pratice, this is only used by `QgsPoint3DSymbol`
for the Phong material.

This method does not handle the selection state of an entity. This
means that the color parameters of the material need to be changed
after `addParametersToEffect()` has been called. This is error prone.

This issue is fixed by adding a `QgsMaterialContext` to this
method. It allows to directly set the correct color parameters
according to the selection context.
  • Loading branch information
@ptitjano @nyalldawson
ptitjano authored and nyalldawson committed Jun 19, 2024
1 parent 5d38c5d commit 684a802
Show file tree
Hide file tree
Showing 15 changed files with 35 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/3d/materials/qgsabstractmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class _3D_EXPORT QgsAbstractMaterialSettings SIP_ABSTRACT
/**
* Adds parameters from the material to a destination \a effect.
*/
virtual void addParametersToEffect( Qt3DRender::QEffect *effect ) const = 0;
virtual void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const = 0;

// *INDENT-OFF*
//! Data definable properties.
Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgsgoochmaterialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ Qt3DRender::QMaterial *QgsGoochMaterialSettings::toMaterial( QgsMaterialSettings
return nullptr;
}

void QgsGoochMaterialSettings::addParametersToEffect( Qt3DRender::QEffect * ) const
void QgsGoochMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *, const QgsMaterialContext & ) const
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgsgoochmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class _3D_EXPORT QgsGoochMaterialSettings : public QgsAbstractMaterialSettings

#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;

QByteArray dataDefinedVertexColorsAsByte( const QgsExpressionContext &expressionContext ) const override;
int dataDefinedByteStride() const override;
Expand Down
3 changes: 1 addition & 2 deletions src/3d/materials/qgsmetalroughmaterialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ QMap<QString, QString> QgsMetalRoughMaterialSettings::toExportParameters() const
return parameters;
}

void QgsMetalRoughMaterialSettings::addParametersToEffect( Qt3DRender::QEffect * ) const
void QgsMetalRoughMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *, const QgsMaterialContext & ) const
{

}

2 changes: 1 addition & 1 deletion src/3d/materials/qgsmetalroughmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class _3D_EXPORT QgsMetalRoughMaterialSettings : public QgsAbstractMaterialSetti

#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override SIP_FACTORY;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;
#endif

// TODO c++20 - replace with = default
Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgsnullmaterialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ QMap<QString, QString> QgsNullMaterialSettings::toExportParameters() const
return parameters;
}

void QgsNullMaterialSettings::addParametersToEffect( Qt3DRender::QEffect * ) const
void QgsNullMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *, const QgsMaterialContext & ) const
{
}
2 changes: 1 addition & 1 deletion src/3d/materials/qgsnullmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class _3D_EXPORT QgsNullMaterialSettings : public QgsAbstractMaterialSettings
QMap<QString, QString> toExportParameters() const override;
#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override SIP_FACTORY;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;
#endif

};
Expand Down
17 changes: 10 additions & 7 deletions src/3d/materials/qgsphongmaterialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,19 @@ QMap<QString, QString> QgsPhongMaterialSettings::toExportParameters() const
return parameters;
}

void QgsPhongMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect ) const
void QgsPhongMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const
{
const QColor ambient = materialContext.isSelected() ? materialContext.selectionColor().darker() : mAmbient;
const QColor diffuse = materialContext.isSelected() ? materialContext.selectionColor() : mDiffuse;

Qt3DRender::QParameter *ambientParameter = new Qt3DRender::QParameter( QStringLiteral( "ambientColor" ),
QColor::fromRgbF( mAmbient.redF() * mAmbientCoefficient,
mAmbient.greenF() * mAmbientCoefficient,
mAmbient.blueF() * mAmbientCoefficient ) );
QColor::fromRgbF( ambient.redF() * mAmbientCoefficient,
ambient.greenF() * mAmbientCoefficient,
ambient.blueF() * mAmbientCoefficient ) );
Qt3DRender::QParameter *diffuseParameter = new Qt3DRender::QParameter( QStringLiteral( "diffuseColor" ),
QColor::fromRgbF( mDiffuse.redF() * mDiffuseCoefficient,
mDiffuse.greenF() * mDiffuseCoefficient,
mDiffuse.blueF() * mDiffuseCoefficient ) );
QColor::fromRgbF( diffuse.redF() * mDiffuseCoefficient,
diffuse.greenF() * mDiffuseCoefficient,
diffuse.blueF() * mDiffuseCoefficient ) );
Qt3DRender::QParameter *specularParameter = new Qt3DRender::QParameter( QStringLiteral( "specularColor" ),
QColor::fromRgbF( mSpecular.redF() * mSpecularCoefficient,
mSpecular.greenF() * mSpecularCoefficient,
Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgsphongmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class _3D_EXPORT QgsPhongMaterialSettings : public QgsAbstractMaterialSettings

#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override SIP_FACTORY;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;

QByteArray dataDefinedVertexColorsAsByte( const QgsExpressionContext &expressionContext ) const override;
int dataDefinedByteStride() const override;
Expand Down
6 changes: 4 additions & 2 deletions src/3d/materials/qgsphongtexturedmaterialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,11 @@ QMap<QString, QString> QgsPhongTexturedMaterialSettings::toExportParameters() co
return parameters;
}

void QgsPhongTexturedMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect ) const
void QgsPhongTexturedMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const
{
Qt3DRender::QParameter *ambientParameter = new Qt3DRender::QParameter( QStringLiteral( "ambientColor" ), mAmbient );
const QColor ambientColor = materialContext.isSelected() ? materialContext.selectionColor().darker() : mAmbient;

Qt3DRender::QParameter *ambientParameter = new Qt3DRender::QParameter( QStringLiteral( "ambientColor" ), ambientColor );
Qt3DRender::QParameter *specularParameter = new Qt3DRender::QParameter( QStringLiteral( "specularColor" ), mSpecular );
Qt3DRender::QParameter *shininessParameter = new Qt3DRender::QParameter( QStringLiteral( "shininess" ), mShininess );

Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgsphongtexturedmaterialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class _3D_EXPORT QgsPhongTexturedMaterialSettings : public QgsAbstractMaterialSe
void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override SIP_FACTORY;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;
#endif

// TODO c++20 - replace with = default
Expand Down
5 changes: 3 additions & 2 deletions src/3d/materials/qgssimplelinematerialsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,10 @@ QMap<QString, QString> QgsSimpleLineMaterialSettings::toExportParameters() const
return parameters;
}

void QgsSimpleLineMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect ) const
void QgsSimpleLineMaterialSettings::addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const
{
Qt3DRender::QParameter *ambientParameter = new Qt3DRender::QParameter( QStringLiteral( "ambientColor" ), mAmbient );
const QColor ambient = materialContext.isSelected() ? materialContext.selectionColor().darker() : mAmbient;
Qt3DRender::QParameter *ambientParameter = new Qt3DRender::QParameter( QStringLiteral( "ambientColor" ), ambient );
effect->addParameter( ambientParameter );
}

Expand Down
2 changes: 1 addition & 1 deletion src/3d/materials/qgssimplelinematerialsettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class _3D_EXPORT QgsSimpleLineMaterialSettings : public QgsAbstractMaterialSetti
void writeXml( QDomElement &elem, const QgsReadWriteContext &context ) const override;
#ifndef SIP_RUN
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique technique, const QgsMaterialContext &context ) const override SIP_FACTORY;
void addParametersToEffect( Qt3DRender::QEffect *effect ) const override;
void addParametersToEffect( Qt3DRender::QEffect *effect, const QgsMaterialContext &materialContext ) const override;
QByteArray dataDefinedVertexColorsAsByte( const QgsExpressionContext &expressionContext ) const override;
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
void applyDataDefinedToGeometry( Qt3DRender::QGeometry *geometry, int vertexCount, const QByteArray &data ) const override;
Expand Down
23 changes: 7 additions & 16 deletions src/3d/symbols/qgspoint3dsymbol_p.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class QgsInstancedPoint3DSymbolHandler : public QgsFeature3DHandler

private:

static Qt3DRender::QMaterial *material( const QgsPoint3DSymbol *symbol );
static Qt3DRender::QMaterial *material( const QgsPoint3DSymbol *symbol, const QgsMaterialContext &materialContext );
static Qt3DRender::QGeometryRenderer *renderer( const QgsPoint3DSymbol *symbol, const QVector<QVector3D> &positions );
static Qt3DQGeometry *symbolGeometry( const QgsPoint3DSymbol *symbol );

Expand Down Expand Up @@ -204,19 +204,10 @@ void QgsInstancedPoint3DSymbolHandler::finalize( Qt3DCore::QEntity *parent, cons
void QgsInstancedPoint3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const Qgs3DRenderContext &context, PointData &out, bool selected )
{
// build the default material
Qt3DRender::QMaterial *mat = material( mSymbol.get() );

if ( selected )
{
// update the material with selection colors
for ( Qt3DRender::QParameter *param : mat->effect()->parameters() )
{
if ( param->name() == QLatin1String( "diffuseColor" ) )
param->setValue( context.map().selectionColor() );
else if ( param->name() == QLatin1String( "ambientColor" ) )
param->setValue( context.map().selectionColor().darker() );
}
}
QgsMaterialContext materialContext;
materialContext.setIsSelected( selected );
materialContext.setSelectionColor( context.map().selectionColor() );
Qt3DRender::QMaterial *mat = material( mSymbol.get(), materialContext );

// build the entity
Qt3DCore::QEntity *entity = new Qt3DCore::QEntity;
Expand All @@ -230,7 +221,7 @@ void QgsInstancedPoint3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, co



Qt3DRender::QMaterial *QgsInstancedPoint3DSymbolHandler::material( const QgsPoint3DSymbol *symbol )
Qt3DRender::QMaterial *QgsInstancedPoint3DSymbolHandler::material( const QgsPoint3DSymbol *symbol, const QgsMaterialContext &materialContext )
{
Qt3DRender::QFilterKey *filterKey = new Qt3DRender::QFilterKey;
filterKey->setName( QStringLiteral( "renderingStyle" ) );
Expand Down Expand Up @@ -275,7 +266,7 @@ Qt3DRender::QMaterial *QgsInstancedPoint3DSymbolHandler::material( const QgsPoin
effect->addParameter( paramInst );
effect->addParameter( paramInstNormal );

symbol->materialSettings()->addParametersToEffect( effect );
symbol->materialSettings()->addParametersToEffect( effect, materialContext );

Qt3DRender::QMaterial *material = new Qt3DRender::QMaterial;
material->setEffect( effect );
Expand Down
2 changes: 1 addition & 1 deletion tests/src/3d/testqgsmaterialregistry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class DummyMaterialSettings : public QgsAbstractMaterialSettings
static bool supportsTechnique( QgsMaterialSettingsRenderingTechnique ) { return true; }
void readXml( const QDomElement &, const QgsReadWriteContext & ) override { }
void writeXml( QDomElement &, const QgsReadWriteContext & ) const override {}
void addParametersToEffect( Qt3DRender::QEffect * ) const override {}
void addParametersToEffect( Qt3DRender::QEffect *, const QgsMaterialContext & ) const override {}
Qt3DRender::QMaterial *toMaterial( QgsMaterialSettingsRenderingTechnique, const QgsMaterialContext & ) const override { return nullptr; }
QMap<QString, QString> toExportParameters() const override { return QMap<QString, QString>(); }
QByteArray dataDefinedVertexColorsAsByte( const QgsExpressionContext & ) const override {return QByteArray();}
Expand Down

0 comments on commit 684a802

Please sign in to comment.