[browser] Marshalling support float[], Span<float> and ArraySegment<float>#123642
[browser] Marshalling support float[], Span<float> and ArraySegment<float>#123642pavelsavara merged 16 commits intodotnet:mainfrom
Conversation
|
@ArcadeMode feel free to ping me on your PRs, so that I can label it properly. |
|
@pavelsavara to verify that i am following contribution guidelines: I have already changed and implemented an API change, is it okay to ask for API review in PR like this? i.e. would a similar approach suffice for #97381 or should this be done through the issue or otherwise? |
There was a problem hiding this comment.
Pull request overview
This pull request adds marshaling support for float[], Span<float>, and ArraySegment<float> between C# and JavaScript in the browser WASM interop layer, addressing issue #97380.
Changes:
- Added
MemoryViewType.Singleenum value and corresponding Float32Array handling - Implemented C# marshaling methods for float arrays, spans, and array segments
- Updated TypeScript marshaling code to handle Single type in both native and mono implementations
- Added comprehensive test coverage for JSImport and JSExport scenarios with float types
- Updated code generator to recognize Single type patterns
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshaled-types.ts | Added MemoryViewType.Single enum and Float32Array view creation |
| src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts | Added Single to elementSizeMap with 4-byte size |
| src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts | Implemented Single type marshaling for arrays, spans, and array segments |
| src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-cs.ts | Implemented Single type unmarshaling and view type checking |
| src/mono/browser/runtime/marshal.ts | Added Single to array_element_size function and MemoryViewType enum, refactored to if-else chains |
| src/mono/browser/runtime/marshal-to-js.ts | Implemented Single type marshaling for arrays, spans, and array segments in mono runtime |
| src/mono/browser/runtime/marshal-to-cs.ts | Implemented Single type unmarshaling and view type checking in mono runtime |
| src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Single.cs | Added ToJS/ToManaged methods for float[], Span, and ArraySegment |
| src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Object.cs | Added float[] handling in ToJS method for object parameters |
| src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSMarshalerType.cs | Updated validation methods to accept Single type in arrays, spans, and segments; removed extraneous blank line |
| src/libraries/System.Runtime.InteropServices.JavaScript/ref/System.Runtime.InteropServices.JavaScript.cs | Added public API surface for float[], Span, and ArraySegment marshaling |
| src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSGeneratorFactory.cs | Added generator patterns for Single type arrays, spans, and array segments |
| src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JavaScriptTestHelper.cs | Added test helper methods for Single type JSImport and JSExport scenarios |
| src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSInteropTestBase.cs | Added MarshalSingleArrayCases test data |
| src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSImportTest.cs | Added comprehensive tests for float arrays, spans, and array segments; improved variable naming in Double/Int32 tests |
| src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSExportTest.cs | Added JSExport tests for Span and ArraySegment |
...cript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Object.cs
Show resolved
Hide resolved
The process is described here https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md You need to create dedicated issue according to the issue template for API suggestion. You can also look at successfully approved APIs |
|
The code looks good. I added |
...teropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSImportTest.cs
Outdated
Show resolved
Hide resolved
...vices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JavaScriptTestHelper.cs
Show resolved
Hide resolved
|
Please add few float signatures so that the generated code appears in |
…System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSImportTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@pavelsavara just a heads up that I've adressed your review comment |
🤖 Copilot Code Review — PR #123642Holistic AssessmentMotivation: This PR addresses a legitimate gap in the JS interop marshalling layer. Issue #97380 and #123706 clearly document the need for Approach: The implementation correctly mirrors the existing Summary: Detailed Findings✅ C# Marshalling Implementation — CorrectThe
✅ TypeScript MemoryViewType Enum Alignment — CorrectBoth export const enum MemoryViewType {
Byte = 0,
Int32 = 1,
Double = 2,
Single = 3, // New
}The values are consistent between Mono and CoreCLR implementations. ✅ fixupPointer Alignment — CorrectThe TypeScript uses ✅ JSMarshalerType Validation — Correct
✅ Object Marshalling — Correct
|
- minor improvements
|
/ba-g CI failures are OOM on helix (exit code 137) |
Fixes #97380.
Fixes #123706
Implementation is functional for CoreCLR and Mono. Added import and export tests to demonstrate.
How do I go about updating the docs? (and which version might see these changes for that matter, net10/11?)