Cropping mechanism #1602

GGreenix · 2024-11-23T00:12:29Z

Added cropping mechanism, the client side was done by someone else and I just continued its work and finished the core mechanism, the dynamic cropping is working as well and can be transmitted from a NT table via setDynamicCropping method of a PhotonCamera object.

Some stuff still needs to be polished but overall I want to understand if there are certain stuff that needs to be changed before I continue.

thanks in advance looking forward to see it in production :D

mcm001 · 2024-11-23T00:14:28Z

photon-client/package.json

@@ -1,50 +0,0 @@
-{
-  "name": "photonclient",


Why was all of photon client deleted?

I had to branch out from the new master so I copied the code from the other guys branch and the one I worked on some to the new one, its overall the same just with the cropping tab

mcm001 · 2024-11-23T00:15:04Z

Supersedes #1446

mcm001 · 2024-11-23T06:15:02Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/AprilTagDetectionPipe.java

@@ -59,7 +59,6 @@ protected List<AprilTagDetection> process(CVMat in) {
    public void setParams(AprilTagDetectionPipeParams newParams) {
        if (this.params == null || !this.params.equals(newParams)) {
            m_detector.setConfig(newParams.detectorParams);
-            m_detector.setQuadThresholdParameters(newParams.quadParams);


mcm001 · 2024-11-23T06:15:09Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/AprilTagDetectionPipeParams.java

-    public final AprilTagDetector.QuadThresholdParameters quadParams;

-    public AprilTagDetectionPipeParams(
-            AprilTagFamily tagFamily,
-            AprilTagDetector.Config config,
-            AprilTagDetector.QuadThresholdParameters quadParams) {
+    public AprilTagDetectionPipeParams(AprilTagFamily tagFamily, AprilTagDetector.Config config) {


revert these changes?

mcm001 · 2024-11-23T06:15:31Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/DrawCalibrationPipe.java

-                if (c.x < 0 || c.y < 0) {
-                    // Skip if the corner is less than zero
-                    continue;
-                }


Why this diff?

mcm001 · 2024-11-23T06:15:47Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/DrawCalibrationPipe.java

            this.divisor = divisor;
-            this.drawAllSnapshots = drawSnapshots;


accidental changes here? restore?

mcm001 · 2024-11-23T06:16:41Z

photon-core/src/main/java/org/photonvision/vision/pipeline/AdvancedPipelineSettings.java

+    public int dynamic_x = 0;
+    public int dynamic_y = 0;
+    public int dynamic_width = Integer.MAX_VALUE;
+    public int dynamic_height = Integer.MAX_VALUE;


Does this make sense to serialize?

More in general I'm not sure how I feel about dynamic crop via NT -- it's pretty perpendicular to all our other stuff to date.

mcm001 · 2024-11-23T06:17:15Z

photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java

+        Rect staticCrop = settings.getStaticCrop();
+
+        staticCropPipe.setParams(staticCrop);
+        staticCropPipe.setDynamicRect(settings.getDynamicCrop());


static crop
setdynamic

hmmm

mcm001 · 2024-11-23T06:17:27Z

photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java

-        var quadParams = new AprilTagDetector.QuadThresholdParameters();
-        // 5 was the default minClusterPixels in WPILib prior to 2025
-        // increasing it causes detection problems when decimate > 1
-        quadParams.minClusterPixels = 5;
-        // these are the same as the values in WPILib 2025
-        // setting them here to prevent upstream changes from changing behavior of the detector
-        quadParams.maxNumMaxima = 10;
-        quadParams.criticalAngle = 45 * Math.PI / 180.0;
-        quadParams.maxLineFitMSE = 10.0f;
-        quadParams.minWhiteBlackDiff = 5;
-        quadParams.deglitch = false;
-
-        aprilTagDetectionPipe.setParams(


accidental changes?

mcm001 · 2024-11-23T17:24:49Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/CropPipe.java

+        Mat mat = in.getMat();
+        // if (fullyCovers(params, mat)) {
+        //     return in;
+        // }
+        Rect proccesor = false ? this.params : dynamiRect;
+
+        int x = MathUtil.clamp(proccesor.x, 0, mat.width());
+        int y = MathUtil.clamp(proccesor.y, 0, mat.height());
+        int width = MathUtil.clamp(proccesor.width, 0, mat.width() - x);
+        int height = MathUtil.clamp(proccesor.height, 0, mat.height() - y);
+
+        return new CVMat(mat.submat(y, y + height, x, x + width));


The submat here is just a submat of the overall data array, and does not actually own the native memory. but it still needs to be released. So we need to make sure that we release both after we are done with them. I'd argue we should release the cropped Mat immediately after Apriltag detection, since we don't use it anywhere else. See this example:

@Test public void testRelease() { var pipe = new CropPipe(); pipe.setParams(new Rect(10, 10, 50, 50)); var frameProvider = new FileFrameProvider( TestUtils.getApriltagImagePath(TestUtils.ApriltagTestImages.kTag1_640_480, false), TestUtils.WPI2020Image.FOV, TestUtils.get2020LifeCamCoeffs(false)); frameProvider.requestFrameThresholdType(FrameThresholdType.GREYSCALE); var frame = frameProvider.get(); var out = pipe.run(frame.processedImage); System.out.println(frame.processedImage.getMat()); System.out.println(out.output.getMat()); Imgcodecs.imwrite("pre_release_full.png", frame.processedImage.getMat()); Imgcodecs.imwrite("pre_release_crop.png", out.output.getMat()); frame.release(); System.out.println(frame.processedImage.getMat()); System.out.println(out.output.getMat()); Imgcodecs.imwrite("pose_releaseframe_crop.png", out.output.getMat()); out.output.getMat().release(); System.out.println(frame.processedImage.getMat()); System.out.println(out.output.getMat()); }

Output is:

// Starting state Mat [ 480*640*CV_8UC1, isCont=true, isSubmat=false, nativeObj=0x7ff994f80a30, dataAddr=0x7ff994fb26c0 ] Mat [ 50*50*CV_8UC1, isCont=false, isSubmat=true, nativeObj=0x7ff994f87910, dataAddr=0x7ff994fb3fca ] // After releasing the full image Mat Mat [ 0*0*CV_8UC1, isCont=true, isSubmat=false, nativeObj=0x7ff994f80a30, dataAddr=0x0 ] Mat [ 50*50*CV_8UC1, isCont=false, isSubmat=true, nativeObj=0x7ff994f87910, dataAddr=0x7ff994fb3fca ] // after releasing both Mat [ 0*0*CV_8UC1, isCont=true, isSubmat=false, nativeObj=0x7ff994f80a30, dataAddr=0x0 ] Mat [ 0*0*CV_8UC1, isCont=false, isSubmat=true, nativeObj=0x7ff994f87910, dataAddr=0x0 ]

…onvision into cropping-mechanism

Added CropPipeParams object and all depending pipelines updated accordingly

Juniormunk

Looks like the robot examples contain changes as that shouldn't happen either.

Juniormunk · 2024-11-25T02:17:03Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/AprilTagDetectionPipeParams.java

This file should not have changed.

Juniormunk · 2024-11-25T02:17:49Z

photon-core/src/main/java/org/photonvision/vision/pipe/impl/DrawCalibrationPipe.java

This file should not have changed.

crschardt · 2024-11-25T05:03:03Z

photon-server/src/main/resources/web/index.html

This file is autogenerated and is not supposed to be committed.

After resetting it back to the version in master, you can run git update-index --skip-worktree photon-server/src/main/resources/web/index.html to prevent git from ever seeing it as changed and adding it to a commit.

crschardt · 2024-11-25T05:07:50Z

photon-server/src/main/resources/web/index.html

+<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <link rel="icon" href="./favicon.ico" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>Photon Client</title>
+    <script type="module" crossorigin src="./assets/index-CIrfZRaJ.js"></script>
+    <link rel="stylesheet" crossorigin href="./assets/index-Bt-HXv2B.css">
+  </head>
+  <body>
+    <div id="app"></div>
+  </body>
+</html>


Accepting this suggestion will revert it to the version in master:

Suggested change

<!doctype html>

<html lang="en">

<head>

<meta charset="UTF-8" />

<link rel="icon" href="./favicon.ico" />

<meta name="viewport" content="width=device-width, initial-scale=1.0" />

<title>Photon Client</title>

<script type="module" crossorigin src="./assets/index-CIrfZRaJ.js"></script>

<link rel="stylesheet" crossorigin href="./assets/index-Bt-HXv2B.css">

</head>

<body>

<div id="app"></div>

</body>

</html>

<p>UI has not been copied!</p>

crschardt · 2024-11-25T05:41:27Z

photonlib-java-examples/aimandrange/src/main/java/frc/robot/Robot.java

All the changes to files under photonlib-java-examples/aimandrange/ are replacing the existing "Aim and Range" example with an example for using cropping. The cropping example should be moved to it's own folder and these files should be reverted to be identical to master.

GGreenix added 3 commits November 21, 2024 16:02

working cropping with fixed WPI tag detection

880ab3c

added other tab and mikvah

fceb945

dynamic cropping is lit

b51a3bd

GGreenix requested a review from a team as a code owner November 23, 2024 00:12

mcm001 reviewed Nov 23, 2024

View reviewed changes

mcm001 force-pushed the cropping-mechanism branch from b51a3bd to 0ee6405 Compare November 23, 2024 05:59

Alextopher added 3 commits November 22, 2024 22:06

Add static cropping

d6054d2

add newline after license

7d0ba3c

prettier

ffc5645

mcm001 force-pushed the cropping-mechanism branch from aa2250f to 8807195 Compare November 23, 2024 06:11

working cropping with fixed WPI tag detection

8b58b2f

mcm001 force-pushed the cropping-mechanism branch from 8807195 to 8b58b2f Compare November 23, 2024 06:13

mcm001 reviewed Nov 23, 2024

View reviewed changes

GGreenix added 5 commits November 23, 2024 20:03

Merge branch 'cropping-mechanism' of https://github.com/GGreenix/phot…

43aa66d

…onvision into cropping-mechanism

real resolution presented in UI

70da7ae

Added CropPipeParams object and all depending pipelines updated accordingly

Merge branch 'master' into cropping-mechanism

1caa573

test

17dcc4b

im tired.....

30c203c

Juniormunk reviewed Nov 25, 2024

View reviewed changes

crschardt reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cropping mechanism #1602

Cropping mechanism #1602

GGreenix commented Nov 23, 2024

mcm001 Nov 23, 2024

GGreenix Nov 23, 2024

mcm001 commented Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

mcm001 Nov 23, 2024

Juniormunk left a comment

Juniormunk Nov 25, 2024

Juniormunk Nov 25, 2024

crschardt Nov 25, 2024

crschardt Nov 25, 2024

crschardt Nov 25, 2024

crschardt Nov 25, 2024

		this.divisor = divisor;
		this.drawAllSnapshots = drawSnapshots;

Cropping mechanism #1602

Are you sure you want to change the base?

Cropping mechanism #1602

Conversation

GGreenix commented Nov 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcm001 commented Nov 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Juniormunk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment