Refactor camera intrinsic parameter handling.

Move logic from parser to compiler.

PiperOrigin-RevId: 847689906
Change-Id: I844746081973205e1b33a3845add842e8b8c3c07

doc/XMLreference.rst

@@ -2976,6 +2976,28 @@ and the +Y axis points up. Thus the frame position and orientation are the key a
    for most scenes and should likely be reduced. In either case, the horizontal field of view is computed automatically
    given the window size and the vertical field of view.
 
+.. _body-camera-sensorsize:
+
+:at:`sensorsize`: :at-val:`real(2), "0 0"`
+   Size of the camera sensor in length units. When specified, all intrinsic attributes become active and :at:`fovy`
+   is ignored. The field-of-view is then computed automatically from the focal length and sensor size. This
+   also enables frustum visualization when the :ref:`mjVIS_CAMERA<mjtVisFlag>` visualization flag is active.
+
+.. _body-camera-focal:
+.. _body-camera-focalpixel:
+
+:at:`focal` / :at:`focalpixel`: :at-val:`real(2), "0 0"`
+   Focal length in physical length units or in pixels, respectively. If both are specified, the pixel
+   value is used and the length value is ignored.
+
+.. _body-camera-principal:
+.. _body-camera-principalpixel:
+
+:at:`principal` / :at:`principalpixel`: :at-val:`real(2), "0 0"`
+   Offset of the principal point (optical axis intersection with the image plane) from the image center. If both are
+   specified, the pixel value is used. At zero offset, the rendered image is centered on the camera's negative Z axis,
+   as in a standard pinhole camera model.
+
 .. _body-camera-resolution:
 
 :at:`resolution`: :at-val:`int(2), "1 1"`
@@ -2983,36 +3005,6 @@ and the +Y axis points up. Thus the frame position and orientation are the key a
    dimensions are determined by the size of the rendering context. This attribute serves as a convenient
    location to save the required resolution when creating a context.
 
-.. _body-camera-focal:
-
-:at:`focal`: :at-val:`real(2), "0 0"`
-   Focal length of the camera in length units. It is mutually exclusive with :ref:`fovy <body-camera-fovy>`.
-   See :ref:`CCamera` for details.
-
-.. _body-camera-focalpixel:
-
-:at:`focalpixel`: :at-val:`int(2), "0 0"`
-   Focal length of the camera in pixel units. If both :at:`focal` and :at:`focalpixel` are specified, the former is
-   ignored.
-
-.. _body-camera-principal:
-
-:at:`principal`: :at-val:`real(2), "0 0"`
-   Offset of the principal point of the camera with respect to the camera center in length units. It is mutually
-   exclusive with :ref:`fovy <body-camera-fovy>`.
-
-.. _body-camera-principalpixel:
-
-:at:`principalpixel`: :at-val:`real(2), "0 0"`
-   Offset of the principal point of the camera with respect to the camera center in pixel units. If both
-   :at:`principal` and :at:`principalpixel` are specified, the former is ignored.
-
-.. _body-camera-sensorsize:
-
-:at:`sensorsize`: :at-val:`real(2), "0 0"`
-   Size of the camera sensor in length units. It is mutually exclusive with :ref:`fovy <body-camera-fovy>`. If
-   specified, :ref:`resolution <body-camera-resolution>` and :ref:`focal <body-camera-focal>` are required.
-
 .. _body-camera-ipd:
 
 :at:`ipd`: :at-val:`real, "0.068"`

src/user/user_objects.cc

@@ -4116,15 +4116,23 @@ void mjCCamera::Compile(void) {
                    name.c_str(), id, fovy);
   }
 
-  // check that specs are not duplicated
+  // check for advanced camera intrinsic parameters
-  if ((principal_length[0] && principal_pixel[0]) ||
+  bool has_intrinsic = focal_length[0]     || focal_length[1]     ||
-      (principal_length[1] && principal_pixel[1])) {
+                       focal_pixel[0]      || focal_pixel[1]      ||
-    throw mjCError(this, "principal length duplicated in camera");
+                       principal_length[0] || principal_length[1] ||
+                       principal_pixel[0]  || principal_pixel[1];
+  bool has_sensorsize = sensor_size[0] > 0 && sensor_size[1] > 0;
+
+  // intrinsic params require sensorsize
+  if (has_intrinsic && !has_sensorsize) {
+    throw mjCError(this, "focal/principal require sensorsize in camera '%s' (id = %d)",
+                   name.c_str(), id);
   }
 
-  if ((focal_length[0] && focal_pixel[0]) ||
+  // sensorsize requires resolution
-      (focal_length[1] && focal_pixel[1])) {
+  if (has_sensorsize && (resolution[0] <= 0 || resolution[1] <= 0)) {
-    throw mjCError(this, "focal length duplicated in camera");
+    throw mjCError(this, "sensorsize requires positive resolution in camera '%s' (id = %d)",
+                   name.c_str(), id);
   }
 
   // compute number of pixels per unit length
@@ -4134,11 +4142,11 @@ void mjCCamera::Compile(void) {
       (float)resolution[1] / sensor_size[1],
     };
 
-    // defaults are zero, so only one term in each sum is nonzero
+    // pixel values override length values when both are specified
-    intrinsic[0] = focal_pixel[0] / pixel_density[0] + focal_length[0];
+    intrinsic[0] = focal_pixel[0] ? focal_pixel[0] / pixel_density[0] : focal_length[0];
-    intrinsic[1] = focal_pixel[1] / pixel_density[1] + focal_length[1];
+    intrinsic[1] = focal_pixel[1] ? focal_pixel[1] / pixel_density[1] : focal_length[1];
-    intrinsic[2] = principal_pixel[0] / pixel_density[0] + principal_length[0];
+    intrinsic[2] = principal_pixel[0] ? principal_pixel[0] / pixel_density[0] : principal_length[0];
-    intrinsic[3] = principal_pixel[1] / pixel_density[1] + principal_length[1];
+    intrinsic[3] = principal_pixel[1] ? principal_pixel[1] / pixel_density[1] : principal_length[1];
 
     // fovy with principal point at (0, 0)
     fovy = std::atan2(sensor_size[1]/2, intrinsic[1]) * 360.0 / mjPI;

src/xml/xml_native_reader.cc

@@ -1937,24 +1937,15 @@ void mjXReader::OneCamera(XMLElement* elem, mjsCamera* camera) {
     camera->proj = (mjtProjection)n;
   }
 
-  bool has_principal = ReadAttr(elem, "principalpixel", 2, camera->principal_pixel, text) ||
+  ReadAttr(elem, "principalpixel", 2, camera->principal_pixel, text);
-                       ReadAttr(elem, "principal", 2, camera->principal_length, text);
+  ReadAttr(elem, "principal", 2, camera->principal_length, text);
-  bool has_focal = ReadAttr(elem, "focalpixel", 2, camera->focal_pixel, text) ||
+  ReadAttr(elem, "focalpixel", 2, camera->focal_pixel, text);
-                   ReadAttr(elem, "focal", 2, camera->focal_length, text);
+  ReadAttr(elem, "focal", 2, camera->focal_length, text);
-  bool needs_sensorsize = has_principal || has_focal;
+  ReadAttr(elem, "resolution", 2, camera->resolution, text);
-  bool has_sensorsize = ReadAttr(elem, "sensorsize", 2, camera->sensor_size, text, needs_sensorsize);
+  bool sensorsize = ReadAttr(elem, "sensorsize", 2, camera->sensor_size, text);
-  bool has_fovy = ReadAttr(elem, "fovy", 1, &camera->fovy, text);
+  bool fovy = ReadAttr(elem, "fovy", 1, &camera->fovy, text);
-  bool needs_resolution = has_focal || has_sensorsize;
+  if (fovy && sensorsize) {
-  ReadAttr(elem, "resolution", 2, camera->resolution, text, needs_resolution);
+    throw mjXError(elem, "either 'fovy' or 'sensorsize' attribute can be specified, not both");
-
-  if (camera->resolution[0] < 0 || camera->resolution[1] < 0) {
-    throw mjXError(elem, "camera resolution cannot be negative");
-  }
-
-  if (has_fovy && has_sensorsize) {
-    throw mjXError(
-            elem,
-            "either 'fovy' or 'sensorsize' attribute can be specified, not both");
   }
 
   // read userdata

test/xml/xml_native_reader_test.cc

@@ -2001,7 +2001,7 @@ TEST_F(XMLReaderTest, CameraPrincipalRequiresSensorsize) {
   std::array<char, 1024> error;
   mjModel* m = LoadModelFromString(xml, error.data(), error.size());
   EXPECT_THAT(m, IsNull());
-  EXPECT_THAT(error.data(), HasSubstr("attribute missing: 'sensorsize'"));
+  EXPECT_THAT(error.data(), HasSubstr("focal/principal require sensorsize"));
   EXPECT_THAT(error.data(), HasSubstr("line 6"));
 }
 
@@ -2011,7 +2011,7 @@ TEST_F(XMLReaderTest, CameraSensorsizeRequiresResolution) {
     <worldbody>
       <body>
         <geom size="1"/>
-        <camera sensorsize="1 1"/>
+        <camera sensorsize="1 1" resolution="0 0"/>
       </body>
     </worldbody>
   </mujoco>
@@ -2019,7 +2019,7 @@ TEST_F(XMLReaderTest, CameraSensorsizeRequiresResolution) {
   std::array<char, 1024> error;
   mjModel* m = LoadModelFromString(xml, error.data(), error.size());
   EXPECT_THAT(m, IsNull());
-  EXPECT_THAT(error.data(), HasSubstr("attribute missing: 'resolution'"));
+  EXPECT_THAT(error.data(), HasSubstr("requires positive resolution"));
   EXPECT_THAT(error.data(), HasSubstr("line 6"));
 }