feat: add an optional results panel to MediapipeCamera #20
Conversation
| let rows: React.JSX.Element[] = []; | ||
| let count = 0; | ||
|
|
||
| if ((stats?.results.length ?? 0) > 1) { | ||
| console.log("more than one result", stats?.results.length); | ||
| } | ||
|
|
||
| if (stats?.results && stats.results.length > 0 && stats.results[0]) { | ||
| count = stats.results[0].detections.length; | ||
| rows = stats.results[0].detections.map((obj) => { | ||
| const categories = obj.categories | ||
| .map((cat) => `${cat.categoryName} (${cat.score})`) | ||
| .join(", "); | ||
| const text = `${categories}: ${JSON.stringify(obj.boundingBox)}`; | ||
| console.log(text); | ||
| return <Text>{text}</Text>; | ||
| }); | ||
| } |
There was a problem hiding this comment.
While this code is technically correct, it does things in a very un-react-y way.
- prefer const
- prefer to do rendering inline in the markup
- leverage javascript's conciseness
- use hooks (in this case
React.useMemo) to eliminate needless computation on every render
const rows = React.useMemo(()=> (stats?.results ?? []).map( r => {
const categories = r.categories.map(...).join(", ");
return `${categories}: ${JSON.stringify(obj.boundingBox)}`;
}), [stats?.results]);
return (
....
<Text>count : {rows.count}</Text>
{rows.map( r => (<Text>{r}</Text>))}
...
);There was a problem hiding this comment.
Thanks! There will probably be a decent amount of this as I ramp up since Vue and React are similar but different in ways that will likely cause things like this.
| // The linter wants `processor` and `detectorHandle` in the dependency array, but these are | ||
| // _set_ by this useEffect. Making this reactive based on these values would cause an infinite loop | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
disabling react-hooks/exhaustive-deps is almost never a good idea. Yes, the infinite loop problem is a very tricky one but by removing detectorHandle from the deps you have introduced a subtle bug where the correct detectorhandle will not get released when this component gets shut down.
It is for this very reason that I did not use a useState for detectorHandle. Probably best to discuss this in person. hooks can be tricky
There was a problem hiding this comment.
Okay - I am definitely not happy disabling linter rules without fully understanding things. The problem I have here is that according to my observation and this documentation useShareValue is not reactive on the JS side of things. So, when createDetector.then sets the shared value, it doesn't cause a re-evaluate, so we never have the right handle. At least that's what I was seeing on my machine, did the callbacks actually work for you?
In any case, I'll dig into alternate ways of getting the reactivity working correctly without disabling the lining rule if I don't hear anything from you.
There was a problem hiding this comment.
handle. At least that's what I was seein
You are correct. the act of getting the callback does not stimulate a re-render.
The re-render will be stimulated when we have some state that is being set in the onResults callback that we pass to useObjectDetection
There was a problem hiding this comment.
er, the act of setting the sharedvalue does not stimulate... is what I meant to say
There was a problem hiding this comment.
You are correct. the act of getting the callback does not stimulate a re-render.
The re-render will be stimulated when we have some state that is being set in the onResults callback that we pass to
useObjectDetection
Agreed. The fundamental problem here is an asynchronous issue. The initial call of the useLayoutEffect happens when processor.value is its initial value of undefined, so it does nothing. Then createDetector completes and sets processor.value. But since that is not reactive, the useLayoutEffect never gets called again, our detectorMap is never set up, and we never propagate the events to the container.
I tried a number of variations, like getting rid of useLayoutEffect and just setting up the map in the callback from getDetector in useEffect. That seems like it should work, even though that makes the useEffect depend on the callbacks and it means building a new detector if the consumer changes the callbacks. But it just crashes invokes useEffect twice and silently fails.
No urgency on this - I have a working (if unacceptable long-term) version of the code to continue playing with if I decide to continue on filling out the sample in the next few days.
There was a problem hiding this comment.
I see. Interesting. I will look at this later today.
|
Im going to close this PR since we are going to remove the directory |
Add a results panel and display the object detection results in the panel.
This is not meant to be a general solution but allows us to get a bit more idea of what the useObjectDetection hook is actually doing.
This is correctly displaying a panel with the inference time and a list of the object detected with their rectangles.
It also contains some bug fixes and reduces the logging to something more readable.
The most significant fix is that useObjectDetection correctly propagates callbacks the callback function that it is passed. The issue here is that
useSharedValueis reactive in the context of the native workerlet (e.g. if you change something in the shared value, the workerlet will re-run) but does not appear to be reactive in the context of the JavaScript code. See this documentation in reanimated.