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

Emit changes to pointersInside #3348

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oblador
Copy link
Contributor

@oblador oblador commented Jan 21, 2025

Description

Currently onHandlerStateChange will only send events whenever the handler state changes. However for the use case where one wants to resign highlight from the button whenever the user moves their pointer away from it, the only way is to use the onGestureEvent. This handler is however exceptionally noisy especially on Android, to the extent where I get UI thread frame drops when using worklets.

This MR emits additional events for the quite rare case where pointersInside prop changes for active gesture handlers to support this use case in a performant way.

Test plan

@latekvo
Copy link
Contributor

latekvo commented Jan 28, 2025

Hey, do i understand correctly that you want to detect a touch leaving the handled area, while the handler is still active?
Calling onHandlerStateChange despite lack of state change sounds like a significant anti-pattern, and will likely lead to some issues.

I think you should be able to achieve this effect using Gesture.Pan, and it's onUpdate callback, which gives you access to touch coordinates on each touch movement. You should be able to use those coordinates to determine if the touch is inside, or outside your desired area.

@oblador
Copy link
Contributor Author

oblador commented Jan 28, 2025

Yes exactly. Since the pointersInside prop is emitted in the onHandlerStateChange I expect it to get updated whenever it changes. onUpdate is incredibly spammy on Android and tanks performance when using a worklet.

@latekvo
Copy link
Contributor

latekvo commented Jan 28, 2025

I did some tests comparing onUpdate, and I see what you mean - using onUpdate to perform UI updates is approximately 80% more expensive than performing UI updates without using onUpdate. This likely shouldn't be the case, especially since onUpdate is already running on the UI thread, and its only job in the repro below, is toggling a single boolean shared value.
I'll look into why this issue might be occurring.

Collapsed repro
import React, { useRef } from 'react';
import { StyleSheet, Text, View } from 'react-native';
import {
  Gesture,
  GestureDetector,
  ScrollView,
} from 'react-native-gesture-handler';
import Animated, {
  useAnimatedStyle,
  useSharedValue,
  withSpring,
} from 'react-native-reanimated';

export default function EmptyExample() {
  console.log('rerendering');
  let measurementStart = useRef(performance.now());
  let measurementCount = useRef(0);

  const spamCounter = useSharedValue(0);
  const highlight = useSharedValue(false);

  const animatedStyle = useAnimatedStyle(() => {
    measurementCount.current++;
    if (measurementCount.current > 10) {
      // used to confirm same amount of updates-per-frame occurs both from `onUpdate` and from `spamCounter`
      console.log(
        '10 updates took:',
        (performance.now() - measurementStart.current).toFixed(2),
        'ms'
      );
      measurementCount.current = 0;
      measurementStart.current = performance.now();
    }

    const randomOffset = Math.random() * 2;

    if (spamCounter.value > 0) {
      spamCounter.value++;
      highlight.value = !highlight.value;
    }

    return {
      backgroundColor: highlight.value ? '#f2f2f2' : '#ffcfcf',
      transform: [
        {
          translateX: highlight.value
            ? withSpring(-randomOffset)
            : randomOffset,
        },
        {
          translateY: highlight.value
            ? -randomOffset
            : withSpring(randomOffset),
        },
      ],
    };
  });

  const pan = Gesture.Pan().onUpdate(() => {
    highlight.value = !highlight.value;
  });

  const tap = Gesture.Tap().onStart(() => {
    if (spamCounter.value > 0) {
      spamCounter.value = 0;
    } else {
      spamCounter.value++;
    }
  });

  return (
    <View style={styles.container}>
      <GestureDetector gesture={tap}>
        <Animated.View>
          <Text style={{ fontSize: 32, opacity: 0.25 }}>
            Toggle REA-ONLY spam
          </Text>
        </Animated.View>
      </GestureDetector>
      <GestureDetector gesture={pan}>
        <Animated.View style={[animatedStyle]}>
          <Text style={{ fontSize: 128, opacity: 0.25 }}>🎯</Text>
        </Animated.View>
      </GestureDetector>
      <ScrollView>
        {Array(100)
          .fill(0)
          .map((_, idx) => (
            <Animated.View style={[animatedStyle]} key={idx}>
              <Text style={{ fontSize: 128, opacity: 0.25 }}>🎯</Text>
            </Animated.View>
          ))}
      </ScrollView>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
  },
});

Despite this, i think sending out onHandlerStateChange despite no state changes is a bad idea.

(CC @m-bert @j-piasecki)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants