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.

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

fix entity and glTF destroy bug #629

Merged
merged 3 commits into from
Jan 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions packages/core/src/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FeatureManager } from "./FeatureManager";
import { InputManager } from "./input/InputManager";
import { RenderQueueType } from "./material/enums/RenderQueueType";
import { Material } from "./material/Material";
import { ModelMesh, PrimitiveMesh } from "./mesh";
import { PhysicsManager } from "./physics";
import { IHardwareRenderer } from "./renderingHardwareInterface/IHardwareRenderer";
import { ClassPool } from "./RenderPipeline/ClassPool";
Expand Down Expand Up @@ -179,7 +178,7 @@ export class Engine extends EventDispatcher {
* @param physics - native physics Engine
*/
constructor(canvas: Canvas, hardwareRenderer: IHardwareRenderer, physics?: IPhysics, settings?: EngineSettings) {
super(null);
super();
this._hardwareRenderer = hardwareRenderer;
this._hardwareRenderer.init(canvas);
if (physics) {
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/Entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ export class Entity extends EngineObject {
* Destroy self.
*/
destroy(): void {
if (this._destroyed) return;

super.destroy();
const abilityArray = this._components;
for (let i = abilityArray.length - 1; i >= 0; i--) {
abilityArray[i].destroy();
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/RenderPipeline/BasicRenderPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export class BasicRenderPipeline {
const program = _backgroundTextureMaterial.shader._getShaderProgram(engine, Shader._compileMacros);
program.bind();
program.uploadAll(program.materialUniformBlock, _backgroundTextureMaterial.shaderData);
program.uploadUngroupTextures();
program.uploadUnGroupTextures();

_backgroundTextureMaterial.renderState._apply(engine, false);
rhi.drawPrimitive(mesh, mesh.subMesh, program);
Expand Down Expand Up @@ -261,7 +261,7 @@ export class BasicRenderPipeline {
program.bind();
program.groupingOtherUniformBlock();
program.uploadAll(program.materialUniformBlock, shaderData);
program.uploadUngroupTextures();
program.uploadUnGroupTextures();

renderState._apply(engine, false);
rhi.drawPrimitive(mesh, mesh.subMesh, program);
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/RenderPipeline/RenderQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export class RenderQueue {
program.uploadAll(program.cameraUniformBlock, cameraData);
program.uploadAll(program.rendererUniformBlock, rendererData);
program.uploadAll(program.materialUniformBlock, materialData);
// Ungroup textures should upload default value, texture uint maybe change by logic of texture bind.
program.uploadUngroupTextures();
// UnGroup textures should upload default value, texture uint maybe change by logic of texture bind.
program.uploadUnGroupTextures();
program._uploadCamera = camera;
program._uploadRenderer = renderer;
program._uploadMaterial = material;
Expand All @@ -132,9 +132,9 @@ export class RenderQueue {
program.uploadTextures(program.materialUniformBlock, materialData);
}

// We only consider switchProgram case, because ungroup texture's value is always default.
// We only consider switchProgram case, because UnGroup texture's value is always default.
if (switchProgram) {
program.uploadUngroupTextures();
program.uploadUnGroupTextures();
}
}
material.renderState._apply(camera.engine, renderer.entity.transform._isFrontFaceInvert());
Expand Down
9 changes: 0 additions & 9 deletions packages/core/src/Scene.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export class Scene extends EngineObject {
/** @internal */
_globalShaderMacro: ShaderMacroCollection = new ShaderMacroCollection();

private _destroyed: boolean = false;
private _rootEntities: Entity[] = [];
private _ambientLight: AmbientLight;

Expand Down Expand Up @@ -72,13 +71,6 @@ export class Scene extends EngineObject {
return this._rootEntities;
}

/**
* Whether it's destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

/**
* Create scene.
* @param engine - Engine
Expand Down Expand Up @@ -218,7 +210,6 @@ export class Scene extends EngineObject {
this._activeCameras.length = 0;
(Scene.sceneFeatureManager as any)._objects = [];
this.shaderData._addRefCount(-1);
this._destroyed = true;
}

/**
Expand Down
14 changes: 2 additions & 12 deletions packages/core/src/asset/RefObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export abstract class RefObject extends EngineObject implements IRefObject {
isGCIgnored: boolean = false;

private _refCount: number = 0;
private _destroyed: boolean = false;

/**
* Counted by valid references.
Expand All @@ -19,13 +18,6 @@ export abstract class RefObject extends EngineObject implements IRefObject {
return this._refCount;
}

/**
* Whether it has been destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

protected constructor(engine: Engine) {
super(engine);
engine.resourceManager._addRefObject(this.instanceId, this);
Expand All @@ -43,7 +35,7 @@ export abstract class RefObject extends EngineObject implements IRefObject {
// resourceManager maybe null,because engine has destroyed.
// TODO:the right way to fix this is to ensure destroy all when call engine.destroy,thus don't need to add this project.
if (resourceManager) {
resourceManager._deleteAsset(this);
super.destroy();
resourceManager._deleteRefObject(this.instanceId);
}

Expand All @@ -53,7 +45,7 @@ export abstract class RefObject extends EngineObject implements IRefObject {
}
this._engine = null;
this._onDestroy();
this._destroyed = true;

return true;
}

Expand All @@ -66,15 +58,13 @@ export abstract class RefObject extends EngineObject implements IRefObject {

/**
* @internal
* Add reference resource.
*/
_addRefCount(value: number): void {
this._refCount += value;
}

/**
* @internal
* Remove reference resource.
*/
_addToResourceManager(path: string): void {
this._engine.resourceManager._addAsset(path, this);
Expand Down
17 changes: 8 additions & 9 deletions packages/core/src/asset/ResourceManager.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Engine, EngineObject } from "..";
import { ObjectValues } from "../base/Util";
import { AssetPromise } from "./AssetPromise";
import { Loader } from "./Loader";
import { LoadItem } from "./LoadItem";
import { RefObject } from "./RefObject";
import { Engine } from "..";
import { Loader } from "./Loader";
import { AssetType } from "./AssetType";
import { ObjectValues } from "../base/Util";

/**
* ResourceManager
Expand All @@ -17,10 +16,10 @@ export class ResourceManager {
/**
* @internal
*/
static _addLoader(type: string, loader: Loader<any>, extnames: string[]) {
static _addLoader(type: string, loader: Loader<any>, extNames: string[]) {
this._loaders[type] = loader;
for (let i = 0, len = extnames.length; i < len; i++) {
this._extTypeMapping[extnames[i]] = type;
for (let i = 0, len = extNames.length; i < len; i++) {
this._extTypeMapping[extNames[i]] = type;
}
}

Expand Down Expand Up @@ -145,15 +144,15 @@ export class ResourceManager {
/**
* @internal
*/
_addAsset(path: string, asset: RefObject): void {
_addAsset(path: string, asset: EngineObject): void {
this._assetPool[asset.instanceId] = path;
this._assetUrlPool[path] = asset;
}

/**
* @internal
*/
_deleteAsset(asset: RefObject): void {
_deleteAsset(asset: EngineObject): void {
const id = asset.instanceId;
const path = this._assetPool[id];
if (path) {
Expand Down
19 changes: 18 additions & 1 deletion packages/core/src/base/EngineObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ export abstract class EngineObject {
@ignoreClone
readonly instanceId: number = ++EngineObject._instanceIdCounter;

/** Engine to which the object belongs. */
@ignoreClone
protected _engine: Engine;
protected _destroyed: boolean = false;

/**
* Get the engine which the object belongs.
Expand All @@ -22,7 +22,24 @@ export abstract class EngineObject {
return this._engine;
}

/**
* Whether it has been destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

constructor(engine: Engine) {
this._engine = engine;
}

/**
* Destroy self.
*/
destroy(): void {
if (this._destroyed) return;

this._engine.resourceManager?._deleteAsset(this);
this._destroyed = true;
}
}
5 changes: 2 additions & 3 deletions packages/core/src/base/EventDispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { EngineObject } from "./EngineObject";
import { Event } from "./Event";
import { ignoreClone } from "../clone/CloneManager";
import { Event } from "./Event";

/**
* EventDispatcher, which can be inherited as a base class.
*/
export class EventDispatcher extends EngineObject {
export class EventDispatcher {
@ignoreClone
private _evts = Object.create(null);
private _evtCount = 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shader/ShaderProgram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class ShaderProgram {
/**
* Upload ungroup texture shader data in shader uniform block.
*/
uploadUngroupTextures(): void {
uploadUnGroupTextures(): void {
const textureUniforms = this.otherUniformBlock.textureUniforms;
// textureUniforms property maybe null if ShaderUniformBlock not contain any texture.
if (textureUniforms) {
Expand Down
6 changes: 4 additions & 2 deletions packages/loader/src/scene-loader/Oasis.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EventDispatcher, ObjectValues } from "@oasis-engine/core";
import { Engine, EventDispatcher, ObjectValues } from "@oasis-engine/core";
import { AbilityManager } from "./AbilityManager";
import { NodeManager } from "./NodeManager";
import { SceneManager } from "./SceneManager";
Expand All @@ -15,9 +15,11 @@ export class Oasis extends EventDispatcher {
private schema: Schema;
public timeout: number;
private oasis = this;
public engine: Engine;

private constructor(private _options: Options, public readonly pluginManager: PluginManager) {
super(_options.engine);
super();
this.engine = _options.engine;
this.schema = _options.config;
this.timeout = _options.timeout;
_options.scripts = _options.scripts ?? {};
Expand Down