-
Notifications
You must be signed in to change notification settings - Fork 3
Major Feature Overhaul #42
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
base: main
Are you sure you want to change the base?
Conversation
Defines how 2D view NDC are mapped to vectors in 3D space
In the pursuit of a single source of truth, let's resort to the projection matrix (when/if possible?) for this.
For some reason, if I don't do this, the Matrix3D constructor RESURRECTS some previous matrix I used in a previous test
Thanks to @tlambert03 for the suggestion. Could probably be cleaned up further, but at least this is a step in the right direction!
Eventually, we'll want to compute grids ourselves on the model side, I think, but for now this works
...man, this feels so good
It's worse :)
Notably, this limits the type of pygfx-specific interaction available. But we need a scenex version of this anyways - planning to implement the beginnings of this with events
Still need to add that functionality to child nodes...somehow :)
The newer versions of imgui aren't shipping wheels for <3.12 now.
|
I'll post observations as they come, rather than hold them for a big review... The first is similar to something I said on zulip at one point: the resizing behavior has a significantly degraded experience relative to main. (showing pygfx below) here's main. note how the aspect ratio never changes (only the letter-boxing), and the window size strictly follows the mouse without jittering. main.movhere's this PR. Note how the aspect ratio stretches and compresses, and only "fixes" itself after you release the mouse, and also how the window height rapidly flickers as you resize. pr.movWe need to recover the smoothness of main edit vispy.mov |
Ooh, thanks, yeah, I remember seeing this bug. It likely lies within the interconnection of pygfx+wx, since I don't see the flickering with either vispy+wx (which actually has a different bug where the aspect ratio is incorrect until you resize it) or pygfx+qt. Can you tell me whether resizing works well with another pair of backends? EDIT: After investigating further, there are actually two separate issues here:
I also noticed I can't run the
|
This is actually a temporary fix, as it copies changes also made upstream to rendercanvas. We can remove these changes once we can depend upon pygfx/rendercanvas#159
|
@tlambert03 can you confirm c6b8ccb fixes the flickering? |
|
yes I think so! 🎉 side question: Do you know why, on both main and this PR, when I try vispy with |
I actually didn't know, but but I can reproduce if the environment has both vispy and pygfx. Thus we have a bug where:
So we just need to be a bit smarter about how we fail out of |
f6e5c8c to
509cd93
Compare
|
@almarklein I hesitate to loop you into a monster PR, but I'm wondering if
Note that in a failed job where I added |
|
Hey @gselzer IIUC the hanging happened only with the Windows builds, right? Edit: I can reproduce hanging tests with the rendercanvas test suite on Windows. Looking into it: pygfx/rendercanvas#160 |
|
@almarklein it seems like the rendercanvas fixes have resolved the build! Thank you so much for being so speedy on the fix! 💯 |
This removes the need for some of those overridden methods :)
There might be a time where we add a shared document, but not now.
|
@gselzer awesome! And thanks for the heads-up! Aside from the frustration etc., introducing breaking changes is actually a pretty effective way to get to know downstream developers! 😉 |
Vispy uses point size while pygfx uses pixel size. For now, since the text model suggests using pixel size, we'll carry on with that.
|
@tlambert03 just wanted to bump this - are you still hoping to review this code, or is the only thing holding up merge here the fixes to
|
|
definitely still going to do another round of reviews (probably many more). However, if there's anything you know remains to be done... by all means go for it. |
This PR is a major expansion of scenex, introducing multi-view layouts, a comprehensive event system, and new additions to the model hierarchy.
Multi-View Grid System
A flexible grid layout enables multiple synchronized views (e.g., orthoviewers) on a single canvas. Views can be added to any grid position, span rows/columns, and are automatically laid out. The
Canvasnow uses aGridfor structured multi-view support.Architecture:
GridcontainsGridAssignmentobjects mapping views to positionsCanvas Changes:
Canvas.views(single list) replaced withCanvas.grid(structured layout)Comprehensive Event System
A unified event API currently supporting mouse, and wheel events across Qt, wx, and Jupyter.
Eventand its many subclasses are derived from native widget events and are passed to the models.MouseEvents include aworld_rayfor interactive picking, enabling features like node intersection and custom event filtering.Note that, to enable these events, we require significant amounts of app code. This code has been migrated from
ndv, contained within thescenex.appmodule, such that it could be cleanly extracted later.Event Flow:
Node Intersection:
passes_through(ray) -> float | Nonevisible=False) don't intersectNew Geometry Node Types
This PR introduces (Poly)Lines, Meshes, and Text as new
Nodeimplementations. Example instances of each are shown below.ColorModel System
scenex needed structures determining how colors could map to geometry for various
Nodesubclasses. TheColorModelbase class has been added along with concrete subclasses:UniformColor,FaceColors, andVertexColors. This architecture enables all geometric nodes (Points,Line,Mesh) to declare whichColorModels they support (for example,Pointswill only supportUniformColorandVertexColors).Camera Controllers & Projection
The
CameraAPI has been overhauled to deduplicate state. Most notably, theCameraoffers aprojection: Transformwhich describes how 3D space is mapped to/from the 2D canvas. Common projection types can be generated from the utilities withinscenex.util.projections.For user interaction,
Cameraobjects can also be assigned anInteractionStrategywhich defines what happens to theCamerawhenEvents occur.PanZoomandOrbitare two implementations of this model, and it is simple to add additional implementations later on.Finally, the
Viewobject now can be assigned aResizeStrategythat defines what happens when theViewis resized either programmatically or through user events.Letterboxis one implementation of this model, and we could also add additional strategies later.Testing & Documentation
All new features are covered by many new unit tests across all supported backends and platforms, and many examples have been added to showcase major additions. Documentation is overhauled with comprehensive NumPy-style docstrings and new examples, ensuring discoverability and ease of use.
Points of Discussion:
This is obviously a large change. I'd particularly like to draw the viewer's attention to the new models (
ColorModel,ResizeStrategy,InteractionStrategy,Grid, etc.). I am very interested in critique of these structures. Are they well named? Are they declaritive? Are they missing things?There are certainly more features to add as time goes on. One piece of low-hanging fruit is the implementation of
FaceColorsfor meshes. But we can certainly start review before that gets solved.The end goal of these changes, as alluded to by the branch name, is the ability to transform
ndvby removing all of thepygfx/vispy-dependent code. If you'd like to see how this might look, check out the branch that depends on this changeset and feel free to consider and/or test that code.One aspect of the changes that I removed fairly recently was the ability to add per-node event filters - I found this without real usage either here or on my ndv branch using it. I do think that it would be a nice feature to have, just don't want to add it (back) in without concrete need. Open to others' opinions on this, though.
Closes #16 as well as #38, #36, #34, #33, #32, #28, #27 (duplicate code)