Repository navigation
Enable SAGAMetal by default and validate Studio Web export - #214
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 889d96ce1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public func getImage(from _: Texture) -> Image? { | ||
| // Readback is asynchronous; use `readImage(from:)`. | ||
| nil |
There was a problem hiding this comment.
With SAGA now the default Apple renderer, returning nil here breaks existing consumers of Image(texture:). In particular, EditorProjectPreviewCapture.capture in EditorProjectPreviewStore.swift still uses synchronous readback for both saved project thumbnails and requested frames, so thumbnails stop updating and frame requests never receive pixels. EditorTextureAtlasAssetEditor.reloadPreview likewise receives an empty image. Migrate these consumers to the asynchronous readback path, or preserve synchronous readback support, before switching the default backend.
Useful? React with 👍 / 👎.
| let image = resource.image | ||
| let bytesPerRow = pixelFormat.bytesPerComponent * image.width | ||
| let data = unsafe image.data.withUnsafeBytes { unsafe [UInt8]($0) } | ||
| write( | ||
| data, | ||
| bytesPerRow: bytesPerRow, |
There was a problem hiding this comment.
For Texture2D(image:) constructed from an .rgb8 image, Image.Format.toPixelFormat selects .rgba8, but this code uploads the original three-byte-per-pixel data with a four-byte-per-pixel row stride. The upload therefore has insufficient data and cannot reproduce the image correctly. This regresses the default Apple path: MetalGPUTexture explicitly expands RGB8 data to RGBA8 before uploading. Apply the same conversion here, including subresource uploads.
Useful? React with 👍 / 👎.
| return try await withCheckedThrowingContinuation { continuation in | ||
| context.queue.enqueue { device in |
There was a problem hiding this comment.
If the asynchronous SAGA device initializer throws, OrderedGPUWorkQueue skips device jobs and invokes only their optional onUnavailable handlers. This enqueue supplies no such handler, leaving the checked continuation unresolved, so readData() hangs instead of throwing. SAGAGPUTexture.readImage() has the same failure path. Supply unavailable-device cleanup that resumes each readback continuation with an error, as command-buffer completion already does.
Useful? React with 👍 / 👎.
| @MainActor | ||
| struct WESLUIPipelineTests { |
There was a problem hiding this comment.
When ADAENGINE_VALIDATE_WGSL=1 enables these tests in a parallel test run, this standalone suite can call setupRenderEngine() while a Render3DGPUTests test is suspended awaiting GPU work, replacing the process-wide renderer underneath that test. WESLGradientPipelineTests has the same issue. Nest both suites under Render3DGPUTests so they participate in its serialization; @MainActor alone does not prevent interleaving across suspension points. This also violates the explicit GPU-suite placement requirement in AGENTS.md.
AGENTS.md reference: AGENTS.md:L201-L202
Useful? React with 👍 / 👎.
Enable SAGAMetal on macOS, iOS and tvOS and pin SAGA for Metal and Web. Keep the legacy Metal flag for comparison and keep the native Vulkan paths opt-in.
Local validation covered macOS Studio and native export, the iPad Studio simulator, tvOS device and simulator builds, and Chromium WebGPU for Debug and Release Studio exports. Apple CI includes the packaged Studio Web export.