-
Notifications
You must be signed in to change notification settings - Fork 0
feat: support multiple canvas instances #109
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @MinCrohn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses issue #106 by removing the singleton limitation on the core Patchmap library components. The changes involve refactoring the internal architecture to pass instance-specific context (like theme, undo manager, and renderer) through functions and objects, rather than relying on global instances. This enables the library to support multiple independent instances, facilitating multi-canvas applications.
Highlights
- Remove Singleton Pattern: The global
Patchmap,UndoRedoManager,Theme, and texture caching mechanisms have been refactored to be instance-specific. This allows multiple independent Patchmap instances to exist and operate concurrently, enabling multi-canvas support. - Instance Context Passing: Key functions and command handlers throughout the codebase (e.g., in
display/change,display/update,command) now accept and pass anoptionsorcontextobject. This object carries instance-specific data like theviewport,renderer,theme, andundoRedoManager, ensuring operations are performed within the correct instance's context. - Texture Caching Update: Texture generation and caching logic (
src/assets/textures) is updated to usePIXI.Assets.cacheand includes the renderer's unique ID in the cache key. This prevents texture conflicts when multiple instances are generating textures. - Initialization Logic Refinement: The initialization process (
src/init.js) is updated to assign a unique ID to the renderer instance and link the viewport to its application instance. Asset initialization is also made more robust to handle potential multiple calls.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively refactors the codebase to remove singleton patterns for renderer, theme, and undoRedoManager, enabling multi-canvas support. Changes are comprehensive, including passing necessary context through parameters, updating asset initialization, and ensuring renderer-specific texture caching. The approach is solid. A few minor suggestions are provided for improving robustness, documentation, and efficiency in asset loading.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively refactors the library to support multiple PatchMap instances. This was achieved by systematically removing singleton patterns for renderer, theme, undoRedoManager, and GSAP timelines, and instead passing these dependencies via a context object or as direct function parameters. Key technical improvements include renderer-specific texture caching (using renderer.uid) and robust asset initialization checks, which are crucial for preventing conflicts when multiple canvas instances are active. The changes demonstrate a consistent approach and appear well-implemented for the intended multi-instance support.
My review primarily focuses on two aspects:
- Highlighting the need to update JSDoc comments to reflect new function parameters, ensuring long-term maintainability.
- Seeking confirmation on a subtle change in how component configurations are passed during updates, to preempt any potential side effects.
As no specific style guide was provided for this review, my feedback adheres to common JavaScript best practices, emphasizing clarity, correctness, and maintainability.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the codebase to remove singleton patterns, enabling multi-canvas and multi-instance support for PatchMap. The changes improve modularity and flexibility. Key areas like rendering, theming, undo/redo management, animation, and event handling state are now instance-specific. A few points regarding validation and initialization have been raised for consideration.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively refactors the codebase to support multi-canvas instances by removing singleton patterns and introducing instance-specific management for services like rendering, theming, undo/redo, and animations. The changes are comprehensive, touching many parts of the application, and appear to be well-thought-out. The introduction of a context object for passing dependencies and instance-specific state for event handlers are key enablers for this functionality. Asset management has also been improved to handle multiple renderers and avoid global state issues. The code quality is good, with several improvements to clarity and robustness (e.g., Number.isFinite checks, more flexible utility functions).
closed: #106