Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix velocity regression & bug #798

Merged
merged 10 commits into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions package/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@
}
},
"devDependencies": {
"@types/jest": "^27.0.3",
"@types/jest": "^28.1.6",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a specific reason for upgrading Jest here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so we can mark tests as expected failures.

"@types/react-native": "^0.65.0",
"@types/react-reconciler": "^0.26.4",
"eslint": "8.21.0",
"eslint-config-react-native-wcandillon": "3.9.0",
"eslint-plugin-reanimated": "2.0.0",
"jest": "^27.4.3",
"jest": "28.1.3",
"react": "17.0.2",
"react-native": "0.66.2",
"react-native-builder-bob": "^0.18.2",
"ts-jest": "^27.0.7",
"ts-jest": "^28.0.7",
"typescript": "^4.6.4"
},
"dependencies": {
Expand Down
6 changes: 2 additions & 4 deletions package/src/renderer/__tests__/Data.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ import { Fill } from "../components";
import * as SkiaRenderer from "../index";
import type { SkData } from "../../skia/types/Data/Data";

import { importSkia, mountCanvas } from "./setup";
import type { EmptyProps } from "./setup";
import { mountCanvas, importSkia } from "./setup";

const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface EmptyProps {}

const CheckData = ({}: EmptyProps) => {
const { useFont } = importSkia();
const font = useFont(null);
Expand Down
113 changes: 113 additions & 0 deletions package/src/renderer/__tests__/TouchHandler.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import React from "react";

import { TouchType } from "../../views/types";

import type { EmptyProps } from "./setup";
import { mountCanvas, importSkia } from "./setup";

const SimpleActiveTouch = ({}: EmptyProps) => {
const { useTouchHandler } = importSkia();
const touchHandler = useTouchHandler({
onStart: ({ x, y }) => {
expect(x).toBe(10);
expect(y).toBe(10);
},
onActive: ({ x, y, velocityX, velocityY }) => {
expect(x).toBe(20);
expect(y).toBe(20);
expect(velocityX).toBe(10);
expect(velocityY).toBe(10);
},
});
const history = [
[
{
x: 10,
y: 10,
force: 0,
type: TouchType.Start,
id: 2,
timestamp: 0,
},
],
[
{
x: 20,
y: 20,
force: 0,
type: TouchType.Active,
id: 2,
timestamp: 1,
},
],
];
touchHandler(history);
return null;
};

const SimpleEndTouch = ({}: EmptyProps) => {
const { useTouchHandler } = importSkia();
const touchHandler = useTouchHandler({
onStart: ({ x, y }) => {
expect(x).toBe(10);
expect(y).toBe(10);
},
onActive: ({ x, y, velocityX, velocityY }) => {
expect(x).toBe(20);
expect(y).toBe(20);
expect(velocityX).toBe(10);
expect(velocityY).toBe(10);
},
onEnd: ({ x, y, velocityX, velocityY }) => {
expect(x).toBe(30);
expect(y).toBe(30);
expect(velocityX).toBe(10);
expect(velocityY).toBe(10);
},
});
const history = [
[
{
x: 10,
y: 10,
force: 0,
type: TouchType.Start,
id: 2,
timestamp: 0,
},
],
[
{
x: 20,
y: 20,
force: 0,
type: TouchType.Active,
id: 2,
timestamp: 1,
},
],
[
{
x: 30,
y: 30,
force: 0,
type: TouchType.End,
id: 2,
timestamp: 2,
},
],
];
touchHandler(history);
return null;
};

describe("Test handling of touch information", () => {
it("Should check that the received values are correct", () => {
const { draw } = mountCanvas(<SimpleActiveTouch />);
draw();
});
it("Calculates the velocity properly onEnd event", () => {
const { draw } = mountCanvas(<SimpleEndTouch />);
draw();
});
});
13 changes: 11 additions & 2 deletions package/src/renderer/__tests__/setup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { DrawingContext } from "../DrawingContext";
import { CanvasProvider } from "../useCanvas";
import { ValueApi } from "../../values/web";
import { LoadSkiaWeb } from "../../web/LoadSkiaWeb";
import type * as SkiaExports from "../../skia";
import type * as SkiaExports from "../..";

export let font: SkiaExports.SkFont;

Expand All @@ -25,14 +25,23 @@ export let font: SkiaExports.SkFont;
})
);

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface EmptyProps {}

jest.mock("react-native", () => ({
PixelRatio: {
get(): number {
return 1;
},
},
Platform: { OS: "web" },
Image: {
resolveAssetSource: jest.fn,
},
requireNativeComponent: () => ({}),
}));

export const importSkia = (): typeof SkiaExports => require("../../skia");
export const importSkia = (): typeof SkiaExports => require("../..");

beforeAll(async () => {
await LoadSkiaWeb();
Expand Down
36 changes: 15 additions & 21 deletions package/src/views/useTouchHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import type { DependencyList } from "react";
import { useCallback, useRef } from "react";
import { PixelRatio } from "react-native";

import type { Vector } from "../skia/types";

import type {
ExtendedTouchInfo,
TouchHandlers,
Expand All @@ -15,10 +17,8 @@ const useInternalTouchHandler = (
deps: DependencyList = [],
multiTouch = false
): TouchHandler => {
const prevTouchInfoRef = useRef<{ [key: number]: TouchInfo }>({});
const prevVelocityRef = useRef<{ [key: number]: { x: number; y: number } }>(
{}
);
const prevTouchInfoRef = useRef<{ [key: number]: TouchInfo | undefined }>({});
const prevVelocityRef = useRef<{ [key: number]: Vector | undefined }>({});

return useCallback((history: Array<Array<TouchInfo>>) => {
// Process all items in the current touch history
Expand All @@ -30,36 +30,31 @@ const useInternalTouchHandler = (
}

const touch = touches[i];

const prevTouch = prevTouchInfoRef.current[touch.id];
// Calculate the velocity from the previous touch.
const timeDiffseconds =
touch.timestamp -
(prevTouchInfoRef.current[touch.id]?.timestamp ?? touch.timestamp);

const distX =
touch.x - (prevTouchInfoRef.current[touch.id]?.x ?? touch.x);
const distY =
touch.y - (prevTouchInfoRef.current[touch.id]?.y ?? touch.y);
const distX = touch.x - (prevTouch?.x ?? touch.x);
const distY = touch.y - (prevTouch?.y ?? touch.y);

if (
touch.type !== TouchType.Start &&
touch.type !== TouchType.End &&
touch.type !== TouchType.Cancelled
touch.type !== TouchType.Cancelled &&
timeDiffseconds > 0
) {
if (timeDiffseconds > 0) {
prevVelocityRef.current[touch.id] = {
x: distX / timeDiffseconds / PixelRatio.get(),
y: distY / timeDiffseconds / PixelRatio.get(),
};
} else {
prevVelocityRef.current[touch.id] = { x: 0, y: 0 };
}
prevVelocityRef.current[touch.id] = {
x: distX / timeDiffseconds / PixelRatio.get(),
y: distY / timeDiffseconds / PixelRatio.get(),
};
}

const extendedTouchInfo: ExtendedTouchInfo = {
...touch,
velocityX: prevVelocityRef.current[touch.id].x,
velocityY: prevVelocityRef.current[touch.id].y,
velocityX: prevVelocityRef.current[touch.id]?.x ?? 0,
velocityY: prevVelocityRef.current[touch.id]?.y ?? 0,
};

// Save previous touch
Expand All @@ -71,7 +66,6 @@ const useInternalTouchHandler = (
} else if (touch.type === TouchType.Active) {
handlers.onActive && handlers.onActive(extendedTouchInfo);
} else {
handlers.onActive && handlers.onActive(extendedTouchInfo);
handlers.onEnd && handlers.onEnd(extendedTouchInfo);
}
}
Expand Down