Skip to content

Commit

Permalink
fix(πŸ“): Fix dangling pointer in <Paragraph /> (#2953)
Browse files Browse the repository at this point in the history
  • Loading branch information
wcandillon authored Feb 8, 2025
1 parent ce90d70 commit 55de322
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 78 deletions.
65 changes: 20 additions & 45 deletions apps/paper/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- react-native-safe-area-context (4.10.9):
- react-native-safe-area-context (4.14.1):
- React-Core
- react-native-skia (0.0.0):
- DoubleConversion
Expand All @@ -1262,28 +1262,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- react-native-slider (4.5.2):
- DoubleConversion
- glog
- hermes-engine
- RCT-Folly (= 2024.01.01.00)
- RCTRequired
- RCTTypeSafety
- React-Core
- React-debug
- React-Fabric
- React-featureflags
- React-graphics
- React-ImageManager
- React-NativeModulesApple
- React-RCTFabric
- React-rendererdebug
- React-utils
- ReactCodegen
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- react-native-wgpu (0.1.19):
- react-native-slider (4.5.5):
- DoubleConversion
- glog
- hermes-engine
Expand Down Expand Up @@ -1564,7 +1543,7 @@ PODS:
- React-logger (= 0.75.2)
- React-perflogger (= 0.75.2)
- React-utils (= 0.75.2)
- RNGestureHandler (2.18.1):
- RNGestureHandler (2.23.0):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1585,7 +1564,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- RNReanimated (3.16.5):
- RNReanimated (3.16.7):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1605,10 +1584,10 @@ PODS:
- ReactCodegen
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- RNReanimated/reanimated (= 3.16.5)
- RNReanimated/worklets (= 3.16.5)
- RNReanimated/reanimated (= 3.16.7)
- RNReanimated/worklets (= 3.16.7)
- Yoga
- RNReanimated/reanimated (3.16.5):
- RNReanimated/reanimated (3.16.7):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1628,9 +1607,9 @@ PODS:
- ReactCodegen
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- RNReanimated/reanimated/apple (= 3.16.5)
- RNReanimated/reanimated/apple (= 3.16.7)
- Yoga
- RNReanimated/reanimated/apple (3.16.5):
- RNReanimated/reanimated/apple (3.16.7):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1651,7 +1630,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- RNReanimated/worklets (3.16.5):
- RNReanimated/worklets (3.16.7):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1672,7 +1651,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- RNScreens (4.3.0):
- RNScreens (4.6.0):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1694,7 +1673,7 @@ PODS:
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- Yoga
- RNSVG (15.10.1):
- RNSVG (15.11.1):
- React-Core
- SocketRocket (0.7.0)
- Yoga (0.0.0)
Expand Down Expand Up @@ -1740,7 +1719,6 @@ DEPENDENCIES:
- react-native-safe-area-context (from `../../../node_modules/react-native-safe-area-context`)
- "react-native-skia (from `../../../node_modules/@shopify/react-native-skia`)"
- "react-native-slider (from `../../../node_modules/@react-native-community/slider`)"
- react-native-wgpu (from `../../../node_modules/react-native-wgpu`)
- React-nativeconfig (from `../../../node_modules/react-native/ReactCommon`)
- React-NativeModulesApple (from `../../../node_modules/react-native/ReactCommon/react/nativemodule/core/platform/ios`)
- React-perflogger (from `../../../node_modules/react-native/ReactCommon/reactperflogger`)
Expand Down Expand Up @@ -1770,7 +1748,7 @@ DEPENDENCIES:
- RNGestureHandler (from `../../../node_modules/react-native-gesture-handler`)
- RNReanimated (from `../../../node_modules/react-native-reanimated`)
- RNScreens (from `../../../node_modules/react-native-screens`)
- RNSVG (from `../node_modules/react-native-svg`)
- RNSVG (from `../../../node_modules/react-native-svg`)
- Yoga (from `../../../node_modules/react-native/ReactCommon/yoga`)

SPEC REPOS:
Expand Down Expand Up @@ -1855,8 +1833,6 @@ EXTERNAL SOURCES:
:path: "../../../node_modules/@shopify/react-native-skia"
react-native-slider:
:path: "../../../node_modules/@react-native-community/slider"
react-native-wgpu:
:path: "../../../node_modules/react-native-wgpu"
React-nativeconfig:
:path: "../../../node_modules/react-native/ReactCommon"
React-NativeModulesApple:
Expand Down Expand Up @@ -1916,7 +1892,7 @@ EXTERNAL SOURCES:
RNScreens:
:path: "../../../node_modules/react-native-screens"
RNSVG:
:path: "../node_modules/react-native-svg"
:path: "../../../node_modules/react-native-svg"
Yoga:
:path: "../../../node_modules/react-native/ReactCommon/yoga"

Expand Down Expand Up @@ -1956,10 +1932,9 @@ SPEC CHECKSUMS:
React-logger: 8db32983d75dc2ad54f278f344ccb9b256e694fc
React-Mapbuffer: 1c08607305558666fd16678b85ef135e455d5c96
React-microtasksnativemodule: 87b8de96f937faefece8afd2cb3a518321b2ef99
react-native-safe-area-context: ab8f4a3d8180913bd78ae75dd599c94cce3d5e9a
react-native-safe-area-context: 141eca0fd4e4191288dfc8b96a7c7e1c2983447a
react-native-skia: 30bdfaf0c7d9c38a715f29f36102ba3e60ff3329
react-native-slider: 97ce0bd921f40de79cead9754546d5e4e7ba44f8
react-native-wgpu: 8d0437a304318e0e3d6ccbfed2a39880f8eae4dd
react-native-slider: 4a0f3386a38fc3d2d955efc515aef7096f7d1ee4
React-nativeconfig: 57781b79e11d5af7573e6f77cbf1143b71802a6d
React-NativeModulesApple: 7ff2e2cfb2e5fa5bdedcecf28ce37e696c6ef1e1
React-perflogger: 8a360ccf603de6ddbe9ff8f54383146d26e6c936
Expand All @@ -1986,10 +1961,10 @@ SPEC CHECKSUMS:
React-utils: 81a715d9c0a2a49047e77a86f3a2247408540deb
ReactCodegen: 4c29be59257644159393c3996669167e0d3f19a5
ReactCommon: 6ef348087d250257c44c0204461c03f036650e9b
RNGestureHandler: 939f21fabf5d45a725c0bf175eb819dd25cf2e70
RNReanimated: 9d20a811e6987cba268ef4f56242dfabd40e85c1
RNScreens: b03d696c70cc5235ce4587fcc27ae1a93a48f98c
RNSVG: 7ff26379b2d1871b8571e6f9bc9630de6baf9bdf
RNGestureHandler: b03ae9d1ee403a71cead5b6ab74317453cf6080a
RNReanimated: 68adeb6b44b6e5c610f4c7d2b30cb8390110b423
RNScreens: d682e5fff3175487730bf2aa308fbf0898b69410
RNSVG: 669ed128ab9005090c612a0d627dbecb6ab5c76f
SocketRocket: abac6f5de4d4d62d24e11868d7a2f427e0ef940d
Yoga: 2a45d7e59592db061217551fd3bbe2dd993817ae

Expand Down
4 changes: 4 additions & 0 deletions apps/paper/src/Examples/API/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ export const examples = [
screen: "Paragraphs",
title: "πŸ“š Text & Paragraphs",
},
{
screen: "Paragraphs2",
title: "πŸ“š Text & Paragraphs 2",
},
{
screen: "Clipping",
title: "βœ‚οΈ & 🎭 Clipping & Masking",
Expand Down
115 changes: 115 additions & 0 deletions apps/paper/src/Examples/API/Paragraphs2.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import React, { useEffect, useMemo, useState } from "react";
import { Platform, ScrollView, useWindowDimensions } from "react-native";
import type { DataModule } from "@shopify/react-native-skia";
import {
Canvas,
Group,
PaintStyle,
Paragraph,
Rect,
Skia,
mix,
useFonts,
} from "@shopify/react-native-skia";
import {
useSharedValue,
useDerivedValue,
withRepeat,
withTiming,
} from "react-native-reanimated";

const fonts: Record<string, DataModule[]> = {
Roboto: [
require("../../Tests/assets/Roboto-Medium.ttf"),
require("../../Tests/assets/Roboto-Regular.ttf"),
],
};
// On Web, we need provide a font for emojis
if (Platform.OS === "web") {
fonts.NotoColorEmoji = [require("../../Tests/assets/NotoColorEmoji.ttf")];
}
export const Paragraphs2 = () => {
const { height, width } = useWindowDimensions();
const progress = useSharedValue(1);
const [key, setKey] = useState(0);

useEffect(() => {
progress.value = withRepeat(withTiming(0, { duration: 3000 }), -1, true);
}, [progress]);

const loopedWidth = useDerivedValue(
() => mix(progress.value, width * 0.2, width * 0.8),
[progress]
);

const customFontMgr = useFonts(fonts);

// Create and store paragraph builder separately
const paragraphBuilder = useMemo(() => {
if (customFontMgr === null) {
return null;
}
return Skia.ParagraphBuilder.Make({}, customFontMgr);
}, [customFontMgr]);

// Force recreation of paragraphBuilder periodically
useEffect(() => {
const interval = setInterval(() => {
if (paragraphBuilder) {
// Force paragraph builder to reset and create new paragraph
paragraphBuilder.reset();
setKey((prev) => prev + 1);
}
}, 100); // Rapid recreation

return () => clearInterval(interval);
}, [paragraphBuilder]);

const paragraph = useMemo(() => {
if (!paragraphBuilder) {
return null;
}

paragraphBuilder.reset();
const fontSize = 20;
const strokePaint = Skia.Paint();
strokePaint.setStyle(PaintStyle.Stroke);
strokePaint.setStrokeWidth(1);

const textStyle = {
fontSize,
fontFamilies: ["Roboto", "NotoColorEmoji"],
color: Skia.Color("#000"),
};

// Simplified paragraph content for testing
paragraphBuilder
.pushStyle(textStyle)
.addText("Test Text " + key) // Add key to force rememo
.pop()
.build();

const built = paragraphBuilder.build();

// Try to force garbage collection of previous paragraph
paragraphBuilder.reset();

return built;
}, [paragraphBuilder, key]); // Depend on key to force recreation

return (
<ScrollView>
<Canvas
style={{
width,
height,
}}
>
<Group transform={[{ translateX: 30 }, { translateY: 30 }]}>
<Paragraph paragraph={paragraph} x={0} y={0} width={loopedWidth} />
<Rect x={loopedWidth} y={0} width={1} height={300} />
</Group>
</Canvas>
</ScrollView>
);
};
1 change: 1 addition & 0 deletions apps/paper/src/Examples/API/Routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ export type Routes = {
Snapshot: undefined;
IconsExample: undefined;
Paragraphs: undefined;
Paragraphs2: undefined;
};
8 changes: 8 additions & 0 deletions apps/paper/src/Examples/API/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { IconsExample } from "./Icons";
import { FontMgr } from "./FontMgr";
import { AnimatedImages } from "./AnimatedImages";
import { Paragraphs } from "./Paragraphs";
import { Paragraphs2 } from "./Paragraphs2";

const Stack = createNativeStackNavigator<Routes>();
export const API = () => {
Expand Down Expand Up @@ -67,6 +68,13 @@ export const API = () => {
title: "πŸ“š Text & Paragraphs",
}}
/>
<Stack.Screen
name="Paragraphs2"
component={Paragraphs2}
options={{
title: "πŸ“š Text & Paragraphs 2",
}}
/>
<Stack.Screen
name="Snapshot"
component={Snapshot}
Expand Down
4 changes: 1 addition & 3 deletions packages/skia/cpp/api/JsiSkParagraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,7 @@ class JsiSkParagraph : public JsiSkHostObject {
_paragraph = paragraphBuilder->Build();
}

para::Paragraph *getParagraph() { return _paragraph.get(); }

private:
public:
std::unique_ptr<para::Paragraph> _paragraph;
};

Expand Down
Loading

0 comments on commit 55de322

Please sign in to comment.