Add MToon rendering support#49
Conversation
![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 a comprehensive MToon shader and material system for both VRMRealityKit and VRMSceneKit, enabling consistent rendering, outline generation, and material/expression animations across platforms. It also updates the example applications and adds extensive unit tests to verify the new rendering pipelines. Feedback on the changes includes expanding the conditional compilation check in VRMEntityLoader to support physical visionOS devices without failing on runtime shader compilation, and optimizing the vertex morph target calculations in SCNMorpher+GLTF.swift by replacing manual memcpy loops with faster unaligned loads.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
soramikan
force-pushed
the
add-mtoon-support
branch
from
bd6bdf8 to
76ccb56
Compare
soramikan
force-pushed
the
add-mtoon-support
branch
from
76ccb56 to
c6abf9a
Compare
There was a problem hiding this comment.
Thanks for the MToon support work. I reviewed this mainly from the perspective of spec correctness, implementation approach, and PR scope. Before merging, I would like the following items addressed or clarified.
-
Please add visual comparison screenshots or captures for representative models before and after this PR. Since this PR changes shader/material behavior across SceneKit, RealityKit, iOS, macOS, and visionOS examples, it is difficult to review the rendering quality from code alone. Screenshots showing the previous rendering vs. the new MToon rendering would make the review much easier.
-
For parts of the MToon spec that are intentionally not implemented yet, please leave explicit comments in the code near the fallback/omission. For example, if a behavior is approximated due to RealityKit or SceneKit limitations, it should be visible to future maintainers as an intentional limitation rather than looking like an accidental omission.
-
The PR diff is quite large. Please split changes that are not directly related to MToon shader/material support into separate PRs. In particular, the iOS Example SceneDelegate migration and the Watch Example embed/dependency removal appear unrelated to the core MToon work and make this PR harder to review safely.
-
The MToon mask texture channels do not appear to match the VRMC_materials_mtoon 1.0 spec in the RealityKit shader.
outlineWidthMultiplyTextureanduvAnimationMaskTextureare both sampled from.r, but the spec assigns these packed mask channels separately:shadingShiftTextureuses R,outlineWidthMultiplyTextureuses G, anduvAnimationMaskTextureuses B. Please updatemtoonOutlineGeometryto read the outline width mask from.g, andmtoonAnimatedUVto read the UV animation mask from.b. It would also be useful to add a test texture with distinct R/G/B values so this behavior is covered by tests. -
SceneKit appears to parse
outlineWidthMultiplyTextureanduvAnimationMaskTexture, but they are not effectively applied in the shader modifiers.uvAnimationMaskTextureis assigned to a material property, but the geometry modifier usesmtoonMask = 1.0.outlineWidthMultiplyTexturealso does not seem to participate in the outline width calculation. Please make the SceneKit path apply the UV animation mask and outline width mask as defined by MToon, or explicitly document/comment any unsupported behavior. -
On physical iOS devices, sampler variants do not appear to be reflected because runtime Metal source compilation is skipped and the code falls back to the precompiled default library. That means texture sampler settings such as
CLAMP_TO_EDGEandMIRRORED_REPEATmay render differently on device compared with macOS/simulator. Please either make sampler variants work consistently on device, or clearly document that only the default sampler behavior is supported in that environment. -
The runtime shader header lookup includes hard-coded Xcode installation paths such as
/Applications/Xcode.app/Contents/Developerand/Applications/Xcode-beta.app/Contents/Developer. This is fragile for a library implementation because Xcode may be installed elsewhere, CI environments may differ, and sandboxed/distributed apps should not rely on probing local developer tool paths. Please avoid hard-coded/Applications/Xcode*.appfallback paths. If runtime compilation must remain, prefer explicitly provided build environment paths such asSDKROOT,DEVELOPER_DIR, orxcrun, and consider limiting this path to debug/test fallback behavior. -
The current tests mostly verify that materials are created or that shader source contains expected strings. Please add tests that catch MToon spec-level behavior, especially packed mask channel usage, SceneKit mask application, sampler fallback behavior, and alpha/depth behavior such as
transparentWithZWrite. -
renderQueueOffsetNumberis parsed intoMToonMaterialDescriptor, but I do not see it reflected in SceneKit or RealityKit draw ordering. If this is intentionally unsupported as a fallback, please document that clearly. Otherwise, please wire it into the closest available draw-order mechanism, such as SceneKitrenderingOrderwhere appropriate. -
Several SceneKit MToon texture assignments use
try?, which silently ignores texture loading failures. For shader-critical textures, silent fallback makes broken texture indices or loader inconsistencies hard to diagnose. Please either propagate the error or log enough context, such as material name, texture index, and MToon property name.
tattn
left a comment
tattn
left a comment
There was a problem hiding this comment.
Inline review comments for the MToon implementation and PR scope.
|
Despite the difficulty of reviewing, thank you for your careful review. |
|
Thank you for the detailed review. I addressed the review items in the MToon scope:
RealityKit and SceneKit still approximate parts of UniVRM/MToon where the rendering APIs do not expose the same pipeline, but those limitations are now called out near the fallback behavior. Visual comparison screenshots: |
|
Thanks for addressing the above comments. VRMKit is mainly intended for apps distributed through the App Store, especially iOS and macOS apps. The current implementation depends on compiling the RealityKit shader source at runtime in some environments. This likely only works on developer machines, and is not something I can rely on for App Store-distributed or sandboxed macOS apps. With this approach, MToon rendering may fail or fall back outside the development environment. Is it possible to change this so the RealityKit MToon shaders are available through an App Store-safe mechanism, such as a reliably bundled precompiled Metal library? |
|
Thanks for pointing this out. |
|
Thank you for addressing this. If this is still a work in progress, I apologize for reviewing too early. First, could you explain how you verified the behavior? In my environment, Also, in the iOS sample app, the model orientation is rotated by 180 degrees. Please check the visual result of the sample apps on each platform supported by this library, not only on a specific target like macOS. This is a large change that significantly affects rendering results. Depending on the model or runtime environment, there may be cases where the previous default rendering is preferable. Because of that, I would like to make MToon rendering optional, so users can switch between the existing default rendering and MToon rendering via an option instead of always enabling MToon rendering. Please also review the GitHub diff again and remove any unnecessary changes. For example, the empty-line removal in Regarding the tests, it looks like an important RealityKit MToon path is being skipped with a warning and treated as a success. If the MToon shader cannot be loaded, the test should not pass; it should detect the problem. Please let me know when this is ready for review again. |
Ship per-platform precompiled MToon Metal libraries as package resources so CustomMaterial works regardless of the consumer's build system (swift build does not compile package .metal sources, and newer Xcodes require a separately installed Metal Toolchain). Add an isMToonEnabled option (default true) to both loaders, make shader-load test failures hard failures, unify the example model rotation offsets, and drop unrelated whitespace changes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thank you for the review. I have addressed all the points you raised — could you please take another look?
After investigating, the root cause was in how the shader was being built. Following Apple's guidance (TN3133: CustomMaterial shaders must be compiled offline), I switched to bundling precompiled per-platform metallibs as resources:
With this, MToon works regardless of the consumer's build environment (swift build / Xcode / presence of the Metal Toolchain). To verify it is actually applied at runtime, I confirmed via runtime logging that the
(Screenshots attached)
Introducing a separate rotation offset for RealityKit (
Added an let loader = try VRMEntityLoader(named: "model.vrm", isMToonEnabled: false)
Removed all the places where tests were skipped with a warning when the shader library could not be loaded; they now fail hard (
Removed whitespace-only changes ( Test results
Sorry for the trouble, and thank you for reviewing again. 
|
|
As an addendum, Alicia uses UnlitTexture and does not specify MToon, so it was normal that it was not rendered in MToon. |
Introduce VRMRenderingMode (.nonAR default, .ar) to omit geometry modifiers and outline entities that crash in AR shadow-caster passes, while keeping CustomMaterial MToon shading. Document AR usage in README and add integration tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply DynamicLightShadowComponent and GroundingShadowComponent on AR entity trees, force opaque blending for surface-only MToon, and expand AR integration tests. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The problem of crashing in AR occurred due to adding MToon rendering, so it was fixed. |
| loader: VRMSceneLoader) { | ||
| if let shadeColor = mtoon.shadeColorFactor { | ||
| multiply.contents = SKColor(color3: shadeColor, alpha: 1.0) | ||
| private func applyMToon(_ mtoon: MToonMaterialDescriptor, loader: VRMSceneLoader) throws { |
There was a problem hiding this comment.
This PR adds a fairly large MToon implementation on the SceneKit side, which makes review and maintenance much harder.
However, SceneKit has already been deprecated by Apple, and the direction going forward is to use RealityKit. If we add a new MToon renderer to the SceneKit path, we will need to keep future MToon fixes in sync between both RealityKit and SceneKit.
In fact, this PR spreads very similar MToon shading logic across multiple places.
Therefore, I would like this PR to make MToon support RealityKit-only, and keep the SceneKit path on the existing fallback rendering. What do you think?
There was a problem hiding this comment.
I agree with this direction. I removed the new SceneKit MToon renderer and restored the SceneKit implementation and tests to the upstream main behavior. SceneKit now continues to use its existing fallback material conversion.
The new MToon renderer is RealityKit-only, and this scope is documented in the README.
| material.ambientOcclusion.texture = try whiteCustomTexture() | ||
| } | ||
| applyAlphaMode(mtoon.alphaMode, alphaCutoff: mtoon.alphaCutoff, to: &material) | ||
| material.blending = .opaque |
There was a problem hiding this comment.
In customMToonOutlineMaterial, applyAlphaMode(...) is called, but immediately after that, its result is overwritten with material.blending = .opaque.
applyAlphaMode(mtoon.alphaMode, alphaCutoff: mtoon.alphaCutoff, to: &material)
material.blending = .opaqueBecause of this, even outlines for BLEND materials are always rendered as opaque. This may break the appearance of models that have outlines on transparent materials.
If this is an intentional decision to avoid transparency sorting issues, please leave a comment explaining the reason. Otherwise, I think the result of applyAlphaMode(...) should be respected.
There was a problem hiding this comment.
Fixed. I removed the unconditional material.blending = .opaque override. Outline materials now retain the result of applyAlphaMode(...).
The outline material also binds the base color texture, and the outline shader calculates opacity from the base texture alpha multiplied by the base color factor alpha. A regression test verifies that outlines for BLEND materials remain transparent.
|
Thank you for the detailed follow-up review. I addressed all seven points in the follow-up changes. |
There was a problem hiding this comment.
Thank you for addressing the previous feedback! I still have a few concerns, so could you please take a look at them?
Also, if possible, could you add a toggle to the sample app to enable or disable MToon? This would make it much easier to compare the visual differences across various models, and I think it would be very useful for both of us going forward.
| modelEntitiesByMaterialIndex[materialIndex, default: []].append(modelEntity) | ||
| } | ||
|
|
||
| /// Advances spring bones, node constraints, skinning, and MToon runtime state. |
There was a problem hiding this comment.
This changes the meaning of update(at:) from the elapsed time since the previous frame to an absolute elapsed time. That is source-compatible but behaviorally breaking: existing callers passing SceneEvents.Update.deltaTime (for example, 0.016, 0.017, 0.016) will produce differences of approximately 0, 0.001, and 0. Spring Bone simulation and MToon UV animation may therefore stop or behave unpredictably. Please keep the primary API delta-based, e.g. update(deltaTime:), and let the caller supply the frame delta instead of retaining lastUpdateTime and managing a second clock.
There was a problem hiding this comment.
Thank you for catching this behavioral compatibility issue.
I added update(deltaTime:) as the primary API and updated the RealityKit examples and integration call sites to pass SceneEvents.Update.deltaTime; update(at:) remains as a source-compatible delta-based wrapper without retaining an absolute-time clock.
A regression test verifies that both entry points advance the MToon runtime using the supplied frame deltas.
|
|
||
| updateMToonRuntime(deltaTime: Float(deltaTime)) | ||
| nodeConstraints.forEach { $0.apply() } | ||
| updateSkinning() |
There was a problem hiding this comment.
The expression path still applies each Material Color Bind and Texture Transform Bind directly from its base value. As a result, expressions affecting the same material overwrite one another, and setting one expression back to zero removes effects from other active expressions. Please retain the normalized weight for each ExpressionKey and recompute final material values from all active expressions whenever any weight changes. Please also add tests covering simultaneous color binds, clearing one expression while another remains active, and accumulated texture-transform binds.
There was a problem hiding this comment.
VRMEntity now retains the normalized weight for each canonical ExpressionKey and recomputes morph, material color, and texture-transform results from their base values and all active expressions whenever a weight changes.
Tests cover simultaneous material color binds, clearing one expression while another remains active, and accumulated texture-transform binds.
| let textureParam = try customTexture(withTextureIndex: baseTexture.index, semantic: .color) | ||
| material.baseColor = .init(tint: .white, texture: textureParam) | ||
| } else { | ||
| material.baseColor = .init(tint: .white, texture: try whiteCustomTexture()) |
There was a problem hiding this comment.
shadingShiftTexture is a data texture whose R channel is consumed numerically, so loading it with the .color semantic can apply color-space conversion and change the shader input. Please load it as a linear/raw data texture. The same policy should be applied consistently to outlineWidthMultiplyTexture (G channel) and uvAnimationMaskTexture (B channel), including their regular and outline-material paths.
There was a problem hiding this comment.
MToon texture semantics are now defined centrally per slot: shading shift, outline width, and UV animation mask textures use .raw in both regular and outline paths, normal textures use .normal, and color textures use .color.
Regression tests verify the three raw-data slots, their material bindings, and that raw and color texture requests use separate cached resources.
| self.emissiveTexture = material.emissiveTexture.map(MToonMaterialDescriptor.Texture.init) | ||
| self.shadeMultiplyTexture = mtoon.shadeMultiplyTexture.map(MToonMaterialDescriptor.Texture.init) | ||
| self.shadingShiftTexture = mtoon.shadingShiftTexture.map(MToonMaterialDescriptor.Texture.init) | ||
| self.normalTexture = material.normalTexture.map(MToonMaterialDescriptor.Texture.init) |
There was a problem hiding this comment.
Only the normal-texture index is preserved here, so glTF normalTexture.scale is lost; on the VRM 0.x path, _BumpMap is loaded but _BumpScale is also discarded. Please add normalScale to MToonMaterialDescriptor, populate it from material.normalTexture?.scale ?? 1 for VRM 1.0 and _BumpScale ?? 1 for VRM 0.x, then store it in MToonMaterialParameters and pass it to realitykit::unpack_normal instead of always using 1.0h.
There was a problem hiding this comment.
I added normalScale to MToonMaterialDescriptor, populated it from normalTexture.scale for VRM 1.0 and _BumpScale for VRM 0.x, stored it in MToonMaterialParameters, and passed it to realitykit::unpack_normal.
Tests verify both VRM 0.x and VRM 1.0 descriptor values and the value passed through the shader parameter texture.
| let outlineColor = vectors.simd4("_OutlineColor") ?? SIMD4<Float>(0, 0, 0, 1) | ||
| let alphaMode = GLTF.Material.AlphaMode(vrm0: property, fallback: material.alphaMode) | ||
| let transparentWithZWrite = property.keywordMap["_ZWRITE_ON"] ?? false | ||
| let doubleSided = material.doubleSided || floats.float("_CullMode") == 0 |
There was a problem hiding this comment.
Reducing VRM 0.x _CullMode to doubleSided: Bool loses the distinction between front culling (1) and back culling (2); both currently become single-sided and are later mapped to RealityKit .back. Please represent culling explicitly (for example, .none, .front, and .back), map _CullMode directly to those cases, convert VRM 1.0 doubleSided to .none or .back, and have VRMEntityLoader pass the selected culling direction through unchanged.
There was a problem hiding this comment.
I replaced it with an explicit .none, .front, and .back culling mode; VRM 0.x _CullMode values 0, 1, and 2 map directly to those cases, while VRM 1.0 doubleSided maps to .none or .back.
VRMEntityLoader now preserves the selected direction unchanged, outline materials continue to use front-face culling, and tests cover all VRM 0.x modes, invalid-value fallback, and VRM 1.0 double-sided behavior.
|
Thank you for the detailed follow-up review. |
2 participants
Summary
Related to #18.
This PR adds MToon rendering support for VRM 0.x and VRM 1.0 models across SceneKit and RealityKit, including shader/material handling, expression-driven material updates, texture transforms, outline rendering, and Example updates for both iOS and macOS.
Changes
CustomMaterialsupport with parameter textures, UV animation inputs, light direction sync, and texture/color bind updates.VRMNodeandVRMEntity.Notes
RealityKit support improves the MToon appearance substantially, but it is still not a perfect visual match with the official MToon renderer. Some shading and material behavior is approximated within RealityKit
CustomMaterialconstraints.After this, I plan to work on VRMA's support as well.