From 038e091af450383f42a275dab7642518d53d97a5 Mon Sep 17 00:00:00 2001 From: Leonardo Zide Date: Mon, 19 Oct 2026 21:28:49 -0700 Subject: [PATCH] Return owned include files and consolidate source opening Callback-based include readers hid open failures and required parser callbacks to report success through captured variables. Replace them with OpenPieceDataFile and OpenPrimitiveFile returning lcResult of an owned file positioned at zero. The mesh loader consumes the file directly and retains its existing strict or permissive policy for missing includes. Route include reads, piece builds and primitive builds through the existing OpenFile service for archive and disk access. Preserve registered-source and fallback search order, directory retries after parse failure, and primitive high/low-detail selection. Primitive opening uses its own source metadata, including stud-style archives with directory libraries, and records opening errors through the existing synchronized failure path. Document the owning file contract and retry boundary. Validation: warning-free macOS Qt 6.10 Debug build, all 51 offscreen lifecycle checks and whitespace checks pass. Coverage includes independent cursors across unload, missing/invalid sources, include failure policy, parse retries, official precedence and low-detail/styled primitive builds. Successful GPU rendering was not tested. --- common/lc_library.cpp | 268 +++++++++++++++++---------------------- common/lc_library.h | 7 +- common/lc_meshloader.cpp | 21 ++- docs/asset-loading.md | 8 +- 4 files changed, 134 insertions(+), 170 deletions(-) diff --git a/common/lc_library.cpp b/common/lc_library.cpp index 1f137fe6..fa0427b7 100644 --- a/common/lc_library.cpp +++ b/common/lc_library.cpp @@ -1754,28 +1754,33 @@ lcPieceBuildResult lcPiecesLibrary::BuildPieceData(const lcPieceSourceSnapshot& if (Source.ZipFileType != lcZipFileType::Count && mZipFiles[static_cast(Source.ZipFileType)]) { - lcMemFile PieceFile; + lcFileSource FileSource; + FileSource.ArchiveType = static_cast(Source.ZipFileType); + FileSource.ArchiveIndex = Source.ZipFileIndex; + const lcResult> File = OpenFile(FileSource); - if (mZipFiles[static_cast(Source.ZipFileType)]->ExtractFile(Source.ZipFileIndex, PieceFile)) - Loaded = MeshLoader.LoadMesh(PieceFile, LC_MESHDATA_SHARED); + if (File) + Loaded = MeshLoader.LoadMesh(*File.value(), LC_MESHDATA_SHARED); } else { - lcDiskFile PieceFile; + lcFileSource FileSource; + FileSource.FilePath = QDir(Source.LibraryDirectory).absoluteFilePath(QStringLiteral("parts/") + Source.FileName); + const lcResult> File = OpenFile(FileSource); - PieceFile.SetFileName(QDir(Source.LibraryDirectory).absoluteFilePath(QStringLiteral("parts/") + Source.FileName)); - - if (PieceFile.Open(QIODevice::ReadOnly)) - Loaded = MeshLoader.LoadMesh(PieceFile, LC_MESHDATA_SHARED); + if (File) + Loaded = MeshLoader.LoadMesh(*File.value(), LC_MESHDATA_SHARED); + // Directory loading retries unofficial geometry after an open or parse failure. if (mHasUnofficial && !Loaded) { MeshData.Clear(); - PieceFile.SetFileName(QDir(Source.LibraryDirectory).absoluteFilePath(QStringLiteral("unofficial/parts/") + Source.FileName)); + FileSource.FilePath = QDir(Source.LibraryDirectory).absoluteFilePath(QStringLiteral("unofficial/parts/") + Source.FileName); + const lcResult> UnofficialFile = OpenFile(FileSource); - if (PieceFile.Open(QIODevice::ReadOnly)) - Loaded = MeshLoader.LoadMesh(PieceFile, LC_MESHDATA_SHARED); + if (UnofficialFile) + Loaded = MeshLoader.LoadMesh(*UnofficialFile.value(), LC_MESHDATA_SHARED); } } @@ -1816,126 +1821,106 @@ void lcPiecesLibrary::SaveBuiltPieceDataCache(const lcPieceSourceSnapshot& Sourc WriteArchiveCacheFile(Source.CachePath, MeshData); } -void lcPiecesLibrary::GetPrimitiveFile(lcLibraryPrimitive* Primitive, std::function Callback) +lcResult> lcPiecesLibrary::OpenPrimitiveFile(const lcLibraryPrimitive* Primitive) { - if (mZipFiles[static_cast(lcZipFileType::Official)]) - { - lcMemFile IncludeFile; + lcFileSource Source; - if (mZipFiles[static_cast(Primitive->mZipFileType)]->ExtractFile(Primitive->mZipFileIndex, IncludeFile)) - Callback(IncludeFile); + if (Primitive->mZipFileType != lcZipFileType::Count) + { + Source.ArchiveType = static_cast(Primitive->mZipFileType); + Source.ArchiveIndex = Primitive->mZipFileIndex; } else - { - lcDiskFile IncludeFile(Primitive->mFileName); + Source.FilePath = Primitive->mFileName; - if (IncludeFile.Open(QIODevice::ReadOnly)) - Callback(IncludeFile); - } + return OpenFile(Source); } -void lcPiecesLibrary::GetPieceDataFile(const char* PieceName, std::function Callback) +lcResult> lcPiecesLibrary::OpenPieceDataFile(const char* PieceName) { const std::optional FileInfo = mPieceManager->GetPieceDataFileInfo(PieceName); - if (FileInfo) + if (mZipFiles[static_cast(lcZipFileType::Official)]) { - if (mZipFiles[static_cast(lcZipFileType::Official)] && FileInfo->ZipFileType != lcZipFileType::Count) + if (FileInfo && FileInfo->ZipFileType != lcZipFileType::Count) { - lcMemFile IncludeFile; - - if (mZipFiles[static_cast(FileInfo->ZipFileType)]->ExtractFile(FileInfo->ZipFileIndex, IncludeFile)) - Callback(IncludeFile); + lcFileSource Source; + Source.ArchiveType = static_cast(FileInfo->ZipFileType); + Source.ArchiveIndex = FileInfo->ZipFileIndex; + return OpenFile(Source); } - else + + if (!FileInfo) { - lcDiskFile IncludeFile; - char FileName[LC_MAXPATH]; - bool Found = false; - - snprintf(FileName, sizeof(FileName), "parts/%s", FileInfo->FileName.c_str()); - IncludeFile.SetFileName(mLibraryDir.absoluteFilePath(QLatin1String(FileName))); - Found = IncludeFile.Open(QIODevice::ReadOnly); - - if (mHasUnofficial && !Found) + for (const lcZipFileType Type : { lcZipFileType::Official, lcZipFileType::Unofficial }) { - snprintf(FileName, sizeof(FileName), "unofficial/parts/%s", FileInfo->FileName.c_str()); - IncludeFile.SetFileName(mLibraryDir.absoluteFilePath(QLatin1String(FileName))); - Found = IncludeFile.Open(QIODevice::ReadOnly); + const std::unique_ptr& Archive = mZipFiles[static_cast(Type)]; + + if (!Archive) + continue; + + const QString Prefix = Type == lcZipFileType::Official ? QStringLiteral("ldraw/") : QString(); + + for (const QString& Folder : { QStringLiteral("parts/"), QStringLiteral("p/") }) + { + const QByteArray FileName = (Prefix + Folder + QString::fromLatin1(PieceName)).toLatin1(); + + for (quint32 Index = 0; Index < Archive->mFiles.size(); Index++) + { + if (qstricmp(Archive->mFiles[Index].file_name, FileName.constData())) + continue; + + lcFileSource Source; + Source.ArchiveType = static_cast(Type); + Source.ArchiveIndex = Index; + + if (lcResult> File = OpenFile(Source)) + return File; + + break; + } + } } - if (Found) - Callback(IncludeFile); + return lcUnexpected(tr("Could not open included file %1.").arg(QString::fromLatin1(PieceName))); } } - else + + const QString Name = QString::fromLatin1(FileInfo ? FileInfo->FileName.c_str() : PieceName); + std::vector Folders(1, QStringLiteral("parts/")); + + if (!FileInfo) + Folders.emplace_back(QStringLiteral("p/")); + + if (mHasUnofficial) { - bool Found = false; + Folders.emplace_back(QStringLiteral("unofficial/parts/")); - if (mZipFiles[static_cast(lcZipFileType::Official)]) - { - lcMemFile IncludeFile; + if (!FileInfo) + Folders.emplace_back(QStringLiteral("unofficial/p/")); + } - auto LoadIncludeFile = [&IncludeFile, PieceName, this](const char* Folder, lcZipFileType ZipFileType) - { - char IncludeFileName[LC_MAXPATH]; - snprintf(IncludeFileName, sizeof(IncludeFileName), Folder, PieceName); - return mZipFiles[static_cast(ZipFileType)]->ExtractFile(IncludeFileName, IncludeFile); - }; + for (const QString& Folder : Folders) + { + const QString FileName = Folder + Name; + lcFileSource Source; + Source.FilePath = mLibraryDir.absoluteFilePath(FileName); - Found = LoadIncludeFile("ldraw/parts/%s", lcZipFileType::Official); - - if (!Found) - Found = LoadIncludeFile("ldraw/p/%s", lcZipFileType::Official); - - if (mZipFiles[static_cast(lcZipFileType::Unofficial)] && !Found) - { - Found = LoadIncludeFile("parts/%s", lcZipFileType::Unofficial); - - if (!Found) - Found = LoadIncludeFile("p/%s", lcZipFileType::Unofficial); - } - - if (Found) - Callback(IncludeFile); - } - else - { - lcDiskFile IncludeFile; - - auto LoadIncludeFile = [&IncludeFile, PieceName, this](const QLatin1String& Folder) - { - const QString IncludeFileName = Folder + PieceName; - IncludeFile.SetFileName(mLibraryDir.absoluteFilePath(IncludeFileName)); - if (IncludeFile.Open(QIODevice::ReadOnly)) - return true; + if (lcResult> File = OpenFile(Source)) + return File; #if defined(Q_OS_MACOS) || defined(Q_OS_LINUX) - // todo: search the parts/primitive lists and get the file name from there instead of using toLower - IncludeFile.SetFileName(mLibraryDir.absoluteFilePath(IncludeFileName.toLower())); - return IncludeFile.Open(QIODevice::ReadOnly); -#else - return false; -#endif - }; + if (!FileInfo) + { + Source.FilePath = mLibraryDir.absoluteFilePath(FileName.toLower()); - Found = LoadIncludeFile(QLatin1String("parts/")); - - if (!Found) - Found = LoadIncludeFile(QLatin1String("p/")); - - if (mHasUnofficial && !Found) - { - Found = LoadIncludeFile(QLatin1String("unofficial/parts/")); - - if (!Found) - Found = LoadIncludeFile(QLatin1String("unofficial/p/")); - } - - if (Found) - Callback(IncludeFile); + if (lcResult> File = OpenFile(Source)) + return File; } +#endif } + + return lcUnexpected(tr("Could not open included file %1.").arg(QString::fromLatin1(PieceName))); } void lcPiecesLibrary::ReleaseBuffers() @@ -2281,10 +2266,9 @@ lcResult lcPiecesLibrary::LoadPrimitiveData(lcLibraryPrimitive* Primitive) lcMeshLoader MeshLoader(Primitive->mMeshData, nullptr, lcMeshLoaderFlag::Optimize | lcMeshLoaderFlag::RequireAllIncludes); - const auto LoadFailed = [this, Primitive, &MeshLoader]() -> lcResult + const auto LoadFailed = [this, Primitive](QString Error) -> lcResult { Primitive->mMeshData.Clear(); - QString Error = MeshLoader.GetError(); if (Error.isEmpty()) Error = tr("Could not load included file %1.").arg(QString::fromLatin1(Primitive->mName)); @@ -2296,64 +2280,38 @@ lcResult lcPiecesLibrary::LoadPrimitiveData(lcLibraryPrimitive* Primitive) return lcUnexpected(std::move(Error)); }; - if (mZipFiles[static_cast(lcZipFileType::Official)]) + lcLibraryPrimitive* LowPrimitive = nullptr; + + if (mZipFiles[static_cast(lcZipFileType::Official)] && Primitive->mStud && !Primitive->mStudStyle) { - lcLibraryPrimitive* LowPrimitive = nullptr; - - lcMemFile PrimFile; - - if (Primitive->mStud && !Primitive->mStudStyle) + if (strncmp(Primitive->mName, "8/", 2)) // todo: low LOD for loose file libraries { - if (strncmp(Primitive->mName, "8/", 2)) // todo: this is currently the only place that uses mName so use mFileName instead. this should also be done for the loose file libraries. - { - char Name[LC_PIECE_NAME_LEN]; - lcstrcpy(Name, "8/"); - lcstrcat(Name, Primitive->mName); - lcstrupr(Name); + char Name[LC_PIECE_NAME_LEN]; + lcstrcpy(Name, "8/"); + lcstrcat(Name, Primitive->mName); + lcstrupr(Name); - LowPrimitive = FindPrimitive(Name); // todo: low primitives don't work with studlogo, because the low stud gets added as shared - } - } - - if (!mZipFiles[static_cast(Primitive->mZipFileType)]->ExtractFile(Primitive->mZipFileIndex, PrimFile)) - return LoadFailed(); - - if (!LowPrimitive) - { - if (!MeshLoader.LoadMesh(PrimFile, LC_MESHDATA_SHARED)) - return LoadFailed(); - } - else - { - if (!MeshLoader.LoadMesh(PrimFile, LC_MESHDATA_HIGH)) - return LoadFailed(); - - if (!mZipFiles[static_cast(LowPrimitive->mZipFileType)]->ExtractFile(LowPrimitive->mZipFileIndex, PrimFile)) - return LoadFailed(); - - if (!MeshLoader.LoadMesh(PrimFile, LC_MESHDATA_LOW)) - return LoadFailed(); + LowPrimitive = FindPrimitive(Name); // todo: low primitives don't work with studlogo } } - else + + const lcResult> File = OpenPrimitiveFile(Primitive); + + if (!File) + return LoadFailed(File.error()); + + if (!MeshLoader.LoadMesh(*File.value(), LowPrimitive ? LC_MESHDATA_HIGH : LC_MESHDATA_SHARED)) + return LoadFailed(MeshLoader.GetError()); + + if (LowPrimitive) { - if (Primitive->mZipFileType == lcZipFileType::Count) - { - lcDiskFile PrimFile(Primitive->mFileName); + const lcResult> LowFile = OpenPrimitiveFile(LowPrimitive); - if (!PrimFile.Open(QIODevice::ReadOnly) || !MeshLoader.LoadMesh(PrimFile, LC_MESHDATA_SHARED)) // todo: LOD like the zip files - return LoadFailed(); - } - else - { - lcMemFile PrimFile; + if (!LowFile) + return LoadFailed(LowFile.error()); - if (!mZipFiles[static_cast(Primitive->mZipFileType)]->ExtractFile(Primitive->mZipFileIndex, PrimFile)) - return LoadFailed(); - - if (!MeshLoader.LoadMesh(PrimFile, LC_MESHDATA_SHARED)) - return LoadFailed(); - } + if (!MeshLoader.LoadMesh(*LowFile.value(), LC_MESHDATA_LOW)) + return LoadFailed(MeshLoader.GetError()); } mPrimitiveMutex.lock(); diff --git a/common/lc_library.h b/common/lc_library.h index dbbd0fc9..675996cd 100644 --- a/common/lc_library.h +++ b/common/lc_library.h @@ -274,9 +274,10 @@ public: // Loads shared primitive data, waiting for another worker when necessary. lcResult LoadPrimitive(lcLibraryPrimitive* Primitive); - // Opens an include for the synchronous callback on the reading thread. - void GetPrimitiveFile(lcLibraryPrimitive* Primitive, std::function Callback); - void GetPieceDataFile(const char* FileName, std::function Callback); + // Opens an owned include on the reading thread, positioned at zero, or returns an error. + // Piece lookup preserves official-first search order; callers decide whether missing includes are fatal. + lcResult> OpenPrimitiveFile(const lcLibraryPrimitive* Primitive); + lcResult> OpenPieceDataFile(const char* PieceName); // Worker builds poll this shutdown flag without consulting live UI state. bool ShouldCancelLoading() const diff --git a/common/lc_meshloader.cpp b/common/lc_meshloader.cpp index 89e0dcb2..2ffbb454 100644 --- a/common/lc_meshloader.cpp +++ b/common/lc_meshloader.cpp @@ -1632,11 +1632,6 @@ bool lcMeshLoader::ReadMeshData(lcFile& File, const lcMatrix44& CurrentTransform bool Mirror = IncludeTransform.Determinant() < 0.0f; bool IncludeLoaded = true; - const auto FileCallback = [this, &IncludeTransform, &ColorCode, &Mirror, &InvertNext, &MeshDataType, &IncludeLoaded](lcFile& File) - { - IncludeLoaded = ReadMeshData(File, IncludeTransform, ColorCode, Mirror ^ InvertNext, MeshDataType); - }; - if (Primitive) { // Read shared loading state under the library's primitive lock. @@ -1659,20 +1654,24 @@ bool lcMeshLoader::ReadMeshData(lcFile& File, const lcMatrix44& CurrentTransform } else { - if (mFlags.testFlag(lcMeshLoaderFlag::RequireAllIncludes)) - IncludeLoaded = false; + const lcResult> IncludeFile = Library->OpenPrimitiveFile(Primitive); - Library->GetPrimitiveFile(Primitive, FileCallback); + if (IncludeFile) + IncludeLoaded = ReadMeshData(*IncludeFile.value(), IncludeTransform, ColorCode, Mirror ^ InvertNext, MeshDataType); + else + IncludeLoaded = !mFlags.testFlag(lcMeshLoaderFlag::RequireAllIncludes); } mMeshData.mHasStyleStud |= Primitive->mStudStyle | Primitive->mMeshData.mHasStyleStud; } else { - if (mFlags.testFlag(lcMeshLoaderFlag::RequireAllIncludes)) - IncludeLoaded = false; + const lcResult> IncludeFile = Library->OpenPieceDataFile(FileName); - Library->GetPieceDataFile(FileName, FileCallback); + if (IncludeFile) + IncludeLoaded = ReadMeshData(*IncludeFile.value(), IncludeTransform, ColorCode, Mirror ^ InvertNext, MeshDataType); + else + IncludeLoaded = !mFlags.testFlag(lcMeshLoaderFlag::RequireAllIncludes); } if (!IncludeLoaded) diff --git a/docs/asset-loading.md b/docs/asset-loading.md index a955d7e8..4360d20b 100644 --- a/docs/asset-loading.md +++ b/docs/asset-loading.md @@ -154,7 +154,13 @@ run on a worker, so file opening and archive extraction stay on that worker. The manager stores no file-reader callback or library instance. Success returns an owned `lcFile` positioned at zero: disk sources retain an open `lcDiskFile`, archive entries and decoded embedded data use `lcMemFile`. -Opening never searches for another source or parses its contents. Failures carry +Opening never searches for another source or parses its contents. +`OpenPieceDataFile()` and `OpenPrimitiveFile()` return the same owning result for +includes and use `OpenFile()` for disk/archive access. Piece include lookup keeps +its registered-source precedence and official-first fallback search; each result +has a separate cursor. The mesh loader consumes the file directly and decides +whether a missing include is fatal from `RequireAllIncludes`. Directory piece +building still retries unofficial geometry after an official open or parse failure. Failures carry an error; the caller retains the result while consuming the file. Archive access is unlocked before image decoding, and returned memory files are independent of their archive or embedded source. Each `lcZipFile` serializes its own extraction;