Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mesh] [feature] App widgets for styling mesh layers #7313

Merged
merged 5 commits into from
Jun 28, 2018

Conversation

PeterPetrik
Copy link
Contributor

@PeterPetrik PeterPetrik commented Jun 26, 2018

App widgets for styling mesh layers

mesh gui

The mesh layer properties could be now styled from properties or
style panel similar to vector or raster layers. These new
actions were introduced in the pull request.

  • Adding datasets to mesh layer from properties panel
  • Information and source panel in properties panel
  • Selection of active dataset (properties or styling panel)
  • Styling of contours/scalars (properties or styling panel)
  • Styling of mesh frame (properties or styling panel)
  • Styling of vector arrows (properties or styling panel)

The contours/raster datasets are styled with new QgsColorShaderWidget
that was extracted from QgsSingleBandPseudoColorRendererWidget. This
widget should be used in both QgsSingleBandPseudoColorRendererWidget and
QgsMeshRendererScalarSettingsWidget in the future. But integration to
QgsSingleBandPseudoColorRendererWidget should be taken with extra care and
I propose to do it in separate PR after this one.

Also, to keep this (already big enough) PR in acceptable size, these are known
limitations of the current implementation:

  • support dataset groups directly in MDAL. Dataset groups are groups of datasets
    that contains data for same quantity (e.g. temperature) but over different time
    (e.g. dataset group is "Temperature" and it consists of datasets "Temperature on Monday morning", "Temperature on Monday evening", ...). The current implementation is based
    on naming convention of datasets.
  • read and write dataset item in QGIS project. Right now only mesh layer URI is persisted
  • read and write styling in QGIS project
  • allow to set and persist styling of dataset groups. Right now the styling is per layer, not per dataset group

Thanks to @wonder-sk for help and internal review of the code.

Relates to QEP-119

- Adding datasets to mesh layer from properties panel
- Information and source panel in properties panel
- Selection of active dataset (properties or styling panel)
- Styling of contours/scalars (properties or styling panel)
- Styling of mesh frame (properties or styling panel)
- Styling of vector arrows (properties or styling panel)
@wonder-sk wonder-sk added this to the 3.4 milestone Jun 26, 2018
@m-kuhn m-kuhn requested a review from wonder-sk June 26, 2018 09:24
Copy link
Member

@wonder-sk wonder-sk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn I have already reviewed Peter's code earlier (PeterPetrik#13) so for me this PR is +1. Another pair of eyes would be useful, but I know it is a lot of code :-)

The good news is that this is probably the last big chunk of mesh-related code and further updates should be much more manageable incremental improvements.

@nyalldawson
Copy link
Collaborator

nyalldawson commented Jun 26, 2018

I had a brief flick through the code and it looks good to me.

I have a couple of UX issues from looking at the screencast. I haven't built, so these may be non-issues, but:

  • After the "assign extra dataset to mesh" step, I can't see any ui feedback showing the extra datasets which have been added (or anyway to edit/remove these)
  • The "groups" tree at the top of the styling widget is very short and needs a bigger minimum size
  • "is on vertex"/"is vector" under metadata show 1/0 - this should be true/false or yes/no. Also the capitalization should be fixed here.
  • Capitalization for min/max
  • what's the top spin box under vector rendering control? There's no label showing this
  • Width % - can you confirm that .15% is indeed 0.15 % and not 15%?
  • What's the units for the spin boxes under native mesh?

@nirvn
Copy link
Contributor

nirvn commented Jun 27, 2018

Couple of styling UI/UX thoughts:

  • I find the current arrangement a bit cumbersome / overwhelming. I would suggest dropping the collapsible group boxes and use tabs exactly like the labelling engine does (generic, buffer, shield, shadow, etc. tabs).
  • Each tab (mesh, vector, etc.) would have on top a checkbox to enable/disable the styling (i.e. [x] draw mesh, [x] draw vector, etc.).
  • Each tab would have an icon, just like labelling, to clearly identify what the styling is about.

IMHO, that'll make for a much cleaner and clearer UI.

As for the color shader widget, nice, +1 for integrating it for the pseudocolor too. BTW, one thing that'd be nice to implement is for the color ramp button to have its ramp updating when the user manually updates the list (steps, colors, etc.).

@nyalldawson
Copy link
Collaborator

+1 to @nirvn's UI proposal

@PeterPetrik
Copy link
Contributor Author

@nyalldawson @nirvn fixed review issues...

mesh2

@nirvn
Copy link
Contributor

nirvn commented Jun 28, 2018

@PeterPetrik , spectacular UI improvements!

@PeterPetrik
Copy link
Contributor Author

It is indeed a lot better than the first try, thank you for suggestions!

@DelazJ
Copy link
Contributor

DelazJ commented Jun 28, 2018

Shouldn't the labels aligned on left? I can see in the Styling panel some checkboxes or group titles (?) that are centered and as far as I remember, this not the usual placement of the options...

@wonder-sk
Copy link
Member

Shouldn't the labels aligned on left? I can see in the Styling panel some checkboxes or group titles (?) that are centered

I think that's just a different style in Qt (default in KDE). For example I also use KDE and a checkable group box looks like this by default:
image

@PeterPetrik PeterPetrik reopened this Jun 28, 2018
@DelazJ
Copy link
Contributor

DelazJ commented Jun 28, 2018

Thanks for the explanation @wonder-sk

@wonder-sk wonder-sk merged commit 97addfc into qgis:master Jun 28, 2018
@PeterPetrik PeterPetrik deleted the mesh_layer_styling_gui branch June 29, 2018 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants