Merge the rest of brush graph code into main#73
Conversation
add interface + test for repo
Rebasing this through the branches proved to be incredibly painful, so it's all in one commit at the top.
clientBrushFamilyId is a non-public field, so we shouldn't support editing it. Leaving in the field in familyNodeData to support round-tripping of proto files to/from brush graph. Tests were broken by a previous change to how texture store was passed around.
Smaller display QOL adjustments (larger node headers, scrollable test canvas menu) + fix a bug which overwrote test brush color on screen rotate.
Fix a myriad of texture store bugs. Be more consistent overall about using the CompositionLocal model. Update CanvasStrokeRenderer when a new texture is loaded, fixing a bug where replacing the bitmap for a texture ID fails to update in the renderer by forcing cache eviction. Fix other bugs around textures for custom brushes in the palette, so they appear properly in the drawing canvas/preview.
[BrushGraph 3/7] Add ViewModel and state management
[BrushGraph 5/7] Add complex node fields
[BrushGraph 7/7] Integrate Brush Graph into the app
maxmmitchell
changed the title
maxmmitchell
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the core architecture for the Brush Graph feature, including the BrushGraphViewModel, BrushGraphRepository, and conversion utilities for transforming between graph models and BrushFamily protocols. It also introduces an auto-save mechanism, a tutorial management system, and shared test infrastructure. The review feedback highlights the use of Thread.sleep in BrushGraphViewModelTest.kt, which should be replaced with more robust coroutine testing utilities like advanceUntilIdle() to prevent test flakiness.
| viewModel.saveToPalette(brushName) | ||
|
|
||
| // Wait for the IO coroutine to finish on the device | ||
| Thread.sleep(500) |
There was a problem hiding this comment.
Using Thread.sleep in tests is a bad practice as it makes tests flaky and slow. Instead, consider using a TestDispatcher or runTest with advanceUntilIdle() to wait for coroutines to complete.
| viewModel.deleteFromPalette(brushName) | ||
|
|
||
| // Wait for the IO coroutine to finish on the device | ||
| Thread.sleep(500) |
There was a problem hiding this comment.
Using Thread.sleep in tests is a bad practice as it makes tests flaky and slow. Instead, consider using a TestDispatcher or runTest with advanceUntilIdle() to wait for coroutines to complete.
…h/5-complex-fields, effectively) Add nodes and complex fields to corpus of brush graph code
…ph/7-integration, effectively) Merge canvas and integration code into corpus of brush graph code
2 participants
Already reviewed in #61, #62, #63, #64, #65, and #66. This is just to get the code into main (previous merging just condensed the code into a subset of the existing branches. Now it is all on brush-graph/2-persistence, and this puts it in main for real).