[canvaskit] Fix freeing of memory in computeTonalColors
Change-Id: I82317cf762362a4fb74933d7d7145555d397c850
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294700
Reviewed-by: Nathaniel Nifong <nifong@google.com>
diff --git a/modules/canvaskit/CHANGELOG.md b/modules/canvaskit/CHANGELOG.md
index 9b3e1ed..17da546 100644
--- a/modules/canvaskit/CHANGELOG.md
+++ b/modules/canvaskit/CHANGELOG.md
@@ -9,7 +9,8 @@
### Fixed
- A bug where loading fonts (and other memory intensive calls) would cause CanvasKit
to infrequently crash with
- `TypeError: Cannot perform %TypedArray%.prototype.set on a neutered ArrayBuffer`
+ `TypeError: Cannot perform %TypedArray%.prototype.set on a neutered ArrayBuffer`.
+ - Incorrectly freeing Malloced colors passed into computeTonalColors.
## [0.16.1] - 2020-06-04
diff --git a/modules/canvaskit/helper.js b/modules/canvaskit/helper.js
index 5c50316..7123c10 100644
--- a/modules/canvaskit/helper.js
+++ b/modules/canvaskit/helper.js
@@ -739,7 +739,7 @@
// This helper will free the given pointer unless the provided array is one
// that was returned by CanvasKit.Malloc.
function freeArraysThatAreNotMallocedByUsers(ptr, arr) {
- if (!arr || !arr['_ck']) {
+ if (!arr['_ck']) {
CanvasKit._free(ptr);
}
}
diff --git a/modules/canvaskit/interface.js b/modules/canvaskit/interface.js
index ad3547f..23a4c4a 100644
--- a/modules/canvaskit/interface.js
+++ b/modules/canvaskit/interface.js
@@ -1201,7 +1201,7 @@
colors.length, mode, flags, localMatrixPtr, colorSpace);
CanvasKit._free(colorPtr);
- freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+ pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
return lgs;
}
@@ -1216,7 +1216,7 @@
colors.length, mode, flags, localMatrixPtr, colorSpace);
CanvasKit._free(colorPtr);
- freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+ pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
return rgs;
}
@@ -1235,7 +1235,7 @@
localMatrixPtr, colorSpace);
CanvasKit._free(colorPtr);
- freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+ pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
return sgs;
}
@@ -1252,7 +1252,7 @@
colorPtr, posPtr, colors.length, mode, flags, localMatrixPtr, colorSpace);
CanvasKit._free(colorPtr);
- freeArraysThatAreNotMallocedByUsers(posPtr, pos);
+ pos && freeArraysThatAreNotMallocedByUsers(posPtr, pos);
return rgs;
}
@@ -1275,17 +1275,23 @@
// ambient: {r, g, b, a},
// spot: {r, g, b, a},
// }
-// Returns the same format
+// Returns the same format. Note, if malloced colors are passed in, the memory
+// housing the passed in colors passed in will be overwritten with the computed
+// tonal colors.
CanvasKit.computeTonalColors = function(tonalColors) {
+ // copy the colors into WASM
var cPtrAmbi = copyColorToWasmNoScratch(tonalColors['ambient']);
var cPtrSpot = copyColorToWasmNoScratch(tonalColors['spot']);
+ // The output of this function will be the same pointers we passed in.
this._computeTonalColors(cPtrAmbi, cPtrSpot);
+ // Read the results out.
var result = {
'ambient': copyColorFromWasm(cPtrAmbi),
'spot': copyColorFromWasm(cPtrSpot),
}
- freeArraysThatAreNotMallocedByUsers(cPtrAmbi);
- freeArraysThatAreNotMallocedByUsers(cPtrSpot);
+ // If the user passed us malloced colors in here, we don't want to clean them up.
+ freeArraysThatAreNotMallocedByUsers(cPtrAmbi, tonalColors['ambient']);
+ freeArraysThatAreNotMallocedByUsers(cPtrSpot, tonalColors['spot']);
return result;
}
diff --git a/modules/canvaskit/tests/core.spec.js b/modules/canvaskit/tests/core.spec.js
index 428ff9f..c6346ac 100644
--- a/modules/canvaskit/tests/core.spec.js
+++ b/modules/canvaskit/tests/core.spec.js
@@ -66,6 +66,25 @@
expect(out.spot[3]).toBeCloseTo(expectedSpot[3], 3);
});
+ it('can compute tonal colors with malloced values', () => {
+ const ambientColor = CanvasKit.Malloc(Float32Array, 4);
+ ambientColor.toTypedArray().set(CanvasKit.BLUE);
+ const spotColor = CanvasKit.Malloc(Float32Array, 4);
+ spotColor.toTypedArray().set(CanvasKit.RED);
+ const input = {
+ ambient: ambientColor,
+ spot: spotColor,
+ };
+ const out = CanvasKit.computeTonalColors(input);
+ expect(new Float32Array(out.ambient)).toEqual(CanvasKit.BLACK);
+ const expectedSpot = [0.173, 0, 0, 0.969];
+ expect(out.spot.length).toEqual(4);
+ expect(out.spot[0]).toBeCloseTo(expectedSpot[0], 3);
+ expect(out.spot[1]).toBeCloseTo(expectedSpot[1], 3);
+ expect(out.spot[2]).toBeCloseTo(expectedSpot[2], 3);
+ expect(out.spot[3]).toBeCloseTo(expectedSpot[3], 3);
+ });
+
// This helper is used for all the MakeImageFromEncoded tests.
// TODO(kjlubick): rewrite this and callers to use gm
function decodeAndDrawSingleFrameImage(imgName, goldName, done) {
