Skip to content

Commit

Permalink
fix: fix critical memory leak block in the scene were not removed fro…
Browse files Browse the repository at this point in the history
…m the memory
  • Loading branch information
zardoy committed May 24, 2024
1 parent 8fdcbdf commit f2137d9
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 29 deletions.
6 changes: 0 additions & 6 deletions prismarine-viewer/viewer/lib/dispose.js

This file was deleted.

30 changes: 15 additions & 15 deletions prismarine-viewer/viewer/lib/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import * as THREE from 'three'
import * as TWEEN from '@tweenjs/tween.js'
import * as Entity from './entity/EntityMesh'
import { dispose3 } from './dispose'
import nbt from 'prismarine-nbt'
import EventEmitter from 'events'
import { PlayerObject, PlayerAnimation } from 'skinview3d'
Expand All @@ -14,10 +13,11 @@ import { NameTagObject } from 'skinview3d/libs/nametag'
import { flat, fromFormattedString } from '@xmcl/text-component'
import mojangson from 'mojangson'
import externalTexturesJson from './entity/externalTextures.json'
import { disposeObject } from './threeJsUtils'

export const TWEEN_DURATION = 50 // todo should be 100

function getUsernameTexture(username, { fontFamily = 'sans-serif' }) {
function getUsernameTexture (username, { fontFamily = 'sans-serif' }) {
const canvas = document.createElement('canvas')
const ctx = canvas.getContext('2d')
if (!ctx) throw new Error('Could not get 2d context')
Expand Down Expand Up @@ -61,7 +61,7 @@ const addNametag = (entity, options, mesh) => {
// todo cleanup
const nametags = {}

function getEntityMesh(entity, scene, options, overrides) {
function getEntityMesh (entity, scene, options, overrides) {
if (entity.name) {
try {
// https://github.com/PrismarineJS/prismarine-viewer/pull/410
Expand Down Expand Up @@ -105,15 +105,15 @@ export class Entities extends EventEmitter {
this.getItemUv = undefined
}

clear() {
clear () {
for (const mesh of Object.values(this.entities)) {
this.scene.remove(mesh)
dispose3(mesh)
disposeObject(mesh)
}
this.entities = {}
}

setDebugMode(mode, /** @type {THREE.Object3D?} */entity = null) {
setDebugMode (mode, /** @type {THREE.Object3D?} */entity = null) {
this.debugMode = mode
for (const mesh of entity ? [entity] : Object.values(this.entities)) {
const boxHelper = mesh.children.find(c => c.name === 'debug')
Expand All @@ -125,14 +125,14 @@ export class Entities extends EventEmitter {
}
}

setVisible(visible, /** @type {THREE.Object3D?} */entity = null) {
setVisible (visible, /** @type {THREE.Object3D?} */entity = null) {
this.visible = visible
for (const mesh of entity ? [entity] : Object.values(this.entities)) {
mesh.visible = visible
}
}

render() {
render () {
const dt = this.clock.getDelta()
for (const entityId of Object.keys(this.entities)) {
const playerObject = this.getPlayerObject(entityId)
Expand All @@ -142,7 +142,7 @@ export class Entities extends EventEmitter {
}
}

getPlayerObject(entityId) {
getPlayerObject (entityId) {
/** @type {(PlayerObject & { animation?: PlayerAnimation }) | undefined} */
const playerObject = this.entities[entityId]?.playerObject
return playerObject
Expand All @@ -152,7 +152,7 @@ export class Entities extends EventEmitter {
defaultSteveTexture

// true means use default skin url
updatePlayerSkin(entityId, username, /** @type {string | true} */skinUrl, /** @type {string | true | undefined} */capeUrl = undefined) {
updatePlayerSkin (entityId, username, /** @type {string | true} */skinUrl, /** @type {string | true | undefined} */capeUrl = undefined) {
let playerObject = this.getPlayerObject(entityId)
if (!playerObject) return
// const username = this.entities[entityId].username
Expand Down Expand Up @@ -235,14 +235,14 @@ export class Entities extends EventEmitter {
playerObject.cape.map = null
}

function isCanvasBlank(canvas) {
function isCanvasBlank (canvas) {
return !canvas.getContext('2d')
.getImageData(0, 0, canvas.width, canvas.height).data
.some(channel => channel !== 0)
}
}

playAnimation(entityPlayerId, /** @type {'walking' | 'running' | 'oneSwing' | 'idle'} */animation) {
playAnimation (entityPlayerId, /** @type {'walking' | 'running' | 'oneSwing' | 'idle'} */animation) {
const playerObject = this.getPlayerObject(entityPlayerId)
if (!playerObject) return

Expand All @@ -262,14 +262,14 @@ export class Entities extends EventEmitter {

}

displaySimpleText(jsonLike) {
displaySimpleText (jsonLike) {
if (!jsonLike) return
const parsed = typeof jsonLike === 'string' ? mojangson.simplify(mojangson.parse(jsonLike)) : nbt.simplify(jsonLike)
const text = flat(parsed).map(x => x.text)
return text.join('')
}

update(/** @type {import('prismarine-entity').Entity & {delete?, pos}} */entity, overrides) {
update (/** @type {import('prismarine-entity').Entity & {delete?, pos}} */entity, overrides) {
let isPlayerModel = entity.name === 'player'
if (entity.name === 'zombie' || entity.name === 'zombie_villager' || entity.name === 'husk') {
isPlayerModel = true
Expand Down Expand Up @@ -456,7 +456,7 @@ export class Entities extends EventEmitter {
if (e.additionalCleanup) e.additionalCleanup()
this.emit('remove', entity)
this.scene.remove(e)
dispose3(e)
disposeObject(e)
// todo dispose textures as well ?
delete this.entities[entity.id]
}
Expand Down
7 changes: 3 additions & 4 deletions prismarine-viewer/viewer/lib/primitives.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const THREE = require('three')
const { MeshLine, MeshLineMaterial } = require('three.meshline')
const { dispose3 } = require('./dispose')

function getMesh (primitive, camera) {
if (primitive.type === 'line') {
Expand Down Expand Up @@ -48,7 +47,7 @@ function getMesh (primitive, camera) {
}

class Primitives {
constructor (scene, camera) {
constructor(scene, camera) {
this.scene = scene
this.camera = camera
this.primitives = {}
Expand All @@ -57,15 +56,15 @@ class Primitives {
clear () {
for (const mesh of Object.values(this.primitives)) {
this.scene.remove(mesh)
dispose3(mesh)
disposeObject(mesh)
}
this.primitives = {}
}

update (primitive) {
if (this.primitives[primitive.id]) {
this.scene.remove(this.primitives[primitive.id])
dispose3(this.primitives[primitive.id])
disposeObject(this.primitives[primitive.id])
delete this.primitives[primitive.id]
}

Expand Down
13 changes: 13 additions & 0 deletions prismarine-viewer/viewer/lib/threeJsUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as THREE from 'three';

export const disposeObject = (obj: THREE.Object3D) => {
if (!obj) return
// not cleaning texture there as it might be used by other objects, but would be good to also do that
if (obj instanceof THREE.Mesh) {
obj.geometry.dispose();
obj.material.dispose();
}
if (obj.children) {
obj.children.forEach(disposeObject);
}
}
7 changes: 3 additions & 4 deletions prismarine-viewer/viewer/lib/worldrendererThree.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import * as THREE from 'three'
import { Vec3 } from 'vec3'
import nbt from 'prismarine-nbt'
import { dispose3 } from './dispose'
import PrismarineChatLoader from 'prismarine-chat'
import { renderSign } from '../sign-renderer/'
import { chunkPos, sectionPos } from './simpleUtils'
import { WorldRendererCommon, WorldRendererConfig } from './worldrendererCommon'
import * as tweenJs from '@tweenjs/tween.js'
import { BloomPass, RenderPass, UnrealBloomPass, EffectComposer, WaterPass, GlitchPass } from 'three-stdlib'
import { disposeObject } from './threeJsUtils'

export class WorldRendererThree extends WorldRendererCommon {
outputFormat = 'threeJs' as const
Expand Down Expand Up @@ -62,7 +62,7 @@ export class WorldRendererThree extends WorldRendererCommon {
let object: THREE.Object3D = this.sectionObjects[data.key]
if (object) {
this.scene.remove(object)
dispose3(object)
disposeObject(object)
delete this.sectionObjects[data.key]
}

Expand Down Expand Up @@ -263,7 +263,7 @@ export class WorldRendererThree extends WorldRendererCommon {
const mesh = this.sectionObjects[key]
if (mesh) {
this.scene.remove(mesh)
dispose3(mesh)
disposeObject(mesh)
}
delete this.sectionObjects[key]
}
Expand Down Expand Up @@ -327,7 +327,6 @@ class StarField {
material.blending = THREE.AdditiveBlending
material.depthTest = false
material.transparent = true
// material.unifo

// Create points and add them to the scene
this.points = new THREE.Points(geometry, material)
Expand Down

0 comments on commit f2137d9

Please sign in to comment.