Skip to content

Conversation

@cs-util
Copy link

@cs-util cs-util commented Dec 21, 2025

No description provided.

…tionality for setting reference distances and measuring
@gemini-code-assist
Copy link

Summary of Changes

Hello @cs-util, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces the foundational mathematical and calibration logic for a new 'Manual Reference Distance' feature. This feature will allow users to define a known distance on an image, enabling accurate measurements even without GPS data, and will integrate with existing GPS calibration by prioritizing manual input. The current changes complete the backend implementation, with UI/UX planned for a subsequent phase.

Highlights

  • New Feature: Manual Reference Distances: This pull request lays the groundwork for a new feature allowing users to define a known distance on an image, enabling accurate measurements even without GPS data. This is crucial for applications like floor plans or blueprints.
  • Core Math Implementation for Fixed Scale: A new fitSimilarityFixedScale function has been implemented in src/geo/transformations.js. This function performs Procrustes analysis to find optimal rotation and translation while adhering to a predefined scale, which is essential for integrating manual reference distances.
  • Calibration System Integration: The calibrateMap function in src/calibration/calibrator.js has been updated to accept an optional referenceScale. When provided for similarity models, this fixed scale takes precedence over GPS-derived scales, ensuring that manual measurements are considered 'ground truth'.
  • Extensive Testing: Comprehensive unit tests (13 new tests for fitSimilarityFixedScale) and integration tests (3 new tests for calibrateMap) have been added. These tests validate the accuracy and robustness of the fixed-scale transformation logic, including edge cases and a documented geometric symmetry property for 2-pair scenarios.
  • Future UI/UX Planning: The documentation includes a detailed plan for the user interface and experience, outlining 'Set Scale' and 'Measure' modes, scale source indicators, multiple measurements, and unit selection, which will be implemented in a subsequent phase.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements the backend mathematical components for a new feature that allows users to define a reference distance on a map. This includes adding a new fitSimilarityFixedScale function to compute a similarity transform with a fixed scale and integrating it into the calibration pipeline. The changes are well-implemented and accompanied by a very comprehensive suite of unit tests. I've also reviewed the detailed design document for the feature.

My feedback is mainly on the design document (docs/feat-reference-distances.md), where I've identified a couple of logical issues in the proposed implementation details for helper functions. The core mathematical changes in the code are solid and look great.

Comment on lines +155 to +164
function getMetersPerPixelFromCalibration(calibrationResult) {
if (!calibrationResult || !calibrationResult.matrix) return null;
const { a, b } = calibrationResult.matrix;
const geoUnitsPerPixel = Math.sqrt(a * a + b * b);
// Convert degrees to meters (approximate, using center latitude)
// 1 degree ≈ 111,320 meters at equator, adjusted by cos(lat)
const centerLat = calibrationResult.centerLat || 0;
const metersPerDegree = 111320 * Math.cos(centerLat * Math.PI / 180);
return geoUnitsPerPixel * metersPerDegree;
}

Choose a reason for hiding this comment

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

high

The proposed implementation of getMetersPerPixelFromCalibration seems to be based on a misunderstanding of the data structure returned by calibrateMap.

  1. It accesses calibrationResult.matrix, but calibrateMap returns a model object inside the result, so it should be calibrationResult.model.
  2. For a similarity transform, the model object contains a scale property directly. There is no matrix property. The function should just use model.scale.
  3. The code const { a, b } = calibrationResult.matrix; and Math.sqrt(a * a + b * b) seems to assume a similarity matrix format [a, b, tx; -b, a, ty], but this is not what fitSimilarity returns.
  4. For affine and homography transforms, the scale is not uniform across the image. A single scale value can be approximated by calculating the Jacobian at a reference point (e.g., image center) and using averageScaleFromJacobian, but this function as written does not account for this.

I recommend redesigning this function to correctly handle the different model types returned by calibrateMap. For example:

function getMetersPerPixelFromCalibration(calibrationResult) {
  if (!calibrationResult || !calibrationResult.model) return null;

  const { model, origin } = calibrationResult;
  let geoUnitsPerPixel;

  if (model.type === 'similarity') {
    geoUnitsPerPixel = model.scale;
  } else {
    // For non-uniform scales, we need a reference point.
    // The center of the image is a reasonable default, but image dimensions are not available here.
    // An alternative is to use the centroid of the pixel pairs, which would need to be passed in or stored.
    // For now, let's use {x: 0, y: 0} as a placeholder reference.
    const jacobian = jacobianForTransform(model, { x: 0, y: 0 });
    geoUnitsPerPixel = averageScaleFromJacobian(jacobian);
  }

  if (!geoUnitsPerPixel) return null;

  // The rest of the lat/lon to meters conversion is correct.
  const centerLat = origin?.lat || 0;
  const metersPerDegree = 111320 * Math.cos(centerLat * Math.PI / 180);
  return geoUnitsPerPixel * metersPerDegree;
}

This is a significant logic error in the design document that should be addressed before implementation.

Comment on lines +104 to +107
const totalInches = meters * METERS_TO_FEET * 12;
const feet = Math.floor(totalInches / 12);
const inches = Math.round(totalInches % 12);
return `${feet}' ${inches}"`;

Choose a reason for hiding this comment

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

medium

The logic for converting to feet and inches can lead to incorrect output, for example 5' 12". The issue is with Math.round(totalInches % 12). When totalInches is 71.6 (which is 5' 11.6"), totalInches % 12 is 11.6, and Math.round(11.6) is 12, resulting in 5' 12".

A more robust approach is to round the total number of inches first, and then calculate feet and inches from that.

Suggested change
const totalInches = meters * METERS_TO_FEET * 12;
const feet = Math.floor(totalInches / 12);
const inches = Math.round(totalInches % 12);
return `${feet}' ${inches}"`;
const totalInches = Math.round(meters * METERS_TO_FEET * 12);
const feet = Math.floor(totalInches / 12);
const inches = totalInches % 12;
return `${feet}' ${inches}"`;

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