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

move viewport rect logic from subplot and docks to Figure #724

Merged
merged 18 commits into from
Feb 21, 2025

Conversation

kushalkolar
Copy link
Member

@kushalkolar kushalkolar commented Feb 10, 2025

First step towards #315 and #670

Things will change once pygfx refactors their viewport stuff, but I think it will be easier to adopt those changes after this PR since it makes Figure manage the bbox of all subplots. I think this is a good separation of responsibilities. After this PR arbitrary bboxes for subplots can be implemented.

  • move the subplot viewport rect and subplot docks viewport rect calculating logic to Figure.
    • clean up the logic for computing the rects, much easier to read now, @clewis7 will be happy 😆
  • Figure._render() and PlotArea._render() are now private, been meaning to do this for a long while.
  • flatten cameras, controllers arrays in Figure to prep things for a flexible rectangles mode.
    • make sure all tests pass with this, adjust the tests if necessary. Users have a lot of options here and the parsing code is kinda gnarly.
  • prepare Figure for flex rects mode by allowing Figure._subplots to be a 1D object array, figure methods have been adjusted accordingly.
  • Figure.spacing is a new property which allows setting the spacing between subplots, default is 2 pixels
  • add new screenshot test for gridplot with all subplot docks of a certain size, and an ImageWidget with all docks set to a certain size.
  • adjust imgui toolbar code accordingly

Introduces one minor breaking API change, Subplot.position no longer exists but it's unlikely anyone was using that.

Todo in future PR:

  • implement flexible resizable rectangle subplots
  • make a separte plot area specifically for subplot titles, remove top subplot "dock" so users have less control over it. It should be solely for subplot title text.

@kushalkolar
Copy link
Member Author

kushalkolar commented Feb 10, 2025

biggest issue right now is when resizing, I think the canvas resizes a while before the new rect sizes are ready which causes wgpu draw errors this is Qt specific: pygfx/rendercanvas#56

@kushalkolar kushalkolar marked this pull request as ready for review February 13, 2025 06:19
@kushalkolar kushalkolar requested a review from clewis7 as a code owner February 13, 2025 06:19
@kushalkolar
Copy link
Member Author

kushalkolar commented Feb 13, 2025

@clewis7 ready for review! Any ubuntu tests won't pass on github actions due to pygfx/pygfx#979 but the mac tests should all pass (except for pygfx-release, the ones using pygfx@main should pass)

also tests are all passing locally

@kushalkolar
Copy link
Member Author

all relevant tests passing now except the docs build but you fixed that in another PR so this is good for another look!

Copy link
Member

@clewis7 clewis7 left a comment

Choose a reason for hiding this comment

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

LGTM

@clewis7 clewis7 merged commit cf5b11b into main Feb 21, 2025
50 of 52 checks passed
@clewis7 clewis7 deleted the subplot-docking branch February 21, 2025 18:54
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.

2 participants