Skip to content

Commit 355ff55

Browse files
authored
Merge pull request #2074 from Shopify/paragraph-builder
Fix substancial performance bottleneck fon ParagraphBuilderFactory
2 parents 177f908 + 937ba31 commit 355ff55

File tree

3 files changed

+61
-33
lines changed

3 files changed

+61
-33
lines changed

package/cpp/api/JsiSkApi.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "JsiSkMatrix.h"
2626
#include "JsiSkPaint.h"
2727
#include "JsiSkParagraphBuilder.h"
28+
#include "JsiSkParagraphBuilderFactory.h"
2829
#include "JsiSkPath.h"
2930
#include "JsiSkPathEffect.h"
3031
#include "JsiSkPathEffectFactory.h"
@@ -62,7 +63,9 @@ class JsiSkApi : public JsiSkHostObject {
6263
*/
6364
JsiSkApi(jsi::Runtime &runtime, std::shared_ptr<RNSkPlatformContext> context)
6465
: JsiSkHostObject(context) {
65-
66+
// We create the system font manager eagerly since it has proven to be too
67+
// slow to do it on demand
68+
JsiSkFontMgrFactory::getFontMgr(getContext());
6669
installFunction("Font", JsiSkFont::createCtor(context));
6770
installFunction("Paint", JsiSkPaint::createCtor(context));
6871
installFunction("RSXform", JsiSkRSXform::createCtor(context));

package/cpp/api/JsiSkParagraphBuilder.h

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <JsiSkFont.h>
99
#include <JsiSkFontMgr.h>
10+
#include <JsiSkFontMgrFactory.h>
1011
#include <JsiSkHostObjects.h>
1112
#include <JsiSkParagraph.h>
1213
#include <JsiSkParagraphStyle.h>
@@ -113,7 +114,8 @@ class JsiSkParagraphBuilder : public JsiSkHostObject {
113114
sk_sp<SkFontMgr> fontManager)
114115
: JsiSkHostObject(std::move(context)) {
115116
_fontCollection = sk_make_sp<para::FontCollection>();
116-
_fontCollection->setDefaultFontManager(getContext()->createFontMgr());
117+
auto fontMgr = JsiSkFontMgrFactory::getFontMgr(getContext());
118+
_fontCollection->setDefaultFontManager(fontMgr);
117119
if (fontManager != nullptr) {
118120
_fontCollection->setAssetFontManager(fontManager);
119121
}
@@ -125,35 +127,4 @@ class JsiSkParagraphBuilder : public JsiSkHostObject {
125127
std::unique_ptr<para::ParagraphBuilder> _builder;
126128
sk_sp<para::FontCollection> _fontCollection;
127129
};
128-
129-
/**
130-
Implementation of the ParagraphBuilderFactory for making ParagraphBuilder JSI
131-
object
132-
*/
133-
class JsiSkParagraphBuilderFactory : public JsiSkHostObject {
134-
public:
135-
JSI_HOST_FUNCTION(Make) {
136-
// Get paragraph style from params
137-
auto paragraphStyle =
138-
count > 0 ? JsiSkParagraphStyle::fromValue(runtime, arguments[0])
139-
: para::ParagraphStyle();
140-
141-
// get font manager
142-
auto fontMgr =
143-
count > 1 ? JsiSkTypefaceFontProvider::fromValue(runtime, arguments[1])
144-
: nullptr;
145-
146-
// Create the paragraph builder
147-
return jsi::Object::createFromHostObject(
148-
runtime, std::make_shared<JsiSkParagraphBuilder>(
149-
getContext(), paragraphStyle, fontMgr));
150-
}
151-
152-
JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkParagraphBuilderFactory, Make))
153-
154-
explicit JsiSkParagraphBuilderFactory(
155-
std::shared_ptr<RNSkPlatformContext> context)
156-
: JsiSkHostObject(std::move(context)) {}
157-
};
158-
159130
} // namespace RNSkia
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#pragma once
2+
3+
#include <memory>
4+
#include <utility>
5+
6+
#include <jsi/jsi.h>
7+
8+
#include <JsiSkParagraphBuilder.h>
9+
#include <JsiSkParagraphStyle.h>
10+
11+
#pragma clang diagnostic push
12+
#pragma clang diagnostic ignored "-Wdocumentation"
13+
14+
#include "ParagraphBuilder.h"
15+
16+
#pragma clang diagnostic pop
17+
18+
namespace RNSkia {
19+
20+
namespace jsi = facebook::jsi;
21+
22+
namespace para = skia::textlayout;
23+
24+
/**
25+
Implementation of the ParagraphBuilderFactory for making ParagraphBuilder JSI
26+
object
27+
*/
28+
class JsiSkParagraphBuilderFactory : public JsiSkHostObject {
29+
public:
30+
JSI_HOST_FUNCTION(Make) {
31+
// Get paragraph style from params
32+
auto paragraphStyle =
33+
count > 0 ? JsiSkParagraphStyle::fromValue(runtime, arguments[0])
34+
: para::ParagraphStyle();
35+
36+
// get font manager
37+
auto fontMgr =
38+
count > 1 ? JsiSkTypefaceFontProvider::fromValue(runtime, arguments[1])
39+
: nullptr;
40+
41+
// Create the paragraph builder
42+
return jsi::Object::createFromHostObject(
43+
runtime, std::make_shared<JsiSkParagraphBuilder>(
44+
getContext(), paragraphStyle, fontMgr));
45+
}
46+
47+
JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkParagraphBuilderFactory, Make))
48+
49+
explicit JsiSkParagraphBuilderFactory(
50+
std::shared_ptr<RNSkPlatformContext> context)
51+
: JsiSkHostObject(std::move(context)) {}
52+
};
53+
54+
} // namespace RNSkia

0 commit comments

Comments
 (0)