Provide a single instance location so that views don't need to share handles anymore.#131
Open
pragmatrix wants to merge 6 commits into
Open
Provide a single instance location so that views don't need to share handles anymore.#131pragmatrix wants to merge 6 commits into
pragmatrix wants to merge 6 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #127 by removing cross-subsystem sharing of mutable scene handles and instead introducing a single, stable instance root location that views can parent to, so the desktop can present instances without referencing view-local handle definitions that may not yet be ingested by the renderer.
Changes:
- Introduces
Ref<T>as a read-only handle type and updates scene objects/ergonomics to useReffor location/transform references. - Adds
InstanceRoot(transform + location) to provide a shared instance node for presenters and views, and updates desktop presentation flow to use it. - Simplifies
ViewCreationInfoand view creation so the desktop no longer receives/uses a view-providedLocationhandle.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scene/src/transform_resolver.rs | Switches resolver caching/lookup to use Ref<Location> keys. |
| scene/src/objects.rs | Migrates Visual/Location to store Ref references instead of mutable Handles. |
| scene/src/handle.rs | Adds Ref<T> and HandleValue, and tightens update/change collection ordering in InnerHandle. |
| scene/src/ergonomics.rs | Updates positioning helpers (At, VisualWithoutLocation::at) to accept Ref<Location>. |
| desktop/src/instance_presenter.rs | Uses InstanceRoot and a shared instance location/transform node for background + view alpha. |
| desktop/src/instance_manager.rs | Introduces and stores InstanceRoot, and passes a Ref<Location> parent into InstanceContext. |
| desktop/src/desktop.rs | Updates command handling for the new DestroyView signature (no collector payload). |
| desktop/src/desktop_system/presentation.rs | Threads InstanceRoot through instance presentation. |
| desktop/src/desktop_system/command_dispatch.rs | Creates/passes InstanceRoot during instance spawn and presentation. |
| applications/src/view.rs | Parents view-local location under a provided parent Ref<Location>; removes sending desktop-owned location in CreateView. |
| applications/src/view_builder.rs | Carries parent: Ref<Location> through the builder into View::new. |
| applications/src/instance_context.rs | Stores a view_parent: Ref<Location> and changes DestroyView command payload. |
| .github/copilot-instructions.md | Adds repo orientation, boundaries, and validation guidance for future edits. |
…s the scene module
…on't need to be shared anymore
21532a5 to
2b10eb0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #127