feat(ui): BloomView — embed the Bloom engine in a Perry UI app (#2395)#5518
Merged
Conversation
…nside a Perry UI app (#2395) Adds a `BloomView(width, height)` Perry UI widget (Windows) that reserves a native child window in the view tree and exposes its raw HWND via `bloomViewGetHwnd(view)`. An external renderer such as the Bloom engine then builds its surface on that window and drives frames from the host's run loop. perry-ui-windows itself never links or references Bloom — it only owns the window and hands out the handle, so apps that don't use BloomView pull in nothing extra. The Bloom-side embedding (`attachToHwnd`/`resize` FFI) lives in the engine repo. - widget: crates/perry-ui-windows/src/widgets/bloomview.rs (+ widgets/mod.rs, ffi/widget_create.rs) - table-driven like Canvas, no codegen special-case: perry-dispatch ui_table rows + perry-api-manifest entries (BloomView, bloomViewGetHwnd) - types: BloomView/bloomViewGetHwnd (+ onFrame/cancelFrame) in perry/ui d.ts Windows-only for now; codegen/manifest/dispatch are shared, so wiring other platform UI crates is an additive follow-up. Per request, no version bump or CHANGELOG entry — maintainer to add at merge if desired. Claude-Session: https://claude.ai/code/session_011z6cNYoSsNG4ffLYLXYRwc
|
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 (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds ChangesBloomView Cross-Platform Widget Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 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: 1
🤖 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-windows/src/widgets/bloomview.rs`:
- Around line 49-74: The ensure_class_registered function ignores the return
value of RegisterClassExW (which may fail) and still marks the class as
registered, and the calls to GetModuleHandleW use unwrap() which will panic on
failure. Since this function is called from the FFI entrypoint
perry_ui_bloomview_create, panics will cross the FFI boundary and crash the
process. Replace the unwrap() call on GetModuleHandleW with proper error
handling that returns early or logs an error, check the return value of
RegisterClassExW and only set r.set(true) if the registration succeeds, and
ensure all Win32 function calls within ensure_class_registered are wrapped with
appropriate error handling instead of unwrap().
🪄 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: 32a80fb5-eb7e-4eb8-b85b-72b2e1e57fbd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/perry-api-manifest/src/entries.rscrates/perry-dispatch/src/ui_table.rscrates/perry-ui-windows/src/ffi/widget_create.rscrates/perry-ui-windows/src/widgets/bloomview.rscrates/perry-ui-windows/src/widgets/mod.rstypes/perry/ui/index.d.ts
Comment on lines
+49
to
+74
| fn ensure_class_registered() { | ||
| thread_local! { | ||
| static REGISTERED: std::cell::Cell<bool> = const { std::cell::Cell::new(false) }; | ||
| } | ||
| REGISTERED.with(|r| { | ||
| if r.get() { | ||
| return; | ||
| } | ||
| unsafe { | ||
| let hinstance = GetModuleHandleW(None).unwrap(); | ||
| let class_name = to_wide("PerryBloomView"); | ||
| let wc = WNDCLASSEXW { | ||
| cbSize: std::mem::size_of::<WNDCLASSEXW>() as u32, | ||
| style: CS_HREDRAW | CS_VREDRAW, | ||
| lpfnWndProc: Some(bloom_host_wndproc), | ||
| hInstance: HINSTANCE(hinstance.0), | ||
| lpszClassName: windows::core::PCWSTR(class_name.as_ptr()), | ||
| // No background brush: the renderer fills the whole client area. | ||
| hbrBackground: HBRUSH(std::ptr::null_mut()), | ||
| ..Default::default() | ||
| }; | ||
| RegisterClassExW(&wc); | ||
| } | ||
| r.set(true); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'RegisterClassExW|CreateWindowExW|GetModuleHandleW|unwrap\(' crates/perry-ui-windows/src/widgets/bloomview.rs -C2
rg -n 'pub extern "C" fn perry_ui_bloomview_create|pub extern "C" fn perry_ui_bloomview_get_hwnd' crates/perry-ui-windows/src/ffi/widget_create.rs -C2Repository: PerryTS/perry
Length of output: 1437
🏁 Script executed:
cat -n crates/perry-ui-windows/src/widgets/bloomview.rs | head -120Repository: PerryTS/perry
Length of output: 5536
🏁 Script executed:
rg -n 'unwrap|panic|abort' crates/perry-ui-windows/src/widgets/bloomview.rsRepository: PerryTS/perry
Length of output: 268
Handle Win32 failures without panicking across FFI boundary
Line 70 ignores RegisterClassExW failure and still marks the class as registered. Lines 58, 85, and 100 call .unwrap() on Win32 functions, which will panic if these calls fail. Since create() is called from the pub extern "C" fn perry_ui_bloomview_create FFI entrypoint, panics will propagate across the FFI boundary and abort the process.
Proposed fix
-fn ensure_class_registered() {
+fn ensure_class_registered() -> bool {
thread_local! {
static REGISTERED: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
}
- REGISTERED.with(|r| {
+ REGISTERED.with(|r| {
if r.get() {
- return;
+ return true;
}
unsafe {
- let hinstance = GetModuleHandleW(None).unwrap();
+ let Ok(hinstance) = GetModuleHandleW(None) else { return false; };
let class_name = to_wide("PerryBloomView");
let wc = WNDCLASSEXW {
@@
- RegisterClassExW(&wc);
+ let atom = RegisterClassExW(&wc);
+ if atom == 0 {
+ let err = GetLastError();
+ if err != ERROR_CLASS_ALREADY_EXISTS {
+ return false;
+ }
+ }
}
r.set(true);
- });
+ true
+ })
}
@@
- ensure_class_registered();
+ if !ensure_class_registered() {
+ return 0;
+ }
@@
- let hinstance = GetModuleHandleW(None).unwrap();
- let hwnd = CreateWindowExW(
+ let Ok(hinstance) = GetModuleHandleW(None) else { return 0; };
+ let hwnd = match CreateWindowExW(
WINDOW_EX_STYLE::default(),
@@
- )
- .unwrap();
+ ) {
+ Ok(hwnd) => hwnd,
+ Err(_) => return 0,
+ };🤖 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 `@crates/perry-ui-windows/src/widgets/bloomview.rs` around lines 49 - 74, The
ensure_class_registered function ignores the return value of RegisterClassExW
(which may fail) and still marks the class as registered, and the calls to
GetModuleHandleW use unwrap() which will panic on failure. Since this function
is called from the FFI entrypoint perry_ui_bloomview_create, panics will cross
the FFI boundary and crash the process. Replace the unwrap() call on
GetModuleHandleW with proper error handling that returns early or logs an error,
check the return value of RegisterClassExW and only set r.set(true) if the
registration succeeds, and ensure all Win32 function calls within
ensure_class_registered are wrapped with appropriate error handling instead of
unwrap().
- Regenerate docs/src/api/reference.md and docs/api/perry.d.ts from the API manifest so the BloomView / bloomViewGetHwnd entries no longer drift (fixes the api-docs-drift check). - bloomview.rs: don't unwrap GetModuleHandleW / CreateWindowExW across the FFI boundary — return early / an invalid (0) handle on failure instead of panicking the process (CodeRabbit). Claude-Session: https://claude.ai/code/session_011z6cNYoSsNG4ffLYLXYRwc
Contributor
Author
|
Addressed in f4c42d4:
|
Adds the `perry_ui_bloomview_create` / `perry_ui_bloomview_get_hwnd` exports (and a widget module where applicable) to every Perry UI backend, so a BloomView app links and runs on all targets, not just Windows: - macOS / iOS / visionOS: register a bare NSView/UIView; the getter returns the raw view pointer. - GTK4 (Linux): a GtkDrawingArea; getter returns the GtkWidget* (the issue's MVP target for Vulkan dmabuf embedding). - Android: a bare android.view.View via JNI; getter echoes the handle token (no HWND on Android; a SurfaceView + JNI surface bridge is the follow-up). - tvOS / watchOS: link-stability stubs returning 0, mirroring how those crates already stub WebView. - wasm/web: a <canvas> element registered in __perryUiDispatch; getter returns the handle id. Each mirrors that platform's existing WebView idiom. The native-handle meaning is platform-specific (HWND / NSView* / UIView* / GtkWidget* / handle token). Verified compiling locally: perry-codegen-wasm and perry-ui-windows. The other UI crates are excluded from the blocking Linux CI and require their native hosts to compile (verified by mirroring confirmed crate patterns). Claude-Session: https://claude.ai/code/session_011z6cNYoSsNG4ffLYLXYRwc
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 52-54: The `get_native_handle` function currently returns the
handle value without validation, but it should validate the handle first using
`super::get_widget(handle)` and return 0 if the handle is unknown or invalid
(matching the web peer behavior). If the handle is valid/found in the registry,
return the handle value. This prevents invalid or stale handles from being
treated as attachable by downstream renderers.
- Around line 14-22: The `create` function receives `_width` and `_height`
parameters that are currently unused (indicated by the underscore prefix), but
these dimensions are not being applied to the Android View being created via
`env.new_object`. Remove the underscore prefixes from the width and height
parameters to indicate they will be used, then modify the View construction to
apply these dimensions when creating the View object so that the Android host
respects the intended size rather than defaulting to zero or a default size.
In `@crates/perry-ui-gtk4/src/widgets/bloomview.rs`:
- Around line 17-19: The condition checking `width > 0.0 && height > 0.0` before
calling `area.set_size_request(width as i32, height as i32)` does not adequately
validate the dimensions. Non-finite values like NaN or infinity can pass this
check and produce invalid GTK size requests, while very small positive fractions
will truncate to zero when cast to i32. Add validation logic before the
set_size_request call to clamp width and height values to a reasonable, safe
range ensuring they are finite and within sensible pixel bounds, preventing
invalid size requests and ensuring the widget receives meaningful dimensions.
In `@crates/perry-ui-ios/src/widgets/bloomview.rs`:
- Around line 16-20: The create function in bloomview.rs lacks thread-safety
verification required for iOS and visionOS platforms. Add a MainThreadMarker
guard parameter to the create function signature to ensure the UIView is only
created and registered on the main thread, matching the pattern used in the
macOS version and the 50+ other creation functions across iOS and visionOS
platforms. This prevents the scenario where a UIView created on a background
thread would store an unreachable handle when registered in thread-local
storage.
In `@crates/perry-ui-macos/src/widgets/bloomview.rs`:
- Around line 15-19: The create function uses .expect() on
MainThreadMarker::new(), which will panic if called off the main thread. Since
this is a public C ABI function (perry_ui_bloomview_create), replace the
.expect() call with an explicit None check that returns 0 to represent a failure
handle, ensuring graceful failure handling consistent with other widget creation
functions rather than aborting the entire process.
In `@crates/perry-ui-visionos/src/widgets/bloomview.rs`:
- Around line 16-20: The create function in bloomview.rs does not include a
MainThreadMarker guard, which is required for safe UIKit view creation on the
main thread. Add a MainThreadMarker parameter to the create function signature
(following the pattern of all other 26+ visionOS widgets), and use it to guard
the unsafe block where the UIView is created and registered. This ensures that
background threads cannot register widget handles through register_widget that
the UI thread cannot later resolve from the thread-local WIDGETS registry.
🪄 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: e4d775d2-27e9-491e-a9e3-81c1bd924881
📒 Files selected for processing (19)
crates/perry-codegen-wasm/src/wasm_runtime.jscrates/perry-ui-android/src/ffi/issue_553.rscrates/perry-ui-android/src/widgets/bloomview.rscrates/perry-ui-android/src/widgets/mod.rscrates/perry-ui-gtk4/src/ffi/stubs_webview_attrtext_screenshot.rscrates/perry-ui-gtk4/src/widgets/bloomview.rscrates/perry-ui-gtk4/src/widgets/mod.rscrates/perry-ui-ios/src/ffi/widgets_basic.rscrates/perry-ui-ios/src/widgets/bloomview.rscrates/perry-ui-ios/src/widgets/mod.rscrates/perry-ui-macos/src/lib_ffi/interactivity.rscrates/perry-ui-macos/src/widgets/bloomview.rscrates/perry-ui-macos/src/widgets/mod.rscrates/perry-ui-tvos/src/ffi/media_extras.rscrates/perry-ui-visionos/src/ffi_core.rscrates/perry-ui-visionos/src/widgets/bloomview.rscrates/perry-ui-visionos/src/widgets/mod.rscrates/perry-ui-watchos/src/lib.rstypes/perry/ui/index.d.ts
✅ Files skipped from review due to trivial changes (4)
- crates/perry-ui-android/src/widgets/mod.rs
- crates/perry-ui-macos/src/widgets/mod.rs
- crates/perry-ui-ios/src/widgets/mod.rs
- types/perry/ui/index.d.ts
Comment on lines
+14
to
+22
| pub fn create(_width: f64, _height: f64) -> i64 { | ||
| let mut env = jni_bridge::get_env(); | ||
| let _ = env.push_local_frame(8); | ||
|
|
||
| let activity = super::get_activity(&mut env); | ||
| let view = match env.new_object( | ||
| "android/view/View", | ||
| "(Landroid/content/Context;)V", | ||
| &[JValue::Object(&activity)], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect Android widget implementations for dimension/layout handling.
# Expectation: Either BloomView applies width/height here, or another Android layout path clearly applies them for registered widgets.
rg -n -C3 'setMinimumWidth|setMinimumHeight|setLayoutParams|LayoutParams|fn create\(.*width|_width|_height' crates/perry-ui-android/src/widgetsRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
cat -n crates/perry-ui-android/src/widgets/bloomview.rsRepository: PerryTS/perry
Length of output: 2237
Apply the requested BloomView size on Android.
BloomView(width, height) receives dimensions, but Android drops both values before registering a plain View. Unless another Android layout layer applies these dimensions, this can reserve a zero/default-sized host while GTK/web peers size their render surfaces.
Possible localized fix direction
-pub fn create(_width: f64, _height: f64) -> i64 {
+pub fn create(width: f64, height: f64) -> i64 {
let mut env = jni_bridge::get_env();
let _ = env.push_local_frame(8);
let activity = super::get_activity(&mut env);
let view = match env.new_object(
"android/view/View",
"(Landroid/content/Context;)V",
&[JValue::Object(&activity)],
) {
Ok(v) => v,
Err(_) => {
unsafe {
let _ = env.pop_local_frame(&jni::objects::JObject::null());
}
return 0;
}
};
+
+ if width > 0.0 {
+ let _ = env.call_method(
+ &view,
+ "setMinimumWidth",
+ "(I)V",
+ &[JValue::Int(width.min(i32::MAX as f64) as i32)],
+ );
+ }
+ if height > 0.0 {
+ let _ = env.call_method(
+ &view,
+ "setMinimumHeight",
+ "(I)V",
+ &[JValue::Int(height.min(i32::MAX as f64) as i32)],
+ );
+ }🤖 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 `@crates/perry-ui-android/src/widgets/bloomview.rs` around lines 14 - 22, The
`create` function receives `_width` and `_height` parameters that are currently
unused (indicated by the underscore prefix), but these dimensions are not being
applied to the Android View being created via `env.new_object`. Remove the
underscore prefixes from the width and height parameters to indicate they will
be used, then modify the View construction to apply these dimensions when
creating the View object so that the Android host respects the intended size
rather than defaulting to zero or a default size.
- macOS/iOS/visionOS: guard create() with `MainThreadMarker::new()` and return 0 off the main thread instead of `.expect()`-panicking across the FFI boundary (UIKit/AppKit views must be made on the main thread). - GTK4: validate width/height are finite and >= 1 and clamp before set_size_request, so NaN/inf/sub-pixel values don't produce a bogus request. - Android: get_native_handle now validates the handle via get_widget and returns 0 for unknown/stale handles (matches the web peer). (Skipped the Android create() width/height nit: Android WebView create() likewise ignores them — sizing is applied via the layout system, not at View construction.) Claude-Session: https://claude.ai/code/session_011z6cNYoSsNG4ffLYLXYRwc
Contributor
Author
|
Addressed the 6 review comments on the cross-platform commit in 2bb5a59:
Engine-side support is up as a separate PR: Bloom-Engine/engine#70. |
11 tasks
proggeramlug
added a commit
to Bloom-Engine/engine
that referenced
this pull request
Jun 22, 2026
…rry UI BloomView, #2395) (#70) Lets Bloom render into a host-provided child window instead of always creating its own top-level window — the engine side of the Perry UI `BloomView` widget (PerryTS/perry#5518), analogous to Flame inside Flutter. - native/windows/src/lib.rs: factor the wgpu surface + engine init out of bloom_init_window into init_engine_for_hwnd, shared with a new FFI bloom_attach_hwnd(hwnd, w, h). It builds the surface on the host HWND and classic-subclasses it (WM_SIZE/keyboard/mouse → engine). Add bloom_resize, an EMBEDDED flag (begin_drawing skips its own message pump; should_close reports "stay open" since the host owns the loop). - src/core/index.ts: attachToHwnd / resize TS wrappers (+ src/index.ts export). - package.json: declare bloom_attach_hwnd / bloom_resize in the native-library function manifest. - embed-perry/: a demo Perry UI app embedding a live Bloom 3D scene (requires the BloomView Perry build from PerryTS/perry#5518 to compile/run). Windows only for now; per-platform attach (NSView/UIView/dmabuf) is a follow-up. Co-authored-by: Ralph Kuepper <ralph@skelpo.com>
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.
Closes #2395.
Adds a
BloomViewPerry UI widget that embeds an external GPU renderer — the Bloom game engine — as a live render view inside a normal Perry UI app, the way the issue asks for (analogous to Flame inside Flutter).Design
BloomView(width, height)reserves a nativeWS_CHILDwindow in the Perry UI view tree and exposes its raw HWND viabloomViewGetHwnd(view). User TypeScript hands that HWND to the Bloom package (attachToHwnd), which builds its wgpu (DX12/Vulkan) surface on the window, subclasses it for resize/input, and renders into it. The host (Perry UI) owns the message loop, so the app drives Bloom's frame loop itself.Key choice:
perry-ui-windowsnever links or references Bloom — it only owns the window and hands out the handle. Apps that don't callBloomViewpull in nothing extra; Bloom is glued in entirely from user TS. This mirrors how WebView embeds a self-managing surface, but keeps the engine dependency out of the UI crate.What's in this PR (Perry side)
crates/perry-ui-windows/src/widgets/bloomview.rs(new) — child-HWND host +bloomViewGetHwnd. ReusesWidgetKind::Image, with non-Windows fallbacks.widgets/mod.rs,ffi/widget_create.rs— module +perry_ui_bloomview_create/_get_hwndFFI.Canvas(no codegen special-casing):perry-dispatch/src/ui_table.rsrows +perry-api-manifest/src/entries.rsentries forBloomView/bloomViewGetHwnd.types/perry/ui/index.d.ts—BloomView/bloomViewGetHwnd(and the previously-undeclaredonFrame/cancelFrame).The Bloom-side embedding support (
bloom_attach_hwnd/bloom_resizeFFI +attachToHwndwrappers) lives in the engine repo and is needed for the demo to run.Status / scope
https://claude.ai/code/session_011z6cNYoSsNG4ffLYLXYRwc
Summary by CodeRabbit