feat(ui): BloomView live-render plumbing on every backend (#5519)#5538
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughVersion bumped to 0.5.1199. ChangesBloomView Live-Render Plumbing
Sequence Diagram(s)sequenceDiagram
participant TS as TypeScript caller
participant Perry as Perry dispatch (ui_table)
participant Backend as Native backend (platform)
participant Bloom as Bloom engine
TS->>Perry: bloomViewCreate(width, height)
Perry->>Backend: create SurfaceView / UIView / NSView / DrawingArea
Backend-->>Perry: widget handle (i64)
Perry-->>TS: handle
loop each frame until surface ready
TS->>Perry: bloomViewGetNativeHandle(handle)
Perry->>Backend: validate widget, check surface ready
Backend-->>Perry: 0 (not ready)
Perry-->>TS: 0
end
TS->>Perry: bloomViewGetNativeHandle(handle)
Perry->>Backend: ANativeWindow_fromSurface / NSView* / UIView* / GtkWidget*
Backend-->>Perry: native pointer as i64
Perry-->>TS: non-zero handle
TS->>Bloom: attachToNSView(nativeHandle)
loop render frames
TS->>Bloom: clearBackground + draw + endFrame
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-ui-android/src/widgets/bloomview.rs`:
- Around line 74-80: The get_native_handle function calls
ANativeWindow_fromSurface on every invocation which returns a window reference
with a +1 refcount, but the code never calls ANativeWindow_release to decrement
these references. This causes reference leaks on repeated invocations. Implement
proper reference management by either caching the ANativeWindow pointer and
reusing it instead of creating a new one each time, or by releasing the previous
reference before returning a new one. Additionally, add a cleanup path in the
BloomView destructor to release the final reference when the view is destroyed.
In `@crates/perry-ui-tvos/src/widgets/bloomview.rs`:
- Around line 16-33: The tvOS create() function in bloomview.rs is missing the
main thread safety guard that exists in the iOS implementation. Add a
MainThreadMarker::new() check at the beginning of the function that returns 0 if
the guard fails (when Some variant is not present), before any UIKit operations.
This ensures the function respects UIKit's requirement that all view creation
and manipulation must occur on the main thread.
In `@types/perry/ui/index.d.ts`:
- Around line 446-447: The JSDoc comment in the file contains literal
asterisk-slash sequences (like NSView*/UIView*/GtkWidget*/ANativeWindow*) that
prematurely close the block comment because */ is interpreted as the end of the
JSDoc block. To fix this, escape or separate these sequences to prevent early
comment termination. You can insert a space between the asterisk and slash
(e.g., NSView* /UIView* /) or use other escaping techniques supported by JSDoc
parsers to ensure the comment text containing these character combinations is
properly preserved within the block comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d191eef-d5fe-43ab-a65f-f892e4087d64
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
CHANGELOG.mdCLAUDE.mdCargo.tomlcrates/perry-api-manifest/src/entries.rscrates/perry-dispatch/src/ui_table.rscrates/perry-ui-android/src/widgets/bloomview.rscrates/perry-ui-gtk4/src/widgets/bloomview.rscrates/perry-ui-ios/src/widgets/bloomview.rscrates/perry-ui-macos/src/widgets/bloomview.rscrates/perry-ui-tvos/src/ffi/media_extras.rscrates/perry-ui-tvos/src/widgets/bloomview.rscrates/perry-ui-tvos/src/widgets/mod.rscrates/perry-ui-visionos/src/widgets/bloomview.rsdocs/api/perry.d.tsdocs/src/api/reference.mdexamples/bloomview_embed_demo.tstypes/perry/ui/index.d.ts
Perry-UI side of #5519 (the engine side landed in Bloom-Engine/engine#71 as the platform-neutral bloom_attach_native). Makes a BloomView actually usable as a render surface on every backend, not just Windows. - Rename bloomViewGetHwnd -> bloomViewGetNativeHandle, keeping the old name as a deprecated alias (both dispatch rows route to the same perry_ui_bloomview_get_hwnd runtime symbol). Platform-neutral now that the handle is an NSView*/UIView*/GtkWidget*/ANativeWindow*, not only an HWND. Updated ui_table.rs, api-manifest entries.rs, types/perry/ui/index.d.ts, and regenerated docs/api/perry.d.ts + docs/src/api/reference.md. - Sizing: the non-Windows backends ignored the requested size, so the renderer's surface came up 0x0. BloomView(w,h) now pins the size on macOS/iOS/visionOS/ tvOS (Auto Layout) and Android (LayoutParams); GTK/Windows already did. - tvOS: promoted the 0-handle stub to a real UIView (new widgets/bloomview.rs). - Android: switched View -> SurfaceView; bloomViewGetNativeHandle now returns the real ANativeWindow* (ANativeWindow_fromSurface on the SurfaceHolder surface), 0 until the surface is ready. - Input/focus: the host view is now focusable so a focused BloomView receives key/pointer events for the attached engine (macOS acceptsFirstResponder via a PerryBloomView subclass; iOS/tvOS/visionOS userInteractionEnabled; GTK set_focusable; Android setFocusable). - examples/bloomview_embed_demo.ts: a live Bloom scene inside a Perry UI window (BloomView + bloomViewGetNativeHandle + attachToNSView, driven by onFrame). Verified on macOS: the integrated binary (Perry UI + Bloom engine) links and runs the attach/frame loop without crashing; iOS cross-compiles; tvOS/visionOS host-build; dispatch+manifest tests pass; no api-docs drift. Android/GTK/Windows are best-effort (no local cross-toolchain), mirroring established per-backend patterns.
proggeramlug
force-pushed
the
feat/bloomview-native-handle-5519
branch
from
9c1bf74 to
4d61bf7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
types/perry/ui/index.d.ts (1)
446-447:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix JSDoc text that breaks parsing at Line 446.
The
*/sequences inNSView*/UIView*/...close the block comment early, which causes the parse errors reported at Lines 446-447.Suggested minimal fix
- * is no longer Windows-only (it's an NSView*/UIView*/GtkWidget*/ANativeWindow* + * is no longer Windows-only (it's an `NSView*`, `UIView*`, `GtkWidget*`, or + * `ANativeWindow*`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@types/perry/ui/index.d.ts` around lines 446 - 447, The JSDoc block comment in the typedef section is being prematurely closed by the `*/` sequences appearing in the type names like NSView*/UIView*/GtkWidget*/ANativeWindow* on lines 446-447. To fix this, escape these character sequences by replacing the forward slashes with HTML entities (such as `&`#47`;`) or by adding spaces between the asterisk and forward slash (like `* /`) to prevent them from being interpreted as comment closers. This will allow the JSDoc parser to correctly interpret the full comment block without early termination.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@types/perry/ui/index.d.ts`:
- Around line 446-447: The JSDoc block comment in the typedef section is being
prematurely closed by the `*/` sequences appearing in the type names like
NSView*/UIView*/GtkWidget*/ANativeWindow* on lines 446-447. To fix this, escape
these character sequences by replacing the forward slashes with HTML entities
(such as `&`#47`;`) or by adding spaces between the asterisk and forward slash
(like `* /`) to prevent them from being interpreted as comment closers. This
will allow the JSDoc parser to correctly interpret the full comment block
without early termination.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 67257e11-4a39-4f8f-8f49-c70403fdfa0a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
CHANGELOG.mdCLAUDE.mdCargo.tomlcrates/perry-api-manifest/src/entries.rscrates/perry-dispatch/src/ui_table.rscrates/perry-ui-android/src/widgets/bloomview.rscrates/perry-ui-gtk4/src/widgets/bloomview.rscrates/perry-ui-ios/src/widgets/bloomview.rscrates/perry-ui-macos/src/widgets/bloomview.rscrates/perry-ui-tvos/src/ffi/media_extras.rscrates/perry-ui-tvos/src/widgets/bloomview.rscrates/perry-ui-tvos/src/widgets/mod.rscrates/perry-ui-visionos/src/widgets/bloomview.rsdocs/api/perry.d.tsdocs/src/api/reference.mdexamples/bloomview_embed_demo.tstypes/perry/ui/index.d.ts
✅ Files skipped from review due to trivial changes (6)
- crates/perry-ui-tvos/src/widgets/mod.rs
- CLAUDE.md
- CHANGELOG.md
- Cargo.toml
- docs/api/perry.d.ts
- docs/src/api/reference.md
🚧 Files skipped from review as they are similar to previous changes (10)
- crates/perry-ui-gtk4/src/widgets/bloomview.rs
- crates/perry-ui-tvos/src/widgets/bloomview.rs
- crates/perry-ui-tvos/src/ffi/media_extras.rs
- examples/bloomview_embed_demo.ts
- crates/perry-ui-ios/src/widgets/bloomview.rs
- crates/perry-ui-visionos/src/widgets/bloomview.rs
- crates/perry-api-manifest/src/entries.rs
- crates/perry-dispatch/src/ui_table.rs
- crates/perry-ui-macos/src/widgets/bloomview.rs
- crates/perry-ui-android/src/widgets/bloomview.rs
11 tasks
- android BloomView: cache the ANativeWindow* per handle instead of calling ANativeWindow_fromSurface (which returns a +1-refcount window) on every get_native_handle poll — that leaked a window reference per call. The first ready surface builds the window; later calls reuse the cached pointer. - tvos BloomView create(): add the MainThreadMarker main-thread guard the iOS path has (UIKit view creation must be on the main thread; return 0 otherwise). - types/perry/ui/index.d.ts: the deprecated bloomViewGetHwnd JSDoc had a raw `NSView*/UIView*/…` whose `*/` prematurely closed the block comment — backtick the type names so the sequence no longer terminates the comment.
Contributor
Author
|
Thanks @coderabbitai — all three addressed in 553125f:
Verified: tvOS + the Android crate both build on host; the macOS path is unchanged. |
|
Tip For best results, initiate chat on the files or code changes.
Appreciate the verification on both host builds. This looks good to go! 🐇✨ |
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
1 participant
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.
Perry-UI side of #5519. The engine side landed in Bloom-Engine/engine#71 (the platform-neutral
bloom_attach_native); this makes aBloomViewactually usable as a render surface on every backend, not just Windows.Changes
bloomViewGetHwnd→bloomViewGetNativeHandle, keepingbloomViewGetHwndas a deprecated alias (both dispatch rows route to the sameperry_ui_bloomview_get_hwndruntime symbol — no FFI symbol churn). Platform-neutral now that the handle is anNSView*/UIView*/GtkWidget*/ANativeWindow*, not only anHWND. Touchesui_table.rs, api-manifestentries.rs,types/perry/ui/index.d.ts, and the regenerateddocs/api/perry.d.ts+docs/src/api/reference.md.BloomView(w, h)now pins the size — macOS/iOS/visionOS/tvOS via Auto Layout (set_width/set_height+translatesAutoresizingMaskIntoConstraints = false), Android viaLayoutParams; GTK4/Windows already did.UIView(newcrates/perry-ui-tvos/src/widgets/bloomview.rs), mirroring iOS.android.view.View→android.view.SurfaceView;bloomViewGetNativeHandlenow returns the realANativeWindow*(NDKANativeWindow_fromSurfaceon the view'sSurfaceHolder.getSurface()) instead of echoing the registry token. Returns 0 until the surface is laid out /surfaceCreated.BloomViewreceives keyboard/pointer events for the attached engine to consume (macOSacceptsFirstRespondervia aPerryBloomViewsubclass; iOS/tvOS/visionOSuserInteractionEnabled; GTKset_focusable/set_can_target; AndroidsetFocusable/setFocusableInTouchMode).examples/bloomview_embed_demo.ts— a live Bloom scene inside a Perry UI window (BloomView+bloomViewGetNativeHandle+ the engine'sattachToNSView, driven fromonFrame).Verification
bloomViewGetNativeHandlelowers toperry_ui_bloomview_get_hwnd.perry-dispatch+perry-api-manifesttests pass (36); no api-docs drift; my changed files are clippy-clean.Follow-ups (not blocking)
Full keyboard/mouse event forwarding into the engine is the engine's job once attached (on Windows Bloom subclasses the host wndproc); this PR makes the views focusable/input-capable. Android's surface-ready callback (
surfaceCreated) deferral and the real GTK4GdkSurfacebridge remain per the issue.Closes the Perry-UI checklist of #5519.
Summary by CodeRabbit
Release Notes v0.5.1199
New Features
bloomViewGetNativeHandle()for attaching Bloom rendering to the native BloomView surface, withbloomViewGetHwnd()retained as a deprecated alias.Bug Fixes
Updates
UIViewhost; iOS/macOS/visionOS apply sizing properly.