fix(legacy): the memory misalignment issue with customAttribute - #313
Open
troublemaker52025 wants to merge 3 commits into
Open
Conversation
Contributor
Author
|
@cocos-robot run test cases |
troublemaker52025
requested review from
jk20012001
and
a lite review from Copilot
August 28, 2026 02:21
There was a problem hiding this comment.
Pull request overview
This PR addresses dynamic-mesh customAttributes vertex data misalignment by allowing custom attribute values to be provided as non-float TypedArrays (matching the declared GPU format) and adding debug-time validation to catch mismatches early.
Changes:
- Broadened dynamic custom-attribute
valuesfromFloat32Arrayto a TypedArray union (TS) /TypedArray(native) to support integer formats without float round-trips. - Updated
Mesh.updateSubMesh()(TS + native) to copy/update vertex buffers from arbitrary TypedArrays and validate element-size vs GPU format size in debug builds. - Added documentation/comments explaining the rationale and expected usage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| native/cocos/primitive/PrimitiveDefine.h | Changes dynamic custom attribute storage to TypedArray and documents intended usage. |
| native/cocos/3d/assets/Mesh.cpp | Updates native dynamic submesh update path to accept TypedArray and adds debug validation/typed-array handling. |
| cocos/primitive/define.ts | Introduces DynamicAttributeValues union and updates IDynamicGeometry.customAttributes[].values type. |
| cocos/3d/assets/mesh.ts | Updates TS dynamic submesh update path to accept DynamicAttributeValues and adds debug validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1212
to
1215
| ccstd::vector<TypedArray> buffers; | ||
| if (!geometry.positions.empty()) { | ||
| buffers.push_back(&geometry.positions); | ||
| buffers.push_back(wrapAsTypedArray(geometry.positions)); | ||
| } |
Comment on lines
581
to
587
| return; | ||
| } | ||
|
|
||
| const buffers: Float32Array[] = []; | ||
| const buffers: DynamicAttributeValues[] = []; | ||
| if (dynamicGeometry.positions.length > 0) { | ||
| buffers.push(dynamicGeometry.positions); | ||
| } |
jk20012001
reviewed
Aug 28, 2026
jk20012001
left a comment
jk20012001
left a comment
Contributor
There was a problem hiding this comment.
原理上是对的，但是改动代码太底层了，不确定有没有其他地方按照这里错误的样子配合写的，需要非常全面的测试
Code Size Check Report
Interface Check Report! WARNING this pull request has changed these public interfaces:
@@ -51446,8 +51446,25 @@
*/
export function applyDefaultGeometryOptions<GeometryOptions = IGeometryOptions>(options?: __private.__types_globals__RecursivePartial<IGeometryOptions>): GeometryOptions;
/**
* @en
+ * The set of concrete TypedArray types accepted for a dynamic mesh's custom attribute values.
+ * Unlike the standard attributes (positions/normals/uvs/tangents/colors), which are always
+ * float-based, a custom attribute's physical GPU format (`attr.format`) can be any integer or
+ * float format (e.g. RGBA16UI for joint indices). Accepting any concrete TypedArray lets callers
+ * supply data whose element type already matches the target GPU format (fast copy path),
+ * avoiding an unnecessary and potentially lossy "integer -> float -> integer" round trip.
+ * Float32Array remains valid for FLOAT-typed formats (backward compatible).
+ * @zh
+ * 动态网格定制属性 values 字段接受的具体 TypedArray 类型集合。与始终为 float 的标准属性
+ * （positions/normals/uvs/tangents/colors）不同，定制属性的物理 GPU 格式（`attr.format`）可以是
+ * 任意整型或浮点格式（例如 RGBA16UI 用于骨骼关节索引）。接受任意具体 TypedArray 类型，使调用者可以
+ * 直接传入与目标 GPU 格式匹配的原生类型数组（快速拷贝路径），避免不必要甚至有损的
+ * “整数 -> 浮点 -> 整数”往返转换。Float32Array 对于 FLOAT 类型格式依然有效（向后兼容）。
+ */
+ export type DynamicAttributeValues = Float32Array | Float64Array | Int8Array | Int16Array | Int32Array | Uint8Array | Uint16Array | Uint32Array;
+ /**
+ * @en
* The definition of the parameter for building a primitive geometry.
* @zh
* 几何体参数选项。
*/
@@ -51620,9 +51637,9 @@
* 定制属性列表。
*/
customAttributes?: {
attr: gfx.Attribute;
- values: Float32Array;
+ values: DynamicAttributeValues;
}[];
/**
* @en
* Min position.
|
|
@troublemaker52025, Please check the result of
Task Details
|
|
@troublemaker52025, Please check the result of
Task Details |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@troublemaker52025, Please check the result of
Task Details
|
|
@troublemaker52025, Please check the result of
Task Details |
Contributor
Author
测试方法
|
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
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.
Re: #
Changelog
Float32Array[]内存来存储顶点属性。但是在customAttribute却定义成了RGBA16UI，强行以Float32格式填入(memcpy 不区分类型，按Float32强行写入)，GPU 以RGBA16UI去解读，导致数据错乱vertexCount时，既然已经把RGBA16UI，强行以Float32格式填入，那就应该用Float32的字节宽度计算顶点数量，反而还用预定义好的RGBA16UI来计算，导致顶点数量多了一倍，触发了断言Continuous Integration
This pull request:
Compatibility Check
This pull request: