Skip to content

ChArUco calibration board generation#2495

Open
thatcomputerguy0101 wants to merge 11 commits into
PhotonVision:mainfrom
thatcomputerguy0101:ChArUco-generation
Open

ChArUco calibration board generation#2495
thatcomputerguy0101 wants to merge 11 commits into
PhotonVision:mainfrom
thatcomputerguy0101:ChArUco-generation

Conversation

@thatcomputerguy0101

@thatcomputerguy0101 thatcomputerguy0101 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Description

This adds support for generating custom ChArUco boards to the calibration page, and drops support for generating Checkerboard boards. Similar to Checkerboard, ChArUco will pull the parameters set for calibration detection and generate the matching board. Additionally, the output files are fully vector based, creating sharper corners than the previous image-based approach. The ruler for has been updated with 20mm marks in addition to inch marks. Currently, this uses my own fork of aruco-marker, but I did also create a PR upstream at bhollis/aruco-marker#8.

Old ChArUco:
calibrationTarget-Charuco-old
New ChArUco (inches):
calibrationTarget-ChArUco-new-inches
New ChArUco (millimeters):
calibrationTarget-ChArUco-new-metric\

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why, including events that led to this PR
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with all settings going back to the previous seasons's last release (seasons end after champs ends)
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added
  • If this PR adds a dependency, the license has been checked for compatibility and steps taken to follow it

@thatcomputerguy0101 thatcomputerguy0101 requested a review from a team as a code owner May 16, 2026 15:03
@github-actions github-actions Bot added the frontend Having to do with PhotonClient and its related items label May 16, 2026
@samfreund

Copy link
Copy Markdown
Member

I know we don't want to remove checkerboard support at this time, but I think we should remove the ability to generate them. This should hopefully encourage teams to use charuco.

@thatcomputerguy0101

Copy link
Copy Markdown
Contributor Author

Currently, this only supports 8.5"x11" paper, but we could probably easily add a dropdown for common paper sizes.

@auscompgeek

Copy link
Copy Markdown
Contributor

Currently, this only supports 8.5"x11" paper, but we could probably easily add a dropdown for common paper sizes.

We definitely at least want an option for A4 at some point. Even I struggle to find the option to print at 100% scale sometimes.

@samfreund

Copy link
Copy Markdown
Member

Currently, this only supports 8.5"x11" paper, but we could probably easily add a dropdown for common paper sizes.

Let's include that here if you can and drop chessboard too.

Comment thread photon-client/src/lib/PhotonUtils.ts Outdated
@thatcomputerguy0101

Copy link
Copy Markdown
Contributor Author

safe-units adds about 24KB to the bundle over doing the calculations by hand, which seems worth it for the flexibility it provides.

@thatcomputerguy0101

Copy link
Copy Markdown
Contributor Author

I'm a little uncertain of the checkerboard parity used for boards with an even number of rows. What I did matches calib.io with the bottom-left square always being black, but it seems like (current) OpenCV and https://calibrate.deepen.ai/target-generator has the top-left square always being black.

@thatcomputerguy0101

Copy link
Copy Markdown
Contributor Author

units-and-measurement only adds about 2.2KB to the bundle.

"@fontsource/prompt": "^5.2.6",
"@mdi/font": "^7.4.47",
"@msgpack/msgpack": "^3.1.2",
"aruco-marker": "github:thatcomputerguy0101/aruco-marker#update-and-dictionaries",

@GrahamSH-LLK GrahamSH-LLK May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should either

  • get published on NPM
  • refer to a commit
  • get upstreamed
    I see that you’ve sent a PR, but since the library seems relatively unmaintained the first two are probably fine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own curiosity, any particular reason that github: is discouraged?

@GrahamSH-LLK GrahamSH-LLK May 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own curiosity, any particular reason that github: is discouraged?

Not github: in general, it's more specifically that it refers to a branch and can change under us, which is why referring to a commit hash would be fine

@thatcomputerguy0101 thatcomputerguy0101 May 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The pnpm lock stores the hash from the branch at the time of install, so that reinstall retrieves the same commit. Update pulls the latest contents from the branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, is this package something that should be pulled in under the Photon org if the original maintainer doesn't get back to us?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't mind forking, sure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Ping me when/if we decide we need to do that)

export enum CalibrationBoardTypes {
Chessboard = 0,
Charuco = 1
ChArUco = 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this not need t o match

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think so? They didn't match before, so I'm assuming they are serialized by index. This was changed because the string form of the TS enum is used in generating the PDF.

Comment thread photon-client/src/components/cameras/CameraCalibrationCard.vue Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Having to do with PhotonClient and its related items

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants