Merge pull request #11846 from bghgary/ktx-fix

Fix memory leak and incorrect data copy in KTX2
BabylonJS · Jan 25, 2022 · d98ff0e · d98ff0e
2 parents 87352ce + 0557e97
commit d98ff0e
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 34 deletions.
diff --git a/dist/preview release/babylon.ktx2Decoder.js b/dist/preview release/babylon.ktx2Decoder.js
diff --git a/dist/preview release/babylon.ktx2Decoder.js.map b/dist/preview release/babylon.ktx2Decoder.js.map
diff --git a/dist/preview release/what's new.md b/dist/preview release/what's new.md
@@ -398,6 +398,7 @@
 - Fix camera collisions for right-handed scenes ([carolhmj](https://github.com/carolhmj))
 - Add a null check when setting `imageSrc` on HolographicSlate([carolhmj](https://github.com/carolhmj))
 - Fix issue with physics impostors'unique ID not set correctly if an impostor was disposed ([RaananW](https://github.com/RaananW))
+- Fix memory leak and incorrect data copy in KTX2 ([bghgary](https://github.com/bghgary))
 
 ## Breaking changes
 

diff --git a/ktx2Decoder/src/Transcoders/mscTranscoder.ts b/ktx2Decoder/src/Transcoders/mscTranscoder.ts
@@ -85,49 +85,62 @@ export class MSCTranscoder extends Transcoder {
         return this._getMSCBasisTranscoder().then(() => {
             const basisModule = this._mscBasisModule;
 
-            const transcoder = src === sourceTextureFormat.UASTC4x4 ? new basisModule.UastcImageTranscoder() : new basisModule.BasisLzEtc1sImageTranscoder();
-            const texFormat = src === sourceTextureFormat.UASTC4x4 ? basisModule.TextureFormat.UASTC4x4 : basisModule.TextureFormat.ETC1S;
+            let transcoder: any;
+            let imageInfo: any;
+            let result: any;
+            let textureData: any = null;
 
-            const imageInfo = new basisModule.ImageInfo(texFormat, width, height, level);
+            try {
+                transcoder = src === sourceTextureFormat.UASTC4x4 ? new basisModule.UastcImageTranscoder() : new basisModule.BasisLzEtc1sImageTranscoder();
+                const texFormat = src === sourceTextureFormat.UASTC4x4 ? basisModule.TextureFormat.UASTC4x4 : basisModule.TextureFormat.ETC1S;
 
-            const targetFormat = basisModule.TranscodeTarget[transcodeTarget[dst]]; // works because the labels of the sourceTextureFormat enum are the same than the property names used in TranscodeTarget!
+                imageInfo = new basisModule.ImageInfo(texFormat, width, height, level);
 
-            if (!basisModule.isFormatSupported(targetFormat, texFormat)) {
-                throw new Error(`MSCTranscoder: Transcoding from "${sourceTextureFormat[src]}" to "${transcodeTarget[dst]}" not supported by current transcoder build.`);
-            }
+                const targetFormat = basisModule.TranscodeTarget[transcodeTarget[dst]]; // works because the labels of the sourceTextureFormat enum are the same as the property names used in TranscodeTarget!
 
-            let result: any;
+                if (!basisModule.isFormatSupported(targetFormat, texFormat)) {
+                    throw new Error(`MSCTranscoder: Transcoding from "${sourceTextureFormat[src]}" to "${transcodeTarget[dst]}" not supported by current transcoder build.`);
+                }
 
-            if (src === sourceTextureFormat.ETC1S) {
-                const sgd = ktx2Reader.supercompressionGlobalData;
+                if (src === sourceTextureFormat.ETC1S) {
+                    const sgd = ktx2Reader.supercompressionGlobalData;
 
-                transcoder.decodePalettes(sgd.endpointCount, sgd.endpointsData, sgd.selectorCount, sgd.selectorsData);
-                transcoder.decodeTables(sgd.tablesData);
+                    transcoder.decodePalettes(sgd.endpointCount, sgd.endpointsData, sgd.selectorCount, sgd.selectorsData);
+                    transcoder.decodeTables(sgd.tablesData);
 
-                imageInfo.flags = imageDesc!.imageFlags;
-                imageInfo.rgbByteOffset = 0;
-                imageInfo.rgbByteLength = imageDesc!.rgbSliceByteLength;
-                imageInfo.alphaByteOffset = imageDesc!.alphaSliceByteOffset > 0 ? imageDesc!.rgbSliceByteLength : 0;
-                imageInfo.alphaByteLength = imageDesc!.alphaSliceByteLength;
+                    imageInfo.flags = imageDesc!.imageFlags;
+                    imageInfo.rgbByteOffset = 0;
+                    imageInfo.rgbByteLength = imageDesc!.rgbSliceByteLength;
+                    imageInfo.alphaByteOffset = imageDesc!.alphaSliceByteOffset > 0 ? imageDesc!.rgbSliceByteLength : 0;
+                    imageInfo.alphaByteLength = imageDesc!.alphaSliceByteLength;
 
-                result = transcoder.transcodeImage(targetFormat, encodedData, imageInfo, 0, isVideo);
-            } else {
-                imageInfo.flags = 0;
-                imageInfo.rgbByteOffset = 0;
-                imageInfo.rgbByteLength = uncompressedByteLength;
-                imageInfo.alphaByteOffset = 0;
-                imageInfo.alphaByteLength = 0;
+                    result = transcoder.transcodeImage(targetFormat, encodedData, imageInfo, 0, isVideo);
+                } else {
+                    imageInfo.flags = 0;
+                    imageInfo.rgbByteOffset = 0;
+                    imageInfo.rgbByteLength = uncompressedByteLength;
+                    imageInfo.alphaByteOffset = 0;
+                    imageInfo.alphaByteLength = 0;
 
-                result = transcoder.transcodeImage(targetFormat, encodedData, imageInfo, 0, ktx2Reader.hasAlpha, isVideo);
+                    result = transcoder.transcodeImage(targetFormat, encodedData, imageInfo, 0, ktx2Reader.hasAlpha, isVideo);
+                }
             }
-
-            if (result && result.transcodedImage !== undefined) {
-                const textureData = result.transcodedImage.get_typed_memory_view().slice();
-                result.transcodedImage.delete();
-                return textureData;
+            finally {
+                if (transcoder) {
+                    transcoder.delete();
+                }
+
+                if (imageInfo) {
+                    imageInfo.delete();
+                }
+
+                if (result && result.transcodedImage) {
+                    textureData = result.transcodedImage.get_typed_memory_view().slice();
+                    result.transcodedImage.delete();
+                }
             }
 
-            return null;
+            return textureData;
         });
     }
 }
diff --git a/src/Misc/khronosTextureContainer2.ts b/src/Misc/khronosTextureContainer2.ts
@@ -189,8 +189,10 @@ export class KhronosTextureContainer2 {
                         worker.addEventListener("error", onError);
                         worker.addEventListener("message", onMessage);
 
-                        // note: we can't transfer the ownership of data.buffer because if using a fallback texture the data.buffer buffer will be used by the current thread
-                        worker.postMessage({ action: "decode", data, caps: compressedTexturesCaps, options }/*, [data.buffer]*/);
+                        const dataCopy = new Uint8Array(data.byteLength);
+                        dataCopy.set(new Uint8Array(data.buffer, data.byteOffset, data.byteLength));
+
+                        worker.postMessage({ action: "decode", data: dataCopy, caps: compressedTexturesCaps, options }, [dataCopy.buffer]);
                     });
                 });
             });