Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: deserialized scene should have node transforms invalidated at initial #16353

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Bug: deserialized scene should have node transforms invalidated at initial #16353

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48ca27a
Bug: deserialized scene should have node transforms invalidated
shrinktofit Oct 7, 2023
18815bf
Merge remote-tracking branch 'upstream/v3.8.2' into FixSceneDirty
shrinktofit Oct 7, 2023
abb7ea2
Remove transformFlags modification in _onBatchCreated
shrinktofit Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cocos/scene-graph/node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,7 @@ export class Node extends CCObject implements ISchedulable, CustomSerializable {
@serializable
protected _euler = new Vec3();

protected _transformFlags = TransformBit.NONE; // does the world transform need to update?
protected _transformFlags = TransformBit.TRS; // does the world transform need to update?
protected _eulerDirty = false;

protected _flagChangeVersion = 0;
Expand Down
2 changes: 1 addition & 1 deletion native/cocos/core/scene-graph/Node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ class Node : public CCObject {
// NOTE: TypeArray created in node.jsb.ts _ctor should have the same memory layout
uint32_t _eventMask{0}; // Uint32: 0
uint32_t _layer{static_cast<uint32_t>(Layers::LayerList::DEFAULT)}; // Uint32: 1
uint32_t _transformFlags{0}; // Uint32: 2
uint32_t _transformFlags{static_cast<uint32_t>(TransformBit::TRS)}; // Uint32: 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this._transformFlags |= TransformBit.TRS; was set in _onBatchCreated.

If we modify the initialized value to TRS, will it mark it as dirty twice and call updateWorldTransform twice?

For instance,

const scene = deserialize(archive) as Scene;
const node = scene.getChildByPath('Node')!;
const worldScale = node.worldScale;  // call updateWorldTransform internally, and reset child._transformFlags = TransformBit.NONE;

// Use worldScale 

director.runScene(scene); // call scene._load -> scene._onBatchCreated -> node._onBatchCreated -> this._transformFlags |= TransformBit.TRS; 
//  this._transformFlags is set to TransformBit.TRS again, so updateWorldTransform will be invoked twice, but in fact, the world transform was not changed at all.

Copy link
Contributor Author

@shrinktofit shrinktofit Oct 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend deleting transform flags modification in Node._onBatchCreated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

abb7ea2

index_t _siblingIndex{0}; // Int32: 0
uint8_t _activeInHierarchy{0}; // Uint8: 0
uint8_t _active{1}; // Uint8: 1
Expand Down
26 changes: 26 additions & 0 deletions tests/scene-graph/scene.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { deserialize } from "../../cocos/serialization";
import '../utils/matchers/value-type-asymmetric-matchers';
import { v3 } from "../../cocos/core";
import { Scene } from "../../cocos/scene-graph";

test(`Bugfix cocos/cocos-engine#16352: deserialized scene should have node transforms invalidated`, () => {
const archive = [{
__type__: 'cc.Scene',
_children: [{
__type__: 'cc.Node',
_name: 'Node',
_lscale: {
__type__: 'cc.Vec3',
x: 2,
y: 1,
z: 1,
},
}],
}];

const scene = deserialize(archive) as Scene;

const node = scene.getChildByPath('Node')!;
expect(node).not.toBeNull();
expect(node.worldScale).toBeCloseToVec3(v3(2, 1, 1));
});