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

Support vtkLegendScaleActor with add_legend_scale #3716

Merged
merged 12 commits into from
Jan 31, 2023
Merged

Conversation

banesullivan
Copy link
Member

@banesullivan banesullivan commented Dec 14, 2022

Overview

Adds support for vtkLegendScaleActor which is a widget style actor for adding a sense of scale to the scene. This has tons of potential configurations options and I tried to expose what I could and make it similar to add_ruler().

Also cleans up the related add_ruler() method by moving it to the `Renderer`` class and adding color setters to resolve #3888

Details

import pyvista as pv

p = pv.Plotter(notebook=0)
p.add_mesh(pv.Cone())
p.renderer.add_legend_scale()
p.set_background('darkgrey')
p.show()

Screen Shot 2022-12-14 at 4 31 23 PM

To do

  • Set the text and component colors to match the active theme
  • Decide if *_axis_visibility should be on or off by default
  • Add example to docstring
  • Add test
  • Converge with add_ruler to make sure APIs are similar
  • Fix any issues with add_ruler - e.g., text color

Known Issues

The label colors are either white or black. This seems to be an upstream bug in VTK where if the color is set at all, it turns the label black.

@github-actions github-actions bot added the enhancement Changes that enhance the library label Dec 14, 2022
@banesullivan banesullivan self-assigned this Dec 14, 2022
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Merging #3716 (37f6b54) into main (86d7356) will increase coverage by 0.00%.
The diff coverage is 93.15%.

@@           Coverage Diff           @@
##             main    #3716   +/-   ##
=======================================
  Coverage   94.18%   94.19%           
=======================================
  Files          91       91           
  Lines       19586    19631   +45     
=======================================
+ Hits        18448    18492   +44     
- Misses       1138     1139    +1     

@MatthewFlamm
Copy link
Contributor

See #2531 which implements the ability to add the rulers using vtkAxisActor2D. vtkLegendScaleActor uses vtkAxisActor2D under the hood with some defaults, but it also has the ability to add that black and white thingy.

I am pointing this out so the documentation can link from one to the other (See Also section) and that we should try to keep the API consistent (maybe necessitating changes to the code from the prior PR).

@banesullivan
Copy link
Member Author

Ah, I missed that PR and that feature - thank you for linking!

@banesullivan banesullivan marked this pull request as ready for review January 28, 2023 01:00
@banesullivan banesullivan changed the title [WIP] add vtkLegendScaleActor Support vtkLegendScaleActor with add_legend_scale Jan 28, 2023
@banesullivan banesullivan added this to the v0.38.0 milestone Jan 28, 2023
Copy link
Contributor

@MatthewFlamm MatthewFlamm left a comment

Choose a reason for hiding this comment

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

Mostly docstring suggestions

The corner offset value.

bottom_border_offset : int, default: 30
Bottom border offset. Recommended value ``50``
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the defaults not the same as recommended for all of these?

Copy link
Member Author

Choose a reason for hiding this comment

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

The defaults match what's set as default in VTK and make the actors visible in testing. The recommended values make them look quite a bit nicer for PyVista's default window_size or bigger

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a hard choice then. I don't have a strong opinion, so leave it up to you and or other reviewers.

pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
Co-authored-by: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com>
@banesullivan
Copy link
Member Author

Thanks @MatthewFlamm!

MatthewFlamm
MatthewFlamm previously approved these changes Jan 29, 2023
Copy link
Contributor

@MatthewFlamm MatthewFlamm left a comment

Choose a reason for hiding this comment

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

LGTM

@banesullivan banesullivan merged commit 4f799bb into main Jan 31, 2023
@banesullivan banesullivan deleted the feat/legend-scale branch January 31, 2023 00:29
@banesullivan banesullivan mentioned this pull request Feb 1, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Changes that enhance the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add_ruler allow user to set text color and tick color
2 participants