From 0f662d643ca3c2d4becbc59008de197e806ba444 Mon Sep 17 00:00:00 2001 From: Filyus Date: Sun, 4 Oct 2026 13:48:41 +0500 Subject: [PATCH] Make wrap decoding exact when the sum leaves int32 PredictionSchemeWrapDecodingTransform::ComputeOriginalValue() adds the predicted value and the correction in uint32_t, to avoid signed overflow on malformed input, and then wraps once by the span max_dif. The encoder wraps each correction into [min_correction, max_correction], half the span, so the exact sum is at most one span outside [min, max] and one wrap is enough, but only if the sum is exact. When min and max are far apart the uint32_t sum itself wraps, the single wrap lands 2^32 away from the coded value, and every later prediction reads the wrong number. The decoder then either rejects the stream or decodes different values, for a stream its own encoder wrote. Doing the addition in int64_t keeps it exact for valid streams and still free of overflow for malformed ones. Sums that fit in int32 decode exactly as before. TestTexCoordPredictionThatWrapsRoundTrips builds six points with uint16 positions under constrained multi-parallelogram prediction and int32 texture coordinates under portable prediction, encodes them with EdgeBreaker at speeds 0/4 into a 911-byte stream and decodes it. Before this change the decode fails with "Failed to decode point attributes". After it, every decoded point carries a position and texture coordinate an input point carried. The mesh was found by fuzzing an independent implementation of the bitstream, which decodes the same stream. --- ...rediction_scheme_wrap_decoding_transform.h | 23 +++-- src/draco/compression/encode_test.cc | 83 +++++++++++++++++++ 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/src/draco/compression/attributes/prediction_schemes/prediction_scheme_wrap_decoding_transform.h b/src/draco/compression/attributes/prediction_schemes/prediction_scheme_wrap_decoding_transform.h index e100c738a..0349c5fc8 100644 --- a/src/draco/compression/attributes/prediction_schemes/prediction_scheme_wrap_decoding_transform.h +++ b/src/draco/compression/attributes/prediction_schemes/prediction_scheme_wrap_decoding_transform.h @@ -46,20 +46,19 @@ class PredictionSchemeWrapDecodingTransform predicted_vals = this->ClampPredictedValue(predicted_vals); - // Perform the wrapping using unsigned coordinates to avoid potential signed - // integer overflows caused by malformed input. - const uint32_t *const uint_predicted_vals = - reinterpret_cast(predicted_vals); - const uint32_t *const uint_corr_vals = - reinterpret_cast(corr_vals); + // The sum is computed in int64_t, where it is exact. The encoder keeps each + // correction within half of the span, so the sum lies at most one span + // outside [min, max] and a single wrap gives the original value. Malformed + // corrections cannot overflow int64_t either. for (int i = 0; i < this->num_components(); ++i) { - out_original_vals[i] = - static_cast(uint_predicted_vals[i] + uint_corr_vals[i]); - if (out_original_vals[i] > this->max_value()) { - out_original_vals[i] -= this->max_dif(); - } else if (out_original_vals[i] < this->min_value()) { - out_original_vals[i] += this->max_dif(); + int64_t value = static_cast(predicted_vals[i]) + + static_cast(corr_vals[i]); + if (value > this->max_value()) { + value -= this->max_dif(); + } else if (value < this->min_value()) { + value += this->max_dif(); } + out_original_vals[i] = static_cast(value); } } diff --git a/src/draco/compression/encode_test.cc b/src/draco/compression/encode_test.cc index 461dddef7..afb0c4f26 100644 --- a/src/draco/compression/encode_test.cc +++ b/src/draco/compression/encode_test.cc @@ -15,6 +15,7 @@ #include "draco/compression/encode.h" +#include #include #include #include @@ -637,4 +638,86 @@ TEST_F(EncodeTest, TestDracoCompressionOptionsGridQuantizationWithOffset) { } #endif // DRACO_TRANSCODER_SUPPORTED +// Six points with uint16 positions under constrained multi-parallelogram +// prediction and int32 texture coordinates under portable prediction, encoded +// with EdgeBreaker at speeds 0/4. During decoding a prediction plus its +// correction exceeds the int32 range, and the stream must still decode. +TEST_F(EncodeTest, TestTexCoordPredictionThatWrapsRoundTrips) { + // Positions and texture coordinates in point order. + const uint16_t positions[6][3] = {{32511, 13823, 13312}, {53, 308, 63587}, + {51712, 36676, 65535}, {308, 63523, 65280}, + {25601, 30005, 46389}, {308, 63523, 128}}; + const int32_t tex_coords[6][2] = {{901068149, 1661023232}, {16261889, -1}, + {3538814, 895746356}, {901071249, 67108864}, + {587281656, 872448248}, {-518656, 520036351}}; + const int faces[45][3] = { + {0, 1, 2}, {2, 3, 3}, {4, 2, 4}, {2, 5, 1}, {3, 0, 1}, {2, 2, 3}, + {3, 5, 2}, {4, 2, 5}, {5, 1, 4}, {5, 1, 4}, {1, 1, 5}, {4, 0, 1}, + {1, 2, 0}, {1, 3, 5}, {1, 0, 4}, {1, 2, 3}, {2, 4, 5}, {2, 1, 4}, + {2, 5, 1}, {0, 1, 2}, {0, 0, 2}, {1, 3, 1}, {2, 3, 1}, {4, 3, 1}, + {5, 3, 2}, {1, 4, 5}, {1, 4, 0}, {1, 5, 4}, {5, 1, 2}, {1, 0, 2}, + {1, 5, 5}, {0, 0, 1}, {1, 1, 0}, {0, 2, 3}, {1, 1, 3}, {5, 3, 3}, + {1, 3, 4}, {3, 4, 0}, {4, 4, 0}, {2, 1, 1}, {2, 1, 0}, {5, 1, 3}, + {2, 5, 4}, {5, 3, 3}, {2, 1, 3}}; + draco::Mesh mesh; + mesh.set_num_points(6); + draco::GeometryAttribute pos; + pos.Init(draco::GeometryAttribute::POSITION, nullptr, 3, draco::DT_UINT16, + true, sizeof(uint16_t) * 3, 0); + const int pos_id = mesh.AddAttribute(pos, true, 6); + draco::GeometryAttribute tex; + tex.Init(draco::GeometryAttribute::TEX_COORD, nullptr, 2, draco::DT_INT32, + false, sizeof(int32_t) * 2, 0); + const int tex_id = mesh.AddAttribute(tex, true, 6); + for (int i = 0; i < 6; ++i) { + mesh.attribute(pos_id)->SetAttributeValue(draco::AttributeValueIndex(i), + positions[i]); + mesh.attribute(tex_id)->SetAttributeValue(draco::AttributeValueIndex(i), + tex_coords[i]); + } + for (const auto &f : faces) { + mesh.AddFace({draco::PointIndex(f[0]), draco::PointIndex(f[1]), + draco::PointIndex(f[2])}); + } + + draco::ExpertEncoder encoder(mesh); + encoder.SetSpeedOptions(0, 4); + encoder.SetEncodingMethod(draco::MESH_EDGEBREAKER_ENCODING); + DRACO_ASSERT_OK(encoder.SetAttributePredictionScheme( + pos_id, draco::MESH_PREDICTION_CONSTRAINED_MULTI_PARALLELOGRAM)); + DRACO_ASSERT_OK(encoder.SetAttributePredictionScheme( + tex_id, draco::MESH_PREDICTION_TEX_COORDS_PORTABLE)); + draco::EncoderBuffer buffer; + DRACO_ASSERT_OK(encoder.EncodeToBuffer(&buffer)); + + draco::DecoderBuffer in; + in.Init(buffer.data(), buffer.size()); + draco::Decoder decoder; + auto decoded = decoder.DecodeMeshFromBuffer(&in); + ASSERT_TRUE(decoded.ok()) << decoded.status().error_msg_string(); + + // EdgeBreaker reorders the points. Every decoded point must carry a position + // and a texture coordinate that some input point carried together. + const draco::Mesh &out = *decoded.value(); + const draco::PointAttribute *const out_pos = + out.GetNamedAttribute(draco::GeometryAttribute::POSITION); + const draco::PointAttribute *const out_tex = + out.GetNamedAttribute(draco::GeometryAttribute::TEX_COORD); + ASSERT_NE(out_pos, nullptr); + ASSERT_NE(out_tex, nullptr); + for (draco::PointIndex p(0); p < out.num_points(); ++p) { + uint16_t position[3]; + int32_t tex_coord[2]; + out_pos->GetMappedValue(p, position); + out_tex->GetMappedValue(p, tex_coord); + bool found = false; + for (int i = 0; i < 6 && !found; ++i) { + found = std::equal(position, position + 3, positions[i]) && + std::equal(tex_coord, tex_coord + 2, tex_coords[i]); + } + EXPECT_TRUE(found) << "point " << p.value() << " decoded to a value no " + << "input point carried"; + } +} + } // namespace