[feature] Add extra mesh layer legend node for currently displayed vector datasets. #55746

nstrahl · 2024-01-02T15:30:24Z

[feature] Fixes #55745 feature request.
Mesh layer legend on the layer tree are now shown as two collapsible sub-items. One for contours, another one for vector display.

For single-color Vector rendering configuration, the arrow icon is simply re-colored in the legend (e.g. green)

This now allows for the items to be shown (and modified) in a print layout.

right-hand-side symbol display was also checked and is working:

github-actions · 2024-01-02T16:05:02Z

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit ff65d90)

github-actions · 2024-01-02T18:20:50Z

Tests failed for Qt 5

One or more tests failed using the build from commit 955168f

legend_diagram_attributes (testDiagramAttributeLegend)

legend_diagram_attributes

Test failed at _verifyImage at tests/src/core/testqgslegendrenderer.cpp:220

Rendered image did not match tests/testdata/control_images/legend/expected_legend_diagram_attributes/expected_legend_diagram_attributes.png (found 172 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

github-actions · 2024-01-02T18:20:53Z

Tests failed for Qt 6

One or more tests failed using the build from commit 236e729

legend_mesh_diagram_no_vector (testDiagramMeshLegend)

legend_mesh_diagram_no_vector

Test failed at _verifyImage at tests/src/core/testqgslegendrenderer.cpp:167

Rendered image did not match tests/testdata/control_images/legend/expected_legend_mesh_diagram_no_vector/expected_legend_mesh_diagram_no_vector.png (found 697 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

uclaros · 2024-01-08T10:20:37Z

@nstrahl Thanks for the PR! Can you make sure the code formatting is according to the QGIS coding standards?

There's a script named scripts/prepare_commit.sh which is meant to be used as a pre-commit hook and should take care of properly formatting your code, so make sure you use this before committing.
For the current PR you can use scripts/astyle_all.sh or manually use scripts/astyle.sh for the modified files.

nstrahl · 2024-01-08T12:37:25Z

@uclaros Thanks for the reply! I'm on it. I switched over to linux now, but out of curiosity how can the code be formated on Windows? I only see .sh scripts for this.

uclaros · 2024-01-08T13:49:53Z

Hmm, not sure about that, sorry, but I'd guess either using cygwin on wsl

uclaros · 2024-01-18T21:47:08Z

src/core/layertree/qgslayertreemodel.cpp

+  if ( layerType == Qgis::LayerType::Mesh )
+  {
+    QHash<QString, QgsLayerTreeModelLegendNode *> rule2node;
+    rule2node[QString()] = nullptr;
+    for ( QgsLayerTreeModelLegendNode *n : nodes )
+    {
+      QString key = n->objectName();
+      if ( key.isEmpty() ) // in tree all nodes must have key
+        return nullptr;
+      if ( rule2node.contains( key ) ) // and they must be unique
+        return nullptr;
+      rule2node[key] = n;
+    }
+
+    LayerLegendTree *tree = new LayerLegendTree;
+    for ( QgsLayerTreeModelLegendNode *n : nodes )
+    {
+      const QString parentName = n->property( "parentNode" ).toString();
+      QgsLayerTreeModelLegendNode *parent = rule2node.value( parentName, nullptr );
+      tree->parents[n] = parent;
+      tree->children[parent] << n;
+    }
+    return tree;
+  }
+


Duplicating this code for mesh layers and relying on custom properties feels a little hacky...
I would suggest to leave this class as is and re-use existing infrastructure for tree-like legend items.
I think you should populate QgsLayerTreeModelLegendNode::RuleKeyRole for the QgsSimpleLegendNodes and QgsLayerTreeModelLegendNode::ParentRuleKeyRole for the QgsRasterSymbolLegendNodes and QgsColorRampLegendNodes (you'll have to update their data/setData methods as they do not support it now) and the tree should populate itself!

Thanks for the advice. I'll be pushing my commit in a bit. Btw, the KeyRoles are encapsulated in the classes and only settable upon construction and retrievable via the data method, so I stuck to those restrictions.

uclaros · 2024-01-18T21:49:42Z

src/core/layertree/qgslayertreemodellegendnode.cpp

@@ -143,25 +143,24 @@ QSizeF QgsLayerTreeModelLegendNode::drawSymbol( const QgsLegendSettings &setting

  if ( ctx && ctx->painter )
  {
+    const QgsScopedRenderContextScaleToPixels scopedScaleToPixels( *( ctx->context ) );
+    const double scaleFactor = ctx->context->scaleFactor();


Why do we need to multiply by the scale factor here? Won't this affect other symbols too?

Up to this point the nodes of this type have only been assigned a simple QIcon (square solid color rectangle). Under those circumstances it does not really matter if a scale factor is applied. (A square solid color rectangle more or less remains a square solid color rectangle regardless of scaling). But if a more complex QIcon is assigned (e.g. arrow) without scaling then there will be unwanted distortion/pixelation. If you look at the new /expected_legend_diagram_attributes/expected_legend_diagram_attributes_mask.png I uploaded, you will find that the icons are now slightly more rectangular because proper scaling is applied. I am guessing no one else ever bothered to do so because it was never really noticeable.

Watch out, ctx->context may be nullptr here. You'll need to check that and handle gracefully.

nyalldawson · 2024-01-28T23:31:57Z

src/core/layertree/qgscolorramplegendnode.h

+     * \param key rule key
+     * \param parentKey parent rule key


Suggested change

* \param key rule key

* \param parentKey parent rule key

* \param key rule key (since QGIS 3.36)

* \param parentKey parent rule key (since QGIS 3.36)

Also would be better to add some explanatory docs here, eg for when parent rule key should be set.

nyalldawson · 2024-01-28T23:32:18Z

src/core/layertree/qgscolorramplegendnode.h

+     * \param key rule key
+     * \param parentKey parent rule key


Suggested change

* \param key rule key

* \param parentKey parent rule key

* \param key rule key (since QGIS 3.36)

* \param parentKey parent rule key (since QGIS 3.36)

nyalldawson · 2024-01-28T23:33:23Z

src/core/layertree/qgslayertreemodellegendnode.h

@@ -677,8 +677,10 @@ class CORE_EXPORT QgsRasterSymbolLegendNode : public QgsLayerTreeModelLegendNode
     * \param parent attach a parent QObject to the legend node.
     * \param isCheckable set to TRUE to enable the checkbox for the node (since QGIS 3.18)
     * \param ruleKey optional identifier to allow a unique ID to be assigned to the node by a renderer (since QGIS 3.18)
+     * \param parentRuleKey rule key of parent


Suggested change

* \param parentRuleKey rule key of parent

* \param parentRuleKey rule key of parent (since QGIS 3.36)

Also add explanation of when this should be set

nyalldawson · 2024-01-28T23:34:42Z

src/core/qgsapplication.cpp

+    if ( QDir dir = QFileInfo( iconPath ).dir(); !dir.exists() )
+    {
+      dir.mkpath( "." );
+    }


Can you pull this out to a separate pull request? Its unrelated to the legend changes.

Pull request #56171 has now been submitted. It will have to be accepted first so I can then rebase this branch and continue working on this pull request.

nyalldawson · 2024-01-28T23:39:51Z

src/core/layertree/qgslayertreemodellegendnode.cpp

@@ -143,25 +143,24 @@ QSizeF QgsLayerTreeModelLegendNode::drawSymbol( const QgsLegendSettings &setting

  if ( ctx && ctx->painter )
  {
+    const QgsScopedRenderContextScaleToPixels scopedScaleToPixels( *( ctx->context ) );
+    const double scaleFactor = ctx->context->scaleFactor();


Watch out, ctx->context may be nullptr here. You'll need to check that and handle gracefully.

nyalldawson · 2024-01-28T23:41:03Z

Can you also add some tests covering these changes?

nstrahl · 2024-02-12T14:18:12Z

My test fails on Qt6 when comparing the expected vs. generated .png image, but it succeeds on Qt5. Can someone give me some tips? I saw that I don't have an image mask for the expected .png files so I was thinking maybe I should also add a mask. Would this solve my problem?

github-actions · 2024-03-30T02:33:59Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

nstrahl · 2024-04-02T07:12:20Z

@uclaros Is there anything missing on this PR?

uclaros

The difference in the test results is quite significant. Is that caused by the introduction of the scaleFactor ?
Maybe it makes more sense to update the control images instead of the mask?

uclaros · 2024-04-02T12:00:55Z

python/PyQt6/core/auto_generated/layertree/qgscolorramplegendnode.sip.in

+:param key: rule key. optional identifier to allow a unique ID to be assigned to the node by a renderer (since QGIS 3.36)
+:param parentKey: rule key of parent (since QGIS 3.36)


All since mentions should now be changed to QGIS 3.38

src/core/layertree/qgslayertreemodellegendnode.cpp

src/core/qgsmaplayerlegend.cpp

nstrahl · 2024-04-06T12:17:21Z

The difference in the test results is quite significant. Is that caused by the introduction of the scaleFactor ? Maybe it makes more sense to update the control images instead of the mask?

The differences in tests/testdata/control_images/legend/expected_legend_diagram_attributes/expected_legend_diagram_attributes_mask.png are minimal. This is caused by the scaleFactor, but I would say the difference is negligible and even an improvement. (The layer item colored squares now look more "square")

For the brand new tests added in tests/testdata/control_images/legend/expected_legend_mesh_diagram_no_vector, yes the differences are a bit larger, but they arise when switching from Qt5 to Qt6. I am unsure whether the scaleFactor has anything to do with this. There are other tests for the mapLayerLegends and the modifications in this PR did not cause any changes in the test images. I have also opened my Qt5 and Qt6 builds and am unable to visually perceive any differences between the mesh layer legends and layouts of the two. Updating the control images as you suggested won't work since there are some small differences between Qt builds so there has to be a mask file to account for this.

… they pass for qt6 builds as well

nstrahl · 2024-09-08T17:42:55Z

@uclaros @nyalldawson this PR is now probably ready to be merged in.

src/core/layertree/qgslayertreemodellegendnode.cpp

github-actions bot added this to the 3.36.0 milestone Jan 2, 2024

PeterPetrik requested a review from uclaros January 3, 2024 07:19

uclaros reviewed Jan 18, 2024

View reviewed changes

nyalldawson reviewed Jan 28, 2024

View reviewed changes

nstrahl force-pushed the mesh_layer_legend_vector_dataset branch from bce1abc to 3705bc1 Compare February 10, 2024 17:38

github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Feb 27, 2024

qgis deleted a comment from github-actions bot Feb 29, 2024

github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Feb 29, 2024

uclaros closed this Feb 29, 2024

uclaros reopened this Feb 29, 2024

github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 15, 2024

qgis deleted a comment from github-actions bot Mar 15, 2024

uclaros closed this Mar 15, 2024

github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 15, 2024

uclaros reopened this Mar 15, 2024

github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 30, 2024

github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 2, 2024

uclaros reviewed Apr 2, 2024

View reviewed changes

nstrahl requested a review from nyalldawson April 8, 2024 06:05

nstrahl and others added 17 commits August 31, 2024 12:02

improve header docs and sipify

2c4fc34

Rebase

a613cbf

Use new enum class for custom roles

cafa78a

Add new tests for meshlayer node renderer (Scalar and Vector datasets)

b18c3e3

apply formatting

c86d699

Add missing 0 argument

3678dd3

Use None instead of 0

351d856

Add masks and increase size tolerance for mesh legend image checks so…

6a67bdb

… they pass for qt6 builds as well

check if ctx->context is nullptr

5a19a55

Bump API changes version up to 3.38

f688421

set standart test font to avoid large discrepancies in control images

da7564a

Update to new namespace changes

b69c1ab

Update to new namespace changes

6a46984

Update version mentions to 3.40

6316905

Use newer enum namespace

f425171

Use new test functions

8a86634

update test masks

236e729

nstrahl force-pushed the mesh_layer_legend_vector_dataset branch from a0dd696 to 236e729 Compare September 1, 2024 18:04

Update test masks

eff40ef

github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 23, 2024

github-actions bot closed this Sep 30, 2024

uclaros reopened this Sep 30, 2024

github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 30, 2024

qgis deleted a comment from github-actions bot Sep 30, 2024

nyalldawson approved these changes Oct 1, 2024

View reviewed changes

src/core/layertree/qgslayertreemodellegendnode.cpp Outdated Show resolved Hide resolved

Update src/core/layertree/qgslayertreemodellegendnode.cpp

ff65d90

nyalldawson added the Squash! Remember to squash this PR, instead of merging or rebasing label Oct 1, 2024

nyalldawson merged commit 9a08f10 into qgis:master Oct 2, 2024
33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Add extra mesh layer legend node for currently displayed vector datasets. #55746

[feature] Add extra mesh layer legend node for currently displayed vector datasets. #55746

nstrahl commented Jan 2, 2024

github-actions bot commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

uclaros commented Jan 8, 2024

nstrahl commented Jan 8, 2024 •

edited

Loading

uclaros commented Jan 8, 2024

uclaros Jan 18, 2024

nstrahl Jan 28, 2024

uclaros Jan 18, 2024

nstrahl Jan 28, 2024

nyalldawson Jan 28, 2024

nyalldawson Jan 28, 2024

nyalldawson Jan 28, 2024

nyalldawson Jan 28, 2024

nyalldawson Jan 28, 2024

nyalldawson Jan 28, 2024

nstrahl Feb 4, 2024

nyalldawson Jan 28, 2024

nyalldawson commented Jan 28, 2024

nstrahl commented Feb 12, 2024

github-actions bot commented Mar 30, 2024

nstrahl commented Apr 2, 2024

uclaros left a comment

uclaros Apr 2, 2024

nstrahl commented Apr 6, 2024

nstrahl commented Sep 8, 2024

	* \param parentRuleKey rule key of parent
	* \param parentRuleKey rule key of parent (since QGIS 3.36)

		:param key: rule key. optional identifier to allow a unique ID to be assigned to the node by a renderer (since QGIS 3.36)
		:param parentKey: rule key of parent (since QGIS 3.36)

[feature] Add extra mesh layer legend node for currently displayed vector datasets. #55746

[feature] Add extra mesh layer legend node for currently displayed vector datasets. #55746

Conversation

nstrahl commented Jan 2, 2024

github-actions bot commented Jan 2, 2024 • edited Loading

🪟 Windows builds ready!

github-actions bot commented Jan 2, 2024 • edited Loading

Tests failed for Qt 5

legend_diagram_attributes (testDiagramAttributeLegend)

legend_diagram_attributes

github-actions bot commented Jan 2, 2024 • edited Loading

Tests failed for Qt 6

legend_mesh_diagram_no_vector (testDiagramMeshLegend)

legend_mesh_diagram_no_vector

uclaros commented Jan 8, 2024

nstrahl commented Jan 8, 2024 • edited Loading

uclaros commented Jan 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nyalldawson commented Jan 28, 2024

nstrahl commented Feb 12, 2024

github-actions bot commented Mar 30, 2024

nstrahl commented Apr 2, 2024

uclaros left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nstrahl commented Apr 6, 2024

nstrahl commented Sep 8, 2024

github-actions bot commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

nstrahl commented Jan 8, 2024 •

edited

Loading