Skip to content

Commit 4a183b1

Browse files
authored
Fix exception in material uniform animation binding (#14764)
1 parent ea55e3e commit 4a183b1

5 files changed

Lines changed: 189 additions & 10 deletions

File tree

EngineErrorMap.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,22 @@ Previous error occurred when instantiating animation clip %s on node %s.
18451845

18461846
'%s' is not found from '%s'. It's specified as the root node to play animation clip '%s'.
18471847

1848+
### 3940
1849+
1850+
Error when animation attempted to bind material uniform target: target %s is not a material.
1851+
1852+
### 3941
1853+
1854+
Error when animation attempted to bind material uniform target: material %s has no recorded pass %s.
1855+
1856+
### 3942
1857+
1858+
Error when animation attempted to bind material uniform target: material %s at pass %s has no recorded uniform %s.
1859+
1860+
### 3943
1861+
1862+
Error when animation attempted to bind material uniform target: material %s at pass %s's uniform %s has no recorded channel %s.
1863+
18481864
### 4000
18491865

18501866
<!-- DEPRECATED -->

cocos/animation/tracks/track.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,9 @@ export class TrackBinding {
389389
return null;
390390
}
391391
const runtimeProxy = proxy.forTarget(resultTarget);
392+
if (!runtimeProxy) {
393+
return null;
394+
}
392395
const binding: RuntimeBinding = {
393396
setValue: (value) => {
394397
runtimeProxy.set(value);

cocos/animation/value-proxy-factories/uniform.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { deviceManager, Type } from '../../gfx';
3131
import { Pass } from '../../render-scene/core/pass';
3232
import { getDefaultFromType, getStringFromType } from '../../render-scene/core/pass-utils';
3333
import { IValueProxy, IValueProxyFactory } from '../value-proxy';
34-
import { warn } from '../../core';
34+
import { warn, warnID } from '../../core';
3535

3636
/**
3737
* @en
@@ -73,19 +73,38 @@ export class UniformProxyFactory implements IValueProxyFactory {
7373
this.uniformName = uniformName || '';
7474
}
7575

76-
public forTarget (target: Material): IValueProxy {
77-
const pass = target.passes[this.passIndex];
78-
const handle = pass.getHandle(this.uniformName);
76+
public forTarget (target: unknown): IValueProxy | undefined {
77+
if (!(target instanceof Material)) {
78+
warnID(3940, target);
79+
return undefined;
80+
}
81+
82+
const {
83+
passIndex,
84+
uniformName,
85+
channelIndex,
86+
} = this;
87+
88+
if (passIndex < 0 || passIndex >= target.passes.length) {
89+
warnID(3941, target.name, passIndex);
90+
return undefined;
91+
}
92+
93+
const pass = target.passes[passIndex];
94+
const handle = pass.getHandle(uniformName);
7995
if (!handle) {
80-
throw new Error(`Material "${target.name}" has no uniform "${this.uniformName}"`);
96+
warnID(3942, target.name, passIndex, uniformName);
97+
return undefined;
8198
}
99+
82100
const type = Pass.getTypeFromHandle(handle);
83101
if (type < Type.SAMPLER1D) {
84-
const realHandle = this.channelIndex === undefined ? handle : pass.getHandle(this.uniformName, this.channelIndex, Type.FLOAT);
102+
const realHandle = channelIndex === undefined ? handle : pass.getHandle(uniformName, channelIndex, Type.FLOAT);
85103
if (!realHandle) {
86-
throw new Error(`Uniform "${this.uniformName} (in material ${target.name}) has no channel ${this.channelIndex!}"`);
104+
warnID(3943, target.name, passIndex, uniformName, channelIndex);
105+
return undefined;
87106
}
88-
if (isUniformArray(pass, this.uniformName)) {
107+
if (isUniformArray(pass, uniformName)) {
89108
return {
90109
set: (value: any) => {
91110
pass.setUniformArray(realHandle, value);
@@ -99,7 +118,7 @@ export class UniformProxyFactory implements IValueProxyFactory {
99118
};
100119
} else {
101120
const binding = Pass.getBindingFromHandle(handle);
102-
const prop = pass.properties[this.uniformName];
121+
const prop = pass.properties[uniformName];
103122
const texName = prop && prop.value ? `${prop.value as string}${getStringFromType(prop.type)}` : getDefaultFromType(prop.type) as string;
104123
let dftTex = builtinResMgr.get<TextureBase>(texName);
105124
if (!dftTex) {

cocos/animation/value-proxy.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ export interface IValueProxyFactory {
5656
* @zh
5757
* 返回指定目标的曲线值代理。
5858
* @param target The target acquiring the value proxy.
59+
* @returns The value proxy, or undefined if the proxy could not be created.
60+
* In later case, a warn should be given before returning.
5961
*/
60-
forTarget (target: any): IValueProxy;
62+
forTarget (target: any): IValueProxy | undefined;
6163
}
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { Track, UniformProxyFactory, VectorTrack } from "../../../cocos/animation/animation";
2+
import { AnimationClip } from "../../../cocos/animation/animation-clip";
3+
import { AnimationState } from "../../../cocos/animation/animation-state";
4+
import { Pass } from "../../../cocos/render-scene";
5+
import { Component, Node } from "../../../cocos/scene-graph";
6+
import { director, EffectAsset, js, Material, Vec3 } from "../../../exports/base";
7+
import { captureWarnIDs } from "../../utils/log-capture";
8+
9+
describe(`Error sentences`, () => {
10+
beforeAll(() => js.setClassName('MaterialHost', MaterialHost));
11+
afterAll(() => js.unregisterClass(MaterialHost));
12+
13+
test(`Warn if input target is not a material`, () => {
14+
const origin = new Node('SomeOrigin');
15+
const materialHost = origin.addComponent(MaterialHost) as MaterialHost;
16+
17+
const track = new VectorTrack();
18+
track.channels()[0].curve.assignSorted([[0.0, 0.0]]);
19+
track.path.toComponent(MaterialHost);
20+
track.proxy = new UniformProxyFactory();
21+
22+
bindThenAssertWarn(origin, track, [3940, materialHost]);
23+
});
24+
25+
test(`Warn if recorded pass index is invalid`, () => {
26+
const origin = new Node('SomeOrigin');
27+
const materialHost = origin.addComponent(MaterialHost) as MaterialHost;
28+
const material = materialHost.material = mockMaterial([
29+
{ uniforms: {} },
30+
{ uniforms: {} },
31+
{ uniforms: {} },
32+
]);
33+
material.name = 'SomeMaterial';
34+
35+
const track = new VectorTrack();
36+
track.channels()[0].curve.assignSorted([[0.0, 0.0]]);
37+
track.path.toComponent(MaterialHost).toProperty('material');
38+
39+
const proxyFactory = track.proxy = new UniformProxyFactory();
40+
for (const passIndex of [-2, -1, 3, 4]) {
41+
proxyFactory.passIndex = passIndex;
42+
bindThenAssertWarn(origin, track, [3941, material.name, proxyFactory.passIndex]);
43+
}
44+
});
45+
46+
test(`Warn if recorded uniform name is in invalid`, () => {
47+
const origin = new Node('SomeOrigin');
48+
const materialHost = origin.addComponent(MaterialHost) as MaterialHost;
49+
const material = materialHost.material = mockMaterial([
50+
{ uniforms: { 'SomeUniform': { availableChannelCount: 3 } } },
51+
{ uniforms: {} },
52+
{ uniforms: {} },
53+
]);
54+
material.name = 'SomeMaterial';
55+
56+
const track = new VectorTrack();
57+
track.channels()[0].curve.assignSorted([[0.0, 0.0]]);
58+
track.path.toComponent(MaterialHost).toProperty('material');
59+
const proxyFactory = track.proxy = new UniformProxyFactory();
60+
proxyFactory.passIndex = 1;
61+
proxyFactory.uniformName = 'SomeUniform';
62+
proxyFactory.channelIndex = 4;
63+
64+
bindThenAssertWarn(origin, track, [3942, material.name, proxyFactory.passIndex, proxyFactory.uniformName]);
65+
});
66+
67+
test(`Warn if recorded channel index is invalid`, () => {
68+
const origin = new Node('SomeOrigin');
69+
const materialHost = origin.addComponent(MaterialHost) as MaterialHost;
70+
const material = materialHost.material = mockMaterial([
71+
{ uniforms: {} },
72+
{ uniforms: { 'SomeUniform': { availableChannelCount: 3 } } },
73+
{ uniforms: {} },
74+
]);
75+
material.name = 'SomeMaterial';
76+
77+
const track = new VectorTrack();
78+
track.channels()[0].curve.assignSorted([[0.0, 0.0]]);
79+
track.path.toComponent(MaterialHost).toProperty('material');
80+
const proxyFactory = track.proxy = new UniformProxyFactory();
81+
proxyFactory.passIndex = 1;
82+
proxyFactory.uniformName = 'SomeUniform';
83+
84+
for (const channelIndex of [-1, -2, 3, 4]) {
85+
proxyFactory.channelIndex = channelIndex;
86+
bindThenAssertWarn(origin, track, [3943, material.name, proxyFactory.passIndex, proxyFactory.uniformName, proxyFactory.channelIndex]);
87+
}
88+
});
89+
90+
class MaterialHost extends Component {
91+
public declare material: Material;
92+
}
93+
94+
function bindThenAssertWarn(
95+
root: Node,
96+
track: Track,
97+
expectedCapturedWarns: [warnId: number, ...optionalParams: any[]]
98+
) {
99+
const animationClip = new AnimationClip();
100+
animationClip.name = 'Meow';
101+
animationClip.duration = 1.0;
102+
animationClip.addTrack(track);
103+
104+
const warnWatcher = captureWarnIDs();
105+
106+
const state = new AnimationState(animationClip);
107+
state.initialize(root);
108+
109+
expect(warnWatcher.captured).toHaveLength(2);
110+
expect(warnWatcher.captured[0]).toStrictEqual(expectedCapturedWarns);
111+
expect(warnWatcher.captured[1]).toStrictEqual([3937, 'Meow', root.name]);
112+
warnWatcher.clear();
113+
warnWatcher.stop();
114+
}
115+
116+
function mockMaterial(passMocks: Array<{
117+
uniforms: Record<string, { availableChannelCount: number }>;
118+
}>) {
119+
const passes = passMocks.map((passMock) => {
120+
const pass = new Pass(director.root!);
121+
jest.spyOn(pass, 'getHandle').mockImplementation((name, offset) => {
122+
const uniformMock = passMock.uniforms[name];
123+
if (!uniformMock) {
124+
return 0;
125+
}
126+
if (typeof offset !== 'undefined' && (offset < 0 || offset >= uniformMock.availableChannelCount)) {
127+
return 0;
128+
}
129+
return 1;
130+
});
131+
return pass;
132+
});
133+
134+
const material = new Material();
135+
jest.spyOn(material, 'passes', 'get').mockReturnValue(passes);
136+
137+
return material;
138+
}
139+
});

0 commit comments

Comments
 (0)