From c064abc3da5fc796610a30ddc7273ad143436066 Mon Sep 17 00:00:00 2001 From: Yuval Tassa Date: Tue, 9 Dec 2025 07:01:16 -0800 Subject: [PATCH] Change row-flipping logic for heightfields. Move row-flipping logic from `mjCModel::CopyBack` to `mjXWriter`. PiperOrigin-RevId: 842220322 Change-Id: I6fb09db6c9e8689f6e435f82f58dd3e7c4314478 --- doc/XMLreference.rst | 3 ++- src/user/user_model.cc | 6 +----- src/xml/xml_native_writer.cc | 18 +++++++++++++++--- test/engine/testdata/ray/hfield.xml | 19 +++++++++++++++++++ test/xml/xml_native_writer_test.cc | 7 ++++++- 5 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 test/engine/testdata/ray/hfield.xml diff --git a/doc/XMLreference.rst b/doc/XMLreference.rst index 6831bb38..c436aeb9 100644 --- a/doc/XMLreference.rst +++ b/doc/XMLreference.rst @@ -1559,7 +1559,8 @@ also known as terrain map, is a 2D matrix of elevation data. The data can be spe :at:`elevation`: :at-val:`real(nrow*ncol), optional` This attribute specifies the elevation data matrix. Values are automatically normalized to lie between 0 and 1 by first subtracting the minimum value and then dividing by the (maximum-minimum) difference, if not 0. If not provided, - values are set to 0. + values are set to 0. Note that the row order of data in :ref:`mjModel` and :ref:`mjsHField` is flipped w.r.t. the + order in XML i.e., it is bottom-to-top. .. _asset-hfield-size: diff --git a/src/user/user_model.cc b/src/user/user_model.cc index ad7af6dd..06bcd073 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -5318,11 +5318,7 @@ bool mjCModel::CopyBack(const mjModel* m) { int ncol = m->hfield_ncol[i]; float* userdata = phf->get_userdata().data(); float* modeldata = m->hfield_data + m->hfield_adr[i]; - // copy back in reverse row order - for (int j=0; j < nrow; j++) { - int flip = nrow-1-j; - mjuu_copyvec(userdata + flip*ncol, modeldata+j*ncol, ncol); - } + memcpy(userdata, modeldata, nrow*ncol*sizeof(float)); } } diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index 848a7615..ae9e9da4 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -1585,11 +1585,23 @@ void mjXWriter::Asset(XMLElement* root) { WriteAttrTxt(elem, "content_type", hfield->content_type_); WriteAttrTxt(elem, "file", hfield->file_); } else { - WriteAttrInt(elem, "nrow", hfield->nrow); - WriteAttrInt(elem, "ncol", hfield->ncol); + int nrow = hfield->nrow; + int ncol = hfield->ncol; + WriteAttrInt(elem, "nrow", nrow); + WriteAttrInt(elem, "ncol", ncol); if (!hfield->get_userdata().empty()) { + // copy in reverse row order, so XML string is top-to-bottom + std::vector flipped(nrow * ncol); + const std::vector& userdata = hfield->get_userdata(); + for (int i = 0; i < nrow; i++) { + int flip = nrow - 1 - i; + for (int j = 0; j < ncol; j++) { + flipped[i * ncol + j] = userdata[flip * ncol + j]; + } + } + string text; - Vector2String(text, hfield->get_userdata(), hfield->ncol); + Vector2String(text, flipped, ncol); WriteAttrTxt(elem, "elevation", text); } } diff --git a/test/engine/testdata/ray/hfield.xml b/test/engine/testdata/ray/hfield.xml new file mode 100644 index 00000000..f702a197 --- /dev/null +++ b/test/engine/testdata/ray/hfield.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc index 48db4604..b521c84c 100644 --- a/test/xml/xml_native_writer_test.cc +++ b/test/xml/xml_native_writer_test.cc @@ -38,10 +38,11 @@ namespace mujoco { namespace { +using ::testing::ElementsAre; +using ::testing::FloatEq; using ::testing::HasSubstr; using ::testing::Not; using ::testing::NotNull; -using ::testing::FloatEq; using XMLWriterTest = PluginTest; @@ -998,6 +999,10 @@ TEST_F(XMLWriterTest, WritesHfield) { int size = model->hfield_nrow[0]*model->hfield_ncol[0]; EXPECT_EQ(size, 6); + // check that the data is normalized and in row-major, bottom-to-top order + EXPECT_THAT(AsVector(model->hfield_data, 6), + ElementsAre(.8, 1, .4, .6, 0, .2)); + // save and read, compare data mjModel* mtemp = LoadModelFromString(SaveAndReadXml(model)); ASSERT_THAT(mtemp, NotNull());