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

Runtime snapshot runtime tests of Layout Animation on fabric #6538

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type NativeUpdate = {
export function createUpdatesContainer() {
const jsUpdates = makeMutable<Array<JsUpdate>>([]);
const nativeSnapshots = makeMutable<Array<NativeUpdate>>([]);
const unimplementedNativeSnapshotsOnFabric = makeMutable(false);

function _updateNativeSnapshot(updateInfos: JsUpdate[], jsUpdateIndex: number): void {
'worklet';
Expand All @@ -36,7 +37,7 @@ export function createUpdatesContainer() {
for (const prop of propsToUpdate) {
snapshot[prop] = isFabric
? global._obtainPropFabric(updateInfo?.shadowNodeWrapper, prop)
: global._obtainPropPaper(updateInfo?.tag, prop);
: global._obtainPropPaper(updateInfo.tag, prop);
}
values.push({
tag: updateInfo.tag,
Expand All @@ -51,6 +52,7 @@ export function createUpdatesContainer() {

function _updateJsSnapshot(newUpdates: JsUpdate[]): void {
'worklet';

jsUpdates.modify(updates => {
for (const update of newUpdates) {
updates.push(update);
Expand Down Expand Up @@ -82,16 +84,18 @@ export function createUpdatesContainer() {

function pushLayoutAnimationUpdates(tag: number, update: Record<string, unknown>) {
'worklet';
if (global._IS_FABRIC) {
// layout animation doesn't work on Fabric yet
return;
}

// Deep Copy, works with nested objects, but doesn't copy functions (which should be fine here)
const updatesCopy = JSON.parse(JSON.stringify(update));
if ('backgroundColor' in updatesCopy) {
updatesCopy.backgroundColor = convertDecimalColor(updatesCopy.backgroundColor);
}
_updateNativeSnapshot([{ tag, update }], jsUpdates.value.length - 1);
if (!global._IS_FABRIC) {
// TODO Implement native snapshots for layout animations on Fabric
_updateNativeSnapshot([{ tag, update }], jsUpdates.value.length - 1);
} else {
unimplementedNativeSnapshotsOnFabric.value = true;
}
jsUpdates.modify(updates => {
updates.push({
tag,
Expand All @@ -106,6 +110,7 @@ export function createUpdatesContainer() {
propsNames: string[],
): MultiViewSnapshot {
const updatesForTag: Record<number, Array<OperationUpdate>> = {};

for (const updateRequest of updates) {
const { tag } = updateRequest;

Expand Down Expand Up @@ -134,10 +139,16 @@ export function createUpdatesContainer() {
if (viewTags.length === 1) {
return sortedUpdates[Number(viewTags[0])];
}
if (viewTags.length === 0) {
throw new Error("Didn't record any snapshot");
}
throw new Error('Recorded snapshots of many views, specify component you want to get snapshot of');
}
const tag = component?.getTag();
if (!tag || !(tag in sortedUpdates)) {
if (_IS_FABRIC && (-1) in sortedUpdates) {
return sortedUpdates[-1];
}
Comment on lines +149 to +151
Copy link
Member

Choose a reason for hiding this comment

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

This is something we need to verify. Does it happen all the time or just sometimes? Additionally, it will break in cases where we want to test animations for more than one component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it happens all the time and breaks some of our animations. However most of our tests features only one component.

throw new Error('Snapshot of given component not found');
} else {
return sortedUpdates[tag];
Expand All @@ -150,6 +161,9 @@ export function createUpdatesContainer() {
}

async function getNativeSnapshots(component?: TestComponent, propsNames: string[] = []): Promise<SingleViewSnapshot> {
if (unimplementedNativeSnapshotsOnFabric.value) {
return [];
}
const nativeSnapshotsCount = nativeSnapshots.value.length;
const jsUpdatesCount = jsUpdates.value.length;
if (jsUpdatesCount === nativeSnapshotsCount) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ function compareSnapshot(
const comparisonMode = isValidPropName(key) ? getComparisonModeForProp(key) : ComparisonMode.AUTO;
const isEqual = getComparator(comparisonMode);
const expectMismatch = jsValue < 0 && expectNegativeValueMismatch;
const valuesAreMatching = isEqual(jsValue, nativeValue);
let valuesAreMatching = isEqual(jsValue, nativeValue);

if ((key as string) === 'opacity' && jsValue === 1 && nativeValue === undefined) {
// undefined opacity may get translated into 1, as it is the default value
valuesAreMatching = true;
}
Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

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

Also, here we need to verify why in the JS update, we can receive undefined values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In our layout animations we use function maybeRestoreOpacity a lot. After the restoration the opacity is explicitly defined as 1, even if it had been undefined.


if ((!valuesAreMatching && !expectMismatch) || (valuesAreMatching && expectMismatch)) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('withDecay animation, test various config', () => {
{ velocity: 900, clamp: [0, 150], rubberBandEffect: true },
{ velocity: 2000, clamp: [0, 150], rubberBandEffect: true },
{ velocity: 2000, clamp: [0, 150], rubberBandEffect: true, rubberBandFactor: 2 },
] as Array<WithDecayConfig>)('Config ${0}', async config => {
] as Array<WithDecayConfig>)('Config %p', async config => {
const snapshotName = ('decay_' +
Object.entries(config)
.map(([key, val]) => {
Expand Down