-
Notifications
You must be signed in to change notification settings - Fork 107
Fix the bug where the live view of the camera would also adjust because of the drift status message. (we don't want to adjust the live view) #1135
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
base: develop
Are you sure you want to change the base?
Conversation
…se of the drift status message. (we don't want to adjust the live view)
|
We don't want to adjust the part of the height map that is in the live view of the camera. Because that drift is happening during the entire swing of the footstep. (its happening without us doing anything about it. Because the camera in on the robot. And the robot drifting. So the depth values we are reading. Are based on the camera origin which is wherever the drift has taken us.) Also if we adjust it for drift. It gets overwritten with the next frame that is pulled from the camera. So it doesn't make sense to update this and visually its a bit strange. |
Screencast.from.2025-12-11.12-51-12.mp4This video shows the new behavior which is what we want to happen. |
| } | ||
| } | ||
|
|
||
| float zeroValueForEmptySpaces = 0.0f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so the idea behind the next bit of code is to run a kernel, that's going to store the entire height map area. But the only non-zero values will be squares that have data from the local maps. At the beginning we set the entire map to be 0.0 (zeroValueForEmptySpaces) and the values get overwritten if there is data in the cell on the localMeanMap (local height map).
| private int centerIndex; | ||
| private int cellsPerAxis; | ||
| private final int centerIndex; | ||
| private final int cellsPerAxis; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleanup
| planOffsetKernel.withPointer(emptyGlobalHeightMap.data()).withLong(emptyGlobalHeightMap.step()); | ||
| planOffsetKernel.withFloat(zDriftInMeters); | ||
| planOffsetKernel.withFloat(resetOffset); | ||
| planOffsetKernel.withFloat(zeroValueForEmptySpaces); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why we were comparing the value to this resetOffset guy. We want to compare it to the zero value in the emptyGlobalHeightMap that we just computed in the kernel before this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This zerValueForEmptySpaces is always 0.0, right? I don't think you need to pass it to the kernel then, just hard code the value in the kernel. Less memory to transfer between CPU and GPU when launching the kernel that way.
Can like #define ZERO_FOR_EMPTY_SPACES 0.0f or const float ZERO_FOR_EMPTY_SPACES = 0.0f;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting thought. I'm happy to try that as a future improvement to see what the memory transfer would be. Since its currently one float (32 bits). I'm not too worried about it and I find it easier to understand if the field lives in Java so its visible to the user.
| in the live view of the camera, because: a) they will just get overwritten the next image, and b) that data doesn't drift | ||
| because its live data. So we pass in this matrix of values to skip because those values are outside the live view | ||
| of the camera. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if this makes sense!
| planOffsetKernel.withPointer(emptyGlobalHeightMap.data()).withLong(emptyGlobalHeightMap.step()); | ||
| planOffsetKernel.withFloat(zDriftInMeters); | ||
| planOffsetKernel.withFloat(resetOffset); | ||
| planOffsetKernel.withFloat(zeroValueForEmptySpaces); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This zerValueForEmptySpaces is always 0.0, right? I don't think you need to pass it to the kernel then, just hard code the value in the kernel. Less memory to transfer between CPU and GPU when launching the kernel that way.
Can like #define ZERO_FOR_EMPTY_SPACES 0.0f or const float ZERO_FOR_EMPTY_SPACES = 0.0f;
| } | ||
| } | ||
|
|
||
| float zeroValueForEmptySpaces = 0.0f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static final float ZERO_FOR_EMPTY_SPACES = 0.0f; at top of file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok to leave it if you are. I like it being close to the code that's using it.
The drift works by adjusting based on a status message that is received when the current footstep completes. The value we receive determines how much the height map jumps in values. You can see in this video how its working on develop.
Screencast.from.2025-12-11.12-48-11.mp4