Conversation
* vrm1.0 support * Update Example/VisionExample/VisionExampleApp.swift Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * displayName * displayName * Update Example/VisionExample/ContentView.swift Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * materialName * SpringBone * revert Sources/VRMKit/VRM/Node.swift * revert Sources/VRMKit/VRM/VRM1.swift * Revert "revert Sources/VRMKit/VRM/VRM1.swift" This reverts commit b66fe1c. * Revert "revert Sources/VRMKit/VRM/Node.swift" This reverts commit 445e5f6. * fix test * fix BlendShape * fix comment * remove comments * fix comment * mac example * add testcase * remove unnecessary comment * SwiftTesting * revert VRM1Tests.swift * Revert "revert VRM1Tests.swift" This reverts commit 6abaf87. * refactor testcase * allowedUserName * texture transform * fix build error * add example build pipeline --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Summary of ChangesHello @tattn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request merges the 'feature/vrm1.0' branch, adding support for VRM 1.0 models to the VRMKit library. It includes migration logic to convert VRM 1.0 models to the older VRM 0.x format, updates example projects to demonstrate VRM 1.0 compatibility, and improves material and accessor handling. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 introduces comprehensive support for VRM 1.0, a significant addition that includes new data models, a migration layer to VRM 0.x representation, and glTF sparse accessor support, along with updated examples and tests. However, a security audit identified two medium-severity Denial of Service (DoS) vulnerabilities related to insecure handling of external input within the VRM loading logic, which could cause application crashes when processing malicious or malformed models. Additionally, there is a suggestion to address code duplication for improved long-term maintainability.
| private func baseDataForSparse(accessor: GLTF.Accessor, | ||
| vectorSize: Int, | ||
| loader: VRMSceneLoader) throws -> Data { | ||
| if let bufferViewIndex = accessor.bufferView { | ||
| let bufferView = try loader.bufferView(withBufferViewIndex: bufferViewIndex) | ||
| let dataStride = bufferView.stride ?? vectorSize | ||
| return bufferView.bufferView.subdata(offset: accessor.byteOffset, | ||
| size: vectorSize, | ||
| stride: dataStride, | ||
| count: accessor.count) | ||
| } | ||
| return Data(count: vectorSize * accessor.count) | ||
| } | ||
|
|
||
| private func applySparse(sparse: GLTF.Accessor.Sparse, | ||
| accessorCount: Int, | ||
| vectorSize: Int, | ||
| loader: VRMSceneLoader, | ||
| data: inout Data) throws { | ||
| guard sparse.count > 0 else { return } | ||
| let indices = try sparseIndices(sparse: sparse, loader: loader) | ||
| let values = try sparseValues(sparse: sparse, vectorSize: vectorSize, loader: loader) | ||
| let count = min(indices.count, sparse.count) | ||
| data.withUnsafeMutableBytes { rawDst in | ||
| guard let dst = rawDst.bindMemory(to: UInt8.self).baseAddress else { return } | ||
| values.withUnsafeBytes { rawSrc in | ||
| guard let src = rawSrc.bindMemory(to: UInt8.self).baseAddress else { return } | ||
| for i in 0..<count { | ||
| let index = indices[i] | ||
| guard index >= 0, index < accessorCount else { continue } | ||
| let dstPos = index * vectorSize | ||
| let srcPos = i * vectorSize | ||
| memcpy(dst.advanced(by: dstPos), src.advanced(by: srcPos), vectorSize) | ||
| } | ||
| }() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| self.init(data: bufferView, | ||
| semantic: semantic, | ||
| vectorCount: accessor.count, | ||
| usesFloatComponents: accessor.componentType == .float, | ||
| componentsPerVector: componentsPerVector, | ||
| bytesPerComponent: bytesPerComponent, | ||
| dataOffset: accessor.byteOffset, | ||
| dataStride: dataStride) | ||
| private func sparseIndices(sparse: GLTF.Accessor.Sparse, loader: VRMSceneLoader) throws -> [Int] { | ||
| let bufferView = try loader.bufferView(withBufferViewIndex: sparse.indices.bufferView) | ||
| let bytesPerIndex = bytes(of: sparse.indices.componentType) | ||
| let stride = bufferView.stride ?? bytesPerIndex | ||
| let indexData = bufferView.bufferView.subdata(offset: sparse.indices.byteOffset, | ||
| size: bytesPerIndex, | ||
| stride: stride, | ||
| count: sparse.count) | ||
| var indices: [Int] = [] | ||
| indices.reserveCapacity(sparse.count) | ||
| indexData.withUnsafeBytes { raw in | ||
| guard let base = raw.baseAddress else { return } | ||
| for i in 0..<sparse.count { | ||
| let offset = i * bytesPerIndex | ||
| switch sparse.indices.componentType { | ||
| case .unsignedByte: | ||
| indices.append(Int(base.load(fromByteOffset: offset, as: UInt8.self))) | ||
| case .unsignedShort: | ||
| indices.append(Int(base.load(fromByteOffset: offset, as: UInt16.self))) | ||
| case .unsignedInt: | ||
| indices.append(Int(base.load(fromByteOffset: offset, as: UInt32.self))) | ||
| default: | ||
| break | ||
| } | ||
| } | ||
| } | ||
| return indices | ||
| } | ||
|
|
||
| private func sparseValues(sparse: GLTF.Accessor.Sparse, | ||
| vectorSize: Int, | ||
| loader: VRMSceneLoader) throws -> Data { | ||
| let bufferView = try loader.bufferView(withBufferViewIndex: sparse.values.bufferView) | ||
| let stride = bufferView.stride ?? vectorSize | ||
| return bufferView.bufferView.subdata(offset: sparse.values.byteOffset, | ||
| size: vectorSize, | ||
| stride: stride, | ||
| count: sparse.count) | ||
| } |
There was a problem hiding this comment.
There's significant code duplication for handling sparse accessors between this file and Sources/VRMRealityKit/VRMEntityLoader.swift. The functions baseDataForSparse, applySparse, sparseIndices, and sparseValues are nearly identical in both files. To improve maintainability, this common glTF parsing logic should be extracted to a shared location, for example, within the VRMKit module. This would avoid having to maintain two copies of the same code.
Co-authored-by: tatsuya-ogawa <tatsuya-ogawa@users.noreply.github.com>
- Updated VRM.MaterialProperty to VRM0.MaterialProperty for consistency. - Removed VRMFileProtocol as it is no longer needed. - Migrated VRM1 structures to VRM0 equivalents in VRMMigration.swift. - Adjusted VRMLoader to support loading VRM0 thumbnails. - Modified Humanoid and SecondaryAnimation classes to use VRM0 types. - Updated tests to validate VRM0 functionality and migration from VRM1. - Removed deprecated VRM1SceneLoader and related methods. - Added new tests for VRM0 to ensure proper functionality and migration.
|
@codex please review |
![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: 7611477770
ℹ️ 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".
2 participants
This pull request adds support for selecting and loading multiple VRM models in all example apps (iOS, macOS, visionOS, watchOS), introduces UI controls for switching models and blend shape expressions, and updates internal logic to handle these changes. The main improvements are the addition of a second VRM sample model, UI enhancements for model/expression selection, and refactoring of loading logic to support dynamic model switching.
Model selection and loading improvements:
VRM1_Constraint_Twist_Sample.vrm) to the example project resources and updated Xcode project configuration to include it in all targets. (Example.xcodeproj/project.pbxproj) [1] [2] [3] [4] [5] [6]RealityKitViewController.swift), macOS (MacExample/ContentView.swift), and visionOS (VisionExample/ContentView.swift) examples to support dynamic switching between available models via UI controls. [1] [2] [3] [4] [5] [6] [7] [8] [9]User interface enhancements:
Blend shape and pose handling:
Build and CI improvements:
build-examplesjob to the CI workflow to build all example apps across supported platforms, ensuring new models are included in builds. (.github/workflows/ci.yml)Project structure and type safety:
VRMExampleModel,MacExampleModel,AppModel.ModelName) and updated code to use these enums for safer and clearer model handling throughout the examples. [1] [2] [3]