[canvaskit] Always serialize Typeface data into SKPs
Before: http://screen/4gnRTnRGyBuEsA9
After: http://screen/5jdoRcED7X3Taf7
There's a few misc fixes related to Bazel rules and tests too.
Change-Id: I2ac272fe5312c2a825944259f2f9031fff9887e0
Bug: skia:13247
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535477
Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/modules/canvaskit/CHANGELOG.md b/modules/canvaskit/CHANGELOG.md
index f088dd7..25da42a 100644
--- a/modules/canvaskit/CHANGELOG.md
+++ b/modules/canvaskit/CHANGELOG.md
@@ -22,6 +22,10 @@
- If an invalid matrix type is passed in (e.g. not an array, TypedArray, or DOMMatrix), CanvasKit
will throw instead of drawing incorrectly.
+### Fixed
+ - SkParagraph objects no longer have their glyphs garbled when stored to an SkPicture.
+ (skbug.com/13247)
+
## [0.33.0] - 2022-02-03
### Added
diff --git a/modules/canvaskit/canvaskit_bindings.cpp b/modules/canvaskit/canvaskit_bindings.cpp
index 6377c07..e2fc488 100644
--- a/modules/canvaskit/canvaskit_bindings.cpp
+++ b/modules/canvaskit/canvaskit_bindings.cpp
@@ -31,6 +31,7 @@
#include "include/core/SkRRect.h"
#include "include/core/SkSamplingOptions.h"
#include "include/core/SkScalar.h"
+#include "include/core/SkSerialProcs.h"
#include "include/core/SkShader.h"
#include "include/core/SkString.h"
#include "include/core/SkStrokeRec.h"
@@ -680,6 +681,10 @@
}
#endif
+sk_sp<SkData> alwaysSaveTypefaceBytes(SkTypeface* face, void*) {
+ return face->serialize(SkTypeface::SerializeBehavior::kDoIncludeData);
+}
+
// These objects have private destructors / delete methods - I don't think
// we need to do anything other than tell emscripten to do nothing.
namespace emscripten {
@@ -1664,9 +1669,15 @@
// that clients should ever rely on. The format may change at anytime and no promises
// are made for backwards or forward compatibility.
.function("serialize", optional_override([](SkPicture& self) -> Uint8Array {
- // Emscripten doesn't play well with optional arguments, which we don't
- // want to expose anyway.
- sk_sp<SkData> data = self.serialize();
+ // We want to make sure we always save the underlying data of the Typeface to the
+ // SkPicture. By default, the data for "system" fonts is not saved, just an identifier
+ // (e.g. the family name and style). We do not want the user to have to supply a
+ // FontMgr with the correct fonts by name when deserializing, so we choose to always
+ // serialize the underlying data. This makes the SKPs a bit bigger, but easier to use.
+ SkSerialProcs sp;
+ sp.fTypefaceProc = &alwaysSaveTypefaceBytes;
+
+ sk_sp<SkData> data = self.serialize(&sp);
if (!data) {
return emscripten::val::null();
}
diff --git a/modules/canvaskit/tests/bazel/paragraph_test.js b/modules/canvaskit/tests/bazel/paragraph_test.js
index cb7442f..333271a 100644
--- a/modules/canvaskit/tests/bazel/paragraph_test.js
+++ b/modules/canvaskit/tests/bazel/paragraph_test.js
@@ -1030,4 +1030,46 @@
fontMgr.delete();
builder.delete();
});
+
+ // This helped find and resolve skbug.com/13247
+ gm('paragraph saved to skpicture', (canvas) => {
+ canvas.clear(CanvasKit.WHITE);
+ const fontMgr = CanvasKit.FontMgr.FromData(notoSerifFontBuffer, notoSerifBoldItalicFontBuffer);
+
+ const wrapTo = 250;
+
+ const paraStyle = new CanvasKit.ParagraphStyle({
+ textStyle: {
+ fontSize: 20,
+ },
+ });
+
+ const builder = CanvasKit.ParagraphBuilder.Make(paraStyle, fontMgr);
+ builder.addText('This was saved to an SkPicture\n');
+
+ const boldItalic = new CanvasKit.TextStyle({
+ fontStyle: {
+ weight: CanvasKit.FontWeight.Bold,
+ slant: CanvasKit.FontSlant.Italic,
+ }
+ });
+ builder.pushStyle(boldItalic);
+ builder.addText(`Bold, Italic\n`);
+ builder.pop();
+ const paragraph = builder.build();
+ paragraph.layout(wrapTo);
+
+ const recorder = new CanvasKit.PictureRecorder();
+ const skpCanvas = recorder.beginRecording(CanvasKit.LTRBRect(0, 0, 200, 200));
+ skpCanvas.drawParagraph(paragraph, 10, 10);
+ const picture = recorder.finishRecordingAsPicture();
+
+ canvas.drawPicture(CanvasKit.MakePicture(picture.serialize()));
+
+ picture.delete();
+ recorder.delete();
+ paragraph.delete();
+ fontMgr.delete();
+ builder.delete();
+ });
});
diff --git a/modules/canvaskit/tests/bazel/test_reporter.js b/modules/canvaskit/tests/bazel/test_reporter.js
index b8b9e67..408a1e6 100644
--- a/modules/canvaskit/tests/bazel/test_reporter.js
+++ b/modules/canvaskit/tests/bazel/test_reporter.js
@@ -2,7 +2,8 @@
const pngPrefx = 'data:image/png;base64,'
function reportCanvas(canvas, testname) {
- // toDataURL returns a base64 encoded string with a data prefix. We only
+ testname = testname.replaceAll(' ', '_');
+ // toDataURL returns a base64 encoded string with a data prefix. We only
// want the PNG data itself, so we strip that off before submitting it.
const b64 = canvas.toDataURL('image/png')
.substring(pngPrefx.length);
diff --git a/third_party/BUILD.bazel b/third_party/BUILD.bazel
index e94b5ad..966344d 100644
--- a/third_party/BUILD.bazel
+++ b/third_party/BUILD.bazel
@@ -158,6 +158,7 @@
ZLIB_COPTS = [
"-Wno-unused-function",
+ "-Wno-deprecated-non-prototype",
# Make the headers in contrib available, without exposing them in hdrs.
"-isystem third_party/externals/zlib/",
] + select({
@@ -481,6 +482,7 @@
})
WEBP_COPTS = [
+ "-Wno-unused-but-set-variable",
"-isystem third_party/externals/libwebp/",
] + select({
"@platforms//cpu:x86_64": ["-msse4.1"],
