Skip to content

Commit

Permalink
Fix asset destroy cache bug (#622)
Browse files Browse the repository at this point in the history
* fix: glTF or Entity  destroy cache bug
  • Loading branch information
GuoLei1990 authored Dec 30, 2021
1 parent 3cc38d5 commit 4230956
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 45 deletions.
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
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 @@ -140,15 +139,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

0 comments on commit 4230956

Please sign in to comment.