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

222 n rendering layers in overlay #224

Merged
merged 11 commits into from
Feb 16, 2024

Conversation

MattClarkson
Copy link
Contributor

A summary of things to look out for.

  • Internally, 5 rendering layers instead of 3. I didn't see the benefit of putting them in an array, because they are a mixture of image renderers and VTK renderers, each with different purposes, so you would not be iterating through a list.
  • Background, Foreground and Overlay made sense in a 3 layer model. I kept the API as close as I could so, default settings would revert to the old behaviour. As such, it should be a drop in replacement for applications that already use it like BARD.
  • I did run the code through "black" formatter, default settings, to get PEP 8 style.
  • As Python doesn't do private/public/protected, you may find client code or unit tests referring to internal member variables. This is bad practice anyway, so I tried to provide accessor functions.
  • But mostly, I tried to keep the API changes minimal.

Copy link
Collaborator

@thompson318 thompson318 left a comment

Choose a reason for hiding this comment

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

Looks good to me and is passing CI, thank you.

@MattClarkson MattClarkson merged commit a43b544 into master Feb 16, 2024
14 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants