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

Entity support add other Transform #2456

Merged
merged 16 commits into from
Dec 11, 2024
Merged

Entity support add other Transform #2456

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e95b86d
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
d8df9e0
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
0d57088
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
0d004aa
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
b882eab
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
01e0823
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
375e4b7
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
6e635e7
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
406391f
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
4991156
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
08a1b5c
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
a0339f2
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
e83000d
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
90589b0
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
cb29b43
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
68fa63a
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
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
19 changes: 7 additions & 12 deletions packages/core/src/Camera.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ export class Camera extends Component {
@ignoreClone
private _frustumChangeFlag: BoolUpdateFlag;
@ignoreClone
private _transform: Transform;
@ignoreClone
private _isViewMatrixDirty: BoolUpdateFlag;
@ignoreClone
private _isInvViewProjDirty: BoolUpdateFlag;
Expand Down Expand Up @@ -315,7 +313,7 @@ export class Camera extends Component {
this._isViewMatrixDirty.flag = false;

// Ignore scale
const transform = this._transform;
const transform = this._entity.transform;
Matrix.rotationTranslation(transform.worldRotationQuaternion, transform.worldPosition, viewMatrix);
viewMatrix.invert();
return viewMatrix;
Expand Down Expand Up @@ -423,11 +421,9 @@ export class Camera extends Component {
constructor(entity: Entity) {
super(entity);

const transform = this.entity.transform;
this._transform = transform;
this._isViewMatrixDirty = transform.registerWorldChangeFlag();
this._isInvViewProjDirty = transform.registerWorldChangeFlag();
this._frustumChangeFlag = transform.registerWorldChangeFlag();
this._isViewMatrixDirty = entity.registerWorldChangeFlag();
this._isInvViewProjDirty = entity.registerWorldChangeFlag();
this._frustumChangeFlag = entity.registerWorldChangeFlag();
this._renderPipeline = new BasicRenderPipeline(this);
this._addResourceReferCount(this.shaderData, 1);
this._updatePixelViewport();
Expand Down Expand Up @@ -607,7 +603,7 @@ export class Camera extends Component {
const context = engine._renderContext;
const virtualCamera = this._virtualCamera;

const transform = this.entity.transform;
const transform = this._entity.transform;
Matrix.multiply(this.projectionMatrix, this.viewMatrix, virtualCamera.viewProjectionMatrix);
virtualCamera.position.copyFrom(transform.worldPosition);
if (virtualCamera.isOrthographic) {
Expand Down Expand Up @@ -738,7 +734,6 @@ export class Camera extends Component {
this._virtualCamera = null;
this._shaderData = null;
this._frustumChangeFlag = null;
this._transform = null;
this._isViewMatrixDirty = null;
this._isInvViewProjDirty = null;
this._viewport = null;
Expand Down Expand Up @@ -788,7 +783,7 @@ export class Camera extends Component {
private _updateShaderData(): void {
const shaderData = this.shaderData;

const transform = this._transform;
const transform = this._entity.transform;
shaderData.setMatrix(Camera._inverseViewMatrixProperty, transform.worldMatrix);
shaderData.setVector3(Camera._cameraPositionProperty, transform.worldPosition);
shaderData.setVector3(Camera._cameraForwardProperty, transform.worldForward);
Expand All @@ -806,7 +801,7 @@ export class Camera extends Component {
private _getInvViewProjMat(): Matrix {
if (this._isInvViewProjDirty.flag) {
this._isInvViewProjDirty.flag = false;
Matrix.multiply(this._transform.worldMatrix, this._getInverseProjectionMatrix(), this._invViewProjMat);
Matrix.multiply(this._entity.transform.worldMatrix, this._getInverseProjectionMatrix(), this._invViewProjMat);
}
return this._invViewProjMat;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/ComponentsDependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
if (invDependencies) {
for (let i = 0, len = invDependencies.length; i < len; i++) {
if (entity.getComponent(invDependencies[i])) {
throw `Should remove ${invDependencies[i].name} before adding ${type.name}`;
throw `Should remove ${invDependencies[i].name} before remove ${type.name}`;

Check warning on line 47 in packages/core/src/ComponentsDependencies.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/ComponentsDependencies.ts#L47 

Added line #L47 was not covered by tests
}
}
}
Expand Down
75 changes: 60 additions & 15 deletions packages/core/src/Entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { Engine } from "./Engine";
import { Layer } from "./Layer";
import { Scene } from "./Scene";
import { Script } from "./Script";
import { Transform } from "./Transform";
import { Transform, TransformTransitionState } from "./Transform";
import { UpdateFlagManager } from "./UpdateFlagManager";
import { ReferResource } from "./asset/ReferResource";
import { EngineObject } from "./base";
import { ComponentCloner } from "./clone/ComponentCloner";
Expand Down Expand Up @@ -73,8 +74,6 @@ export class Entity extends EngineObject {
name: string;
/** The layer the entity belongs to. */
layer: Layer = Layer.Layer0;
/** Transform component. */
readonly transform: Transform;

/** @internal */
_isActiveInHierarchy: boolean = false;
Expand All @@ -94,14 +93,39 @@ export class Entity extends EngineObject {
_isActive: boolean = true;
/** @internal */
_siblingIndex: number = -1;

/** @internal */
_isTemplate: boolean = false;
/** @internal */
_updateFlagManager: UpdateFlagManager = new UpdateFlagManager();

private _transform: Transform;
private _transformTransitionState: TransformTransitionState;
private _templateResource: ReferResource;
private _parent: Entity = null;
private _activeChangedComponents: Component[];

/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}

set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
pre._generateTransitionState(state);
}
if (value) {
state && value._applyTransitionState(state);
}
this._transform = value;
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to transform setter

The transform state transition logic needs validation and error handling.

Apply this diff to improve robustness:

set transform(value: Transform) {
  const pre = this._transform;
  if (value !== pre) {
+   if (value && !(value instanceof Transform)) {
+     throw new Error("Value must be an instance of Transform");
+   }
    let state = this._transformTransitionState;
    if (pre) {
      state ||= this._transformTransitionState = {};
+     try {
        pre._generateTransitionState(state);
+     } catch (e) {
+       console.error("Failed to generate transition state:", e);
+     }
    }
    if (value) {
+     try {
        state && value._applyTransitionState(state);
+     } catch (e) {
+       console.error("Failed to apply transition state:", e);
+     }
    }
    this._transform = value;
  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}
set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
pre._generateTransitionState(state);
}
if (value) {
state && value._applyTransitionState(state);
}
this._transform = value;
}
}
/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}
set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
if (value && !(value instanceof Transform)) {
throw new Error("Value must be an instance of Transform");
}
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
try {
pre._generateTransitionState(state);
} catch (e) {
console.error("Failed to generate transition state:", e);
}
}
if (value) {
try {
state && value._applyTransitionState(state);
} catch (e) {
console.error("Failed to apply transition state:", e);
}
}
this._transform = value;
}
}


/**
* Whether to activate locally.
*/
Expand Down Expand Up @@ -193,11 +217,17 @@ export class Entity extends EngineObject {
* Create a entity.
* @param engine - The engine the entity belongs to
*/
constructor(engine: Engine, name?: string) {
constructor(engine: Engine, name?: string, ...components: ComponentConstructor[]) {
super(engine);
this.name = name;
this.transform = this.addComponent(Transform);
this._inverseWorldMatFlag = this.transform.registerWorldChangeFlag();
for (let i = 0, n = components.length; i < n; i++) {
const type = components[i];
if (!(type.prototype instanceof Transform) || !this.transform) {
this.addComponent(type);
}
}
singlecoder marked this conversation as resolved.
Show resolved Hide resolved
this.transform || this.addComponent(Transform);
this._inverseWorldMatFlag = this.registerWorldChangeFlag();
}

/**
Expand All @@ -210,6 +240,7 @@ export class Entity extends EngineObject {
type: T,
...args: ComponentArguments<T>
): InstanceType<T> {
type.prototype instanceof Transform && this.transform?.destroy();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid destroying existing Transform instances when adding components

In the addComponent method, the line type.prototype instanceof Transform && this.transform?.destroy(); may inadvertently destroy a valid Transform. This could lead to unintended side effects.

Consider removing or modifying this line to prevent destroying existing Transform components.

Apply this diff:

-type.prototype instanceof Transform && this.transform?.destroy();
+if (type.prototype instanceof Transform && !this.transform) {
+  // If the entity does not already have a Transform, proceed to add one.
+}

Committable suggestion skipped: line range outside the PR's diff.

ComponentsDependencies._addCheck(this, type);
const component = new type(this, ...args) as InstanceType<T>;
this._components.push(component);
Expand Down Expand Up @@ -394,7 +425,10 @@ export class Entity extends EngineObject {
* @returns The child entity
*/
createChild(name?: string): Entity {
const child = new Entity(this.engine, name);
const transform = this._transform;
const child = transform
? new Entity(this.engine, name, transform.constructor as ComponentConstructor)
: new Entity(this.engine, name);
child.layer = this.layer;
child.parent = this;
return child;
Expand Down Expand Up @@ -429,6 +463,14 @@ export class Entity extends EngineObject {
return cloneEntity;
}

/**
* Listen for changes in the world pose of this `Entity`.
* @returns Change flag
*/
registerWorldChangeFlag(): BoolUpdateFlag {
return this._updateFlagManager.createFlag(BoolUpdateFlag);
}

/**
* @internal
*/
Expand All @@ -438,8 +480,10 @@ export class Entity extends EngineObject {
}

private _createCloneEntity(): Entity {
const cloneEntity = new Entity(this._engine, this.name);

const transform = this._transform;
const cloneEntity = transform
? new Entity(this.engine, this.name, transform.constructor as ComponentConstructor)
: new Entity(this.engine, this.name);
const templateResource = this._templateResource;
if (templateResource) {
cloneEntity._templateResource = templateResource;
Expand All @@ -450,10 +494,9 @@ export class Entity extends EngineObject {
cloneEntity._isActive = this._isActive;
const { transform: cloneTransform } = cloneEntity;
const { transform: srcTransform } = this;
cloneTransform.position = srcTransform.position;
cloneTransform.rotation = srcTransform.rotation;
cloneTransform.scale = srcTransform.scale;

const transitionState = (this._transformTransitionState ||= {});
srcTransform._generateTransitionState(transitionState);
cloneTransform._applyTransitionState(transitionState);
const srcChildren = this._children;
for (let i = 0, n = srcChildren.length; i < n; i++) {
cloneEntity.addChild(srcChildren[i]._createCloneEntity());
Expand All @@ -478,7 +521,7 @@ export class Entity extends EngineObject {
for (let i = 0, n = components.length; i < n; i++) {
const sourceComp = components[i];
if (!(sourceComp instanceof Transform)) {
const targetComp = target.addComponent(<new (entity: Entity) => Component>sourceComp.constructor);
const targetComp = target.addComponent(<ComponentConstructor>sourceComp.constructor);
ComponentCloner.cloneComponent(sourceComp, targetComp, srcRoot, targetRoot, deepInstanceMap);
}
}
Expand Down Expand Up @@ -765,3 +808,5 @@ type ComponentArguments<T extends new (entity: Entity, ...args: any[]) => Compon
) => Component
? P
: never;

type ComponentConstructor = new (entity: Entity) => Component;
2 changes: 1 addition & 1 deletion packages/core/src/RenderPipeline/RenderQueue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class RenderQueue {

renderState._applyStates(
engine,
renderer.entity.transform._isFrontFaceInvert(),
renderer._transformEntity.transform._isFrontFaceInvert(),
shaderPass._renderStateDataMap,
material.shaderData,
customStates
Expand Down
24 changes: 14 additions & 10 deletions packages/core/src/Renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export class Renderer extends Component implements IComponentCustomClone {
/** @internal */
@ignoreClone
_batchedTransformShaderData: boolean = false;
/** @internal */
@ignoreClone
_transformEntity: Entity;

@ignoreClone
protected _overrideUpdate: boolean = false;
Expand All @@ -68,8 +71,6 @@ export class Renderer extends Component implements IComponentCustomClone {
protected _dirtyUpdateFlag: number = 0;
@ignoreClone
protected _rendererLayer: Vector4 = new Vector4();
@ignoreClone
protected _transform: Transform;

@deepClone
private _shaderData: ShaderData = new ShaderData(ShaderDataGroup.Renderer);
Expand Down Expand Up @@ -172,7 +173,7 @@ export class Renderer extends Component implements IComponentCustomClone {
this._addResourceReferCount(this.shaderData, 1);

this._onTransformChanged = this._onTransformChanged.bind(this);
this._setTransform(entity.transform);
this._setTransformEntity(entity);

shaderData.enableMacro(Renderer._receiveShadowMacro);
shaderData.setVector4(Renderer._rendererLayerProperty, this._rendererLayer);
Expand Down Expand Up @@ -366,7 +367,7 @@ export class Renderer extends Component implements IComponentCustomClone {
protected override _onDestroy(): void {
super._onDestroy();

this._setTransform(null);
this._setTransformEntity(null);
this._addResourceReferCount(this.shaderData, -1);

const materials = this._materials;
Expand All @@ -392,7 +393,7 @@ export class Renderer extends Component implements IComponentCustomClone {
* @internal
*/
_updateTransformShaderData(context: RenderContext, onlyMVP: boolean, batched: boolean): void {
const worldMatrix = this._transform.worldMatrix;
const worldMatrix = this._transformEntity.transform.worldMatrix;
if (onlyMVP) {
this._updateProjectionRelatedShaderData(context, worldMatrix, batched);
} else {
Expand Down Expand Up @@ -442,7 +443,7 @@ export class Renderer extends Component implements IComponentCustomClone {
Matrix.invert(worldMatrix, normalMatrix);
normalMatrix.transpose();

shaderData.setMatrix(Renderer._localMatrixProperty, this._transform.localMatrix);
shaderData.setMatrix(Renderer._localMatrixProperty, this._transformEntity.transform.localMatrix);
shaderData.setMatrix(Renderer._worldMatrixProperty, worldMatrix);
shaderData.setMatrix(Renderer._mvMatrixProperty, mvMatrix);
shaderData.setMatrix(Renderer._mvInvMatrixProperty, mvInvMatrix);
Expand All @@ -465,10 +466,13 @@ export class Renderer extends Component implements IComponentCustomClone {
/**
* @internal
*/
protected _setTransform(transform: Transform): void {
this._transform?._updateFlagManager.removeListener(this._onTransformChanged);
transform?._updateFlagManager.addListener(this._onTransformChanged);
this._transform = transform;
protected _setTransformEntity(entity: Entity): void {
const preEntity = this._transformEntity;
if (entity !== preEntity) {
preEntity?._updateFlagManager.removeListener(this._onTransformChanged);
entity?._updateFlagManager.addListener(this._onTransformChanged);
this._transformEntity = entity;
}
}

/**
Expand Down
Loading
Loading