From 14cb79e98ff9f5417e585249f1eb6d6fea963b4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 09:43:03 +0000 Subject: [PATCH] Fix multiple crash and instability issues across the codebase - Fix use-after-free in TracksDirectIORunnable::run() (deleted pointers used after delete) - Fix logic bug in PhongImageViewer comparing variable to itself instead of other - Add null checks for _sfmData in MSfMData::getUrlFromViewId() and getViewsIds() - Guard against division by zero in FloatImageViewer::updatePaintNode() - Fix signed integer overflow (UB) in ImageCache::cleanup() - Add null check in FloatTexture::isValid() and setImage() - Initialize _idView in Surface.hpp to prevent undefined behavior - Replace throw in Q_INVOKABLE MViewStats functions with qWarning + return - Add exception handling in QtAliceVisionImageIOHandler::read() - Fix cache size unit error in SequenceCache::getRamInfo() (TB -> GB) Agent-Logs-Url: https://github.com/alicevision/QtAliceVision/sessions/4795f35c-99b2-4e57-ab7d-3a42d0b3f9ea Co-authored-by: fabiencastan <153585+fabiencastan@users.noreply.github.com> --- src/qtAliceVision/FloatImageViewer.cpp | 19 +++++++++++-------- src/qtAliceVision/FloatTexture.cpp | 11 +++++++++-- src/qtAliceVision/ImageCache.cpp | 4 +++- src/qtAliceVision/MSfMData.cpp | 9 +++++++++ src/qtAliceVision/MTracks.cpp | 12 ++++-------- src/qtAliceVision/MViewStats.cpp | 18 ++++++++++++------ src/qtAliceVision/PhongImageViewer.cpp | 2 +- src/qtAliceVision/SequenceCache.cpp | 2 +- src/qtAliceVision/Surface.hpp | 2 +- .../QtAliceVisionImageIOHandler.cpp | 10 +++++++++- 10 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/qtAliceVision/FloatImageViewer.cpp b/src/qtAliceVision/FloatImageViewer.cpp index 2d3c459a..44f28899 100644 --- a/src/qtAliceVision/FloatImageViewer.cpp +++ b/src/qtAliceVision/FloatImageViewer.cpp @@ -589,16 +589,19 @@ QSGNode* FloatImageViewer::updatePaintNode(QSGNode* oldNode, [[maybe_unused]] QQ { _boundingRect = newBoundingRect; - const float windowRatio = static_cast(_boundingRect.width()) / static_cast(_boundingRect.height()); - const float textureRatio = static_cast(_textureSize.width()) / static_cast(_textureSize.height()); QRectF geometryRect = _boundingRect; - if (windowRatio > textureRatio) + if (_boundingRect.height() > 0 && _textureSize.height() > 0) { - geometryRect.setWidth(geometryRect.height() * textureRatio); - } - else - { - geometryRect.setHeight(geometryRect.width() / textureRatio); + const float windowRatio = static_cast(_boundingRect.width()) / static_cast(_boundingRect.height()); + const float textureRatio = static_cast(_textureSize.width()) / static_cast(_textureSize.height()); + if (windowRatio > textureRatio) + { + geometryRect.setWidth(geometryRect.height() * textureRatio); + } + else + { + geometryRect.setHeight(geometryRect.width() / textureRatio); + } } geometryRect.moveCenter(_boundingRect.center()); diff --git a/src/qtAliceVision/FloatTexture.cpp b/src/qtAliceVision/FloatTexture.cpp index 337a5e71..e8842488 100644 --- a/src/qtAliceVision/FloatTexture.cpp +++ b/src/qtAliceVision/FloatTexture.cpp @@ -24,12 +24,19 @@ FloatTexture::~FloatTexture() void FloatTexture::setImage(std::shared_ptr& image) { _srcImage = image; - _textureSize = {_srcImage->width(), _srcImage->height()}; + if (_srcImage) + { + _textureSize = {_srcImage->width(), _srcImage->height()}; + } + else + { + _textureSize = {}; + } _dirty = true; _mipmapsGenerated = false; } -bool FloatTexture::isValid() const { return _srcImage->width() != 0 && _srcImage->height() != 0; } +bool FloatTexture::isValid() const { return _srcImage && _srcImage->width() != 0 && _srcImage->height() != 0; } qint64 FloatTexture::comparisonKey() const { return _rhiTexture ? _rhiTexture->nativeTexture().object : 0; } diff --git a/src/qtAliceVision/ImageCache.cpp b/src/qtAliceVision/ImageCache.cpp index 45e870c7..7ab84321 100644 --- a/src/qtAliceVision/ImageCache.cpp +++ b/src/qtAliceVision/ImageCache.cpp @@ -68,7 +68,9 @@ void ImageCache::cleanup(size_t requestedSize, const CacheKey& toAdd) // After the frameId, the largest the difference, the highest its priority to delete if (diff < 0) { - diff = std::numeric_limits::max() + diff; + // Wrap negative differences to sort them after positive ones. + // Use unsigned arithmetic to avoid signed integer overflow (UB). + diff = static_cast(static_cast(std::numeric_limits::max()) + static_cast(diff)); } orderedKeys[diff] = &key; diff --git a/src/qtAliceVision/MSfMData.cpp b/src/qtAliceVision/MSfMData.cpp index 035a1563..18a15d13 100644 --- a/src/qtAliceVision/MSfMData.cpp +++ b/src/qtAliceVision/MSfMData.cpp @@ -71,6 +71,11 @@ void MSfMData::load() QString MSfMData::getUrlFromViewId(int viewId) { + if (!_sfmData) + { + qWarning() << "[QtAliceVision] MSfMData::getUrlFromViewId: no SfMData loaded."; + return {}; + } return QString::fromUtf8(_sfmData->getView(aliceVision::IndexT(viewId)).getImage().getImagePath().c_str()); } @@ -119,6 +124,10 @@ size_t MSfMData::nbCameras() const QVariantList MSfMData::getViewsIds() const { QVariantList viewsIds; + if (!_sfmData || _status != Ready) + { + return viewsIds; + } for (const auto& id : _sfmData->getValidViews()) { viewsIds.append(id); diff --git a/src/qtAliceVision/MTracks.cpp b/src/qtAliceVision/MTracks.cpp index b5f1ee53..7a96b56e 100644 --- a/src/qtAliceVision/MTracks.cpp +++ b/src/qtAliceVision/MTracks.cpp @@ -50,15 +50,11 @@ void TracksDirectIORunnable::run() if (!aliceVision::track::loadTracks(*tracks, _filename)) { - if (tracks) - { - delete tracks; - } + delete tracks; + delete tracksPerView; - if (tracksPerView) - { - delete tracksPerView; - } + Q_EMIT resultReady(nullptr, nullptr); + return; } aliceVision::track::computeTracksPerView(*tracks, *tracksPerView); diff --git a/src/qtAliceVision/MViewStats.cpp b/src/qtAliceVision/MViewStats.cpp index 8cdeaf1b..4decc240 100644 --- a/src/qtAliceVision/MViewStats.cpp +++ b/src/qtAliceVision/MViewStats.cpp @@ -34,7 +34,8 @@ void MViewStats::fillResidualFullSerie(QXYSeries* residuals) if (residualHistX.size() != residualHistY.size()) { - throw std::runtime_error("MViewStats::fillResidualFullSerie: residualHistX & residualHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillResidualFullSerie: residualHistX & residualHistY size mismatch."; + return; } QPen pen(Qt::red, 1, Qt::DashLine, Qt::FlatCap, Qt::BevelJoin); @@ -69,7 +70,8 @@ void MViewStats::fillResidualViewSerie(QXYSeries* residuals) std::vector residualHistY = _residualHistogramView.GetHist(); if (residualHistX.size() != residualHistY.size()) { - throw std::runtime_error("MViewStats::fillResidualViewSerie: residualHistX & residualHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillResidualViewSerie: residualHistX & residualHistY size mismatch."; + return; } QPen pen(Qt::darkBlue, 3, Qt::SolidLine, Qt::FlatCap, Qt::BevelJoin); @@ -104,7 +106,8 @@ void MViewStats::fillObservationsLengthsFullSerie(QXYSeries* observationsLengths std::vector observationsLengthsHistY = _observationsLengthsHistogramFull.GetHist(); if (observationsLengthsHistX.size() != observationsLengthsHistY.size()) { - throw std::runtime_error("MViewStats::fillObservationsLengthsFullSerie: observationsLengthsHistX & observationsLengthsHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillObservationsLengthsFullSerie: observationsLengthsHistX & observationsLengthsHistY size mismatch."; + return; } QPen pen(Qt::red, 1, Qt::DashLine, Qt::FlatCap, Qt::BevelJoin); @@ -139,7 +142,8 @@ void MViewStats::fillObservationsLengthsViewSerie(QXYSeries* observationsLengths std::vector observationsLengthsHistY = _observationsLengthsHistogramView.GetHist(); if (observationsLengthsHistX.size() != observationsLengthsHistY.size()) { - throw std::runtime_error("MViewStats::fillObservationsLengthsViewSerie: observationsLengthsHistX & observationsLengthsHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillObservationsLengthsViewSerie: observationsLengthsHistX & observationsLengthsHistY size mismatch."; + return; } QPen pen(Qt::darkBlue, 3, Qt::SolidLine, Qt::FlatCap, Qt::BevelJoin); @@ -174,7 +178,8 @@ void MViewStats::fillObservationsScaleFullSerie(QXYSeries* observationsScale) std::vector observationsScaleHistY = _observationsScaleHistogramFull.GetHist(); if (observationsScaleHistX.size() != observationsScaleHistY.size()) { - throw std::runtime_error("MViewStats::fillObservationsScaleFullSerie: observationsScaleHistX & observationsScaleHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillObservationsScaleFullSerie: observationsScaleHistX & observationsScaleHistY size mismatch."; + return; } QPen pen(Qt::red, 1, Qt::DashLine, Qt::FlatCap, Qt::BevelJoin); @@ -209,7 +214,8 @@ void MViewStats::fillObservationsScaleViewSerie(QXYSeries* observationsScale) std::vector observationsScaleHistY = _observationsScaleHistogramView.GetHist(); if (observationsScaleHistX.size() != observationsScaleHistY.size()) { - throw std::runtime_error("MViewStats::fillObservationsScaleViewSerie: observationsScaleHistX & observationsScaleHistY size mismatch."); + qWarning() << "[QtAliceVision] MViewStats::fillObservationsScaleViewSerie: observationsScaleHistX & observationsScaleHistY size mismatch."; + return; } QPen pen(Qt::darkBlue, 3, Qt::SolidLine, Qt::FlatCap, Qt::BevelJoin); diff --git a/src/qtAliceVision/PhongImageViewer.cpp b/src/qtAliceVision/PhongImageViewer.cpp index e2dde5b2..edbc96ba 100644 --- a/src/qtAliceVision/PhongImageViewer.cpp +++ b/src/qtAliceVision/PhongImageViewer.cpp @@ -267,7 +267,7 @@ void PhongImageViewer::reload() } // check source and normal images dimensions - if (responseSourceImage.dim != responseSourceImage.dim) + if (responseSourceImage.dim != responseNormalImage.dim) { clearImages(); setStatus(EStatus::LOADING_ERROR); diff --git a/src/qtAliceVision/SequenceCache.cpp b/src/qtAliceVision/SequenceCache.cpp index 7f30722f..d2409fc4 100644 --- a/src/qtAliceVision/SequenceCache.cpp +++ b/src/qtAliceVision/SequenceCache.cpp @@ -109,7 +109,7 @@ QPointF SequenceCache::getRamInfo() const const auto memInfo = aliceVision::system::getMemoryInfo(); double availableRam = static_cast(memInfo.availableRam) / (1024. * 1024. * 1024.); - double contentSize = static_cast(_fetcher.getCacheSize()) / (1024. * 1024. * 1024. * 1024.); + double contentSize = static_cast(_fetcher.getCacheSize()) / (1024. * 1024. * 1024.); // Return in GB return QPointF(availableRam, contentSize); diff --git a/src/qtAliceVision/Surface.hpp b/src/qtAliceVision/Surface.hpp index 197caefb..13f4f646 100644 --- a/src/qtAliceVision/Surface.hpp +++ b/src/qtAliceVision/Surface.hpp @@ -202,7 +202,7 @@ class Surface : public QObject bool _needToUseIntrinsic = true; // Id View - aliceVision::IndexT _idView; + aliceVision::IndexT _idView = aliceVision::UndefinedIndexT; // Viewer EViewerType _viewerType = EViewerType::DEFAULT; diff --git a/src/qtAliceVisionImageIO/QtAliceVisionImageIOHandler.cpp b/src/qtAliceVisionImageIO/QtAliceVisionImageIOHandler.cpp index 388ee4d9..1dcb2005 100644 --- a/src/qtAliceVisionImageIO/QtAliceVisionImageIOHandler.cpp +++ b/src/qtAliceVisionImageIO/QtAliceVisionImageIOHandler.cpp @@ -79,7 +79,15 @@ bool QtAliceVisionImageIOHandler::read(QImage* image) qDebug() << "[QtAliceVisionImageIO] Read image: " << path.c_str(); aliceVision::image::Image img; - aliceVision::image::readImage(path, img, aliceVision::image::EImageColorSpace::SRGB); + try + { + aliceVision::image::readImage(path, img, aliceVision::image::EImageColorSpace::SRGB); + } + catch (const std::exception& e) + { + qWarning() << "[QtAliceVisionImageIO] Failed to read image:" << e.what(); + return false; + } oiio::ImageBuf inBuf; aliceVision::image::getBufferFromImage(img, inBuf);