69e46da716
The bug here is very subtle, as is the mitigation.
Quick background on WASM memory, there is an object
called wasmMemory (which might be hoisted into scope for
CanvasKit's pre-js functions), of type WebAssembly.Memory
which is a resizable ArrayBuffer. Emscripten provides the
JS code to initialize this and handle size increases.
Emscripten also provides TypedArray "views" into this buffer.
These are called CanvasKit.HEAPU8, CanvasKit.HEAPF32, etc.
When there is a call to CanvasKit._malloc, wasmMemory may
be resized. If that happens, the previous TypedArray views
become invalid. However, in the same call to _malloc,
emscripten will refresh the views [1]. So, dealing with
CanvasKit.HEAPU8 directly (quick aside, we never expect clients
to mess with these views, only us in our glue JS code
[e.g. interface.js]), should always be safe because if they
were to be invalidated in a call to _malloc, the views would
be refreshed before _malloc continues.
The problem that existed before was when we were passing
CanvasKit.HEAP* as a parameter to a function, in which the
function would call _malloc before using the typed array
parameter:
//... let us suppose wasmMemory is backed by ArrayBuffer D
copy1dArray(arr, HEAPU32);
// The HEAPU32 TypedArray (backed by ArrayBuffer D) is stored
// to a function parameter "dest"
function copy1dArray(arr, dest, ptr) {
// ...
if (!ptr) {
ptr = CanvasKit._malloc(arr.length * dest.BYTES_PER_ELEMENT);
// Suppose _malloc needs to resize wasmMemory and is
// now backed by ArrayBuffer E.
// Note: The field CanvasKit.HEAPU32 is correctly backed
// by ArrayBuffer E, but variable dest still points to a
// TypedArray backed by ArrayBuffer D.
}
// dest.set will fail with a "neutered ArrayBuffer" error
// because ArrayBuffer D is effectively gone (replaced by E).
dest.set(arr, ptr / dest.BYTES_PER_ELEMENT);
The fix here is to pass in the field name indicating the TypedArray
view we want to write our data into instead of using the
view itself as the parameter.
[1] e427159553/src/preamble.js (L344)
Change-Id: I46cfb98f8bdf928b61690a5ced034a5961356398
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294516
Reviewed-by: Nathaniel Nifong <nifong@google.com>
30 lines
1.6 KiB
JavaScript
30 lines
1.6 KiB
JavaScript
CanvasKit._extraInitializations = CanvasKit._extraInitializations || [];
|
|
CanvasKit._extraInitializations.push(function() {
|
|
CanvasKit.SkRuntimeEffect.prototype.makeShader = function(floats, isOpaque, localMatrix) {
|
|
// We don't need to free these floats because they will become owned by the shader.
|
|
var fptr = copy1dArray(floats, "HEAPF32");
|
|
var localMatrixPtr = copy3x3MatrixToWasm(localMatrix);
|
|
// Our array has 4 bytes per float, so be sure to account for that before
|
|
// sending it over the wire.
|
|
return this._makeShader(fptr, floats.length * 4, !!isOpaque, localMatrixPtr);
|
|
}
|
|
|
|
// childrenWithShaders is an array of other shaders (e.g. SkImage.makeShader())
|
|
CanvasKit.SkRuntimeEffect.prototype.makeShaderWithChildren = function(floats, isOpaque, childrenShaders, localMatrix) {
|
|
// We don't need to free these floats because they will become owned by the shader.
|
|
var fptr = copy1dArray(floats, "HEAPF32");
|
|
var localMatrixPtr = copy3x3MatrixToWasm(localMatrix);
|
|
var barePointers = [];
|
|
for (var i = 0; i < childrenShaders.length; i++) {
|
|
// childrenShaders are emscriptens smart pointer type. We want to get the bare pointer
|
|
// and send that over the wire, so it can be re-wrapped as an sk_sp.
|
|
barePointers.push(childrenShaders[i].$$.ptr);
|
|
}
|
|
var childrenPointers = copy1dArray(barePointers, "HEAPU32");
|
|
// Our array has 4 bytes per float, so be sure to account for that before
|
|
// sending it over the wire.
|
|
return this._makeShaderWithChildren(fptr, floats.length * 4, !!isOpaque, childrenPointers,
|
|
barePointers.length, localMatrixPtr);
|
|
}
|
|
});
|