From 9877284765c098708904e873cbddb02283aac530 Mon Sep 17 00:00:00 2001 From: danij Date: Fri, 9 Nov 2012 23:53:14 +0000 Subject: [PATCH] Refactor|PathTree: Compose paths to de::Uri instead of de::String Composing the paths to Uri rather than String presents several opportunities to improve both the design and performance of the whole virtual file system. (should a string representation be required it can be trivially obtained with Uri::compose()). As PathTree is already aware of the path fragment hashes (they are produced in the process of building the tree), these can now be passed on to the composed Uri. Uri should now be redesigned so that the internal representation is modelled as a list of path fragments with associated hashes. (in the process, improving the performance of Uri as these same hashes can be used internally to accelerate Uri comparisons). This will in turn mean that PathMap is made redundant. Once these refactorings are completed, a fully composed path will be necessary only when populating the PathTree or when displaying the path to the user (e.g., logging, console). --- doomsday/engine/api/uri.h | 2 +- doomsday/engine/portable/include/file.h | 15 ++++++++++++++- doomsday/engine/portable/include/pathtree.h | 19 ++++++++++++++++++- doomsday/engine/portable/include/wad.h | 10 +++++----- doomsday/engine/portable/include/zip.h | 10 +++++----- doomsday/engine/portable/src/con_data.cpp | 3 ++- doomsday/engine/portable/src/dd_main.cpp | 6 ++++-- doomsday/engine/portable/src/def_main.cpp | 4 +++- doomsday/engine/portable/src/file.cpp | 11 +++++++---- doomsday/engine/portable/src/fonts.cpp | 5 ++--- doomsday/engine/portable/src/fs_main.cpp | 2 +- doomsday/engine/portable/src/pathtreenode.cpp | 15 +++++++-------- .../portable/src/resource/materials.cpp | 3 +-- .../engine/portable/src/resource/textures.cpp | 3 +-- doomsday/engine/portable/src/uri.cpp | 18 ++++++++++-------- doomsday/engine/portable/src/wad.cpp | 14 +++++++------- doomsday/engine/portable/src/zip.cpp | 18 +++++++++--------- 17 files changed, 97 insertions(+), 61 deletions(-) diff --git a/doomsday/engine/api/uri.h b/doomsday/engine/api/uri.h index d894911c83..25b34acc96 100644 --- a/doomsday/engine/api/uri.h +++ b/doomsday/engine/api/uri.h @@ -191,7 +191,7 @@ namespace de * * @return Plain-text String representation. */ - AutoStr* compose() const; + String compose() const; /** * Transform the uri into a human-friendly representation. Percent decoding done. diff --git a/doomsday/engine/portable/include/file.h b/doomsday/engine/portable/include/file.h index 35f275ec67..01f18abf4d 100644 --- a/doomsday/engine/portable/include/file.h +++ b/doomsday/engine/portable/include/file.h @@ -83,14 +83,27 @@ class File1 /// @return Name of this file. virtual String const& name() const; + /** + * Compose the a URI to this file. + * + * @param delimiter Delimit directory using this character. + * + * @return The composed URI. + */ + virtual de::Uri composeUri(QChar delimiter = '/') const; + /** * Compose the absolute VFS path to this file. * * @param delimiter Delimit directory using this character. * * @return String containing the absolute path. + * + * @deprecated Prefer to use @ref composeUri() instead. */ - virtual String composePath(char delimiter = '/') const; + String composePath(QChar delimiter = '/') const { + return composeUri(delimiter).compose(); + } /// @return @c true iff this file is contained by another. bool isContained() const; diff --git a/doomsday/engine/portable/include/pathtree.h b/doomsday/engine/portable/include/pathtree.h index 2974d04617..21b75d93e3 100644 --- a/doomsday/engine/portable/include/pathtree.h +++ b/doomsday/engine/portable/include/pathtree.h @@ -36,6 +36,7 @@ #include #include #include "pathmap.h" +#include "uri.h" /** * @defgroup pathComparisonFlags Path Comparison Flags @@ -165,6 +166,18 @@ namespace de */ int comparePath(PathMap& candidatePath, int flags) const; + /** + * Composes the URI for this node. 'Composing' the path of a node is to + * upwardly reconstruct the whole path toward the root of the hierarchy. + * + * @param delimiter Names in the composed path hierarchy will be delimited + * with this character. Paths to branches always include + * a terminating delimiter. + * + * @return The composed uri. + */ + Uri composeUri(QChar delimiter = '/') const; + /** * Composes the full path for this node. 'Composing' the path of a node is * to upwardly reconstruct the whole path toward the root of the hierarchy. @@ -174,8 +187,12 @@ namespace de * a terminating delimiter. * * @return The composed path. + * + * @deprecated Prefer to use @ref composeUri() instead. */ - String composePath(QChar delimiter = '/') const; + String composePath(QChar delimiter = '/') const { + return composeUri(delimiter).compose(); + } /** * Sets the user-specified custom pointer. diff --git a/doomsday/engine/portable/include/wad.h b/doomsday/engine/portable/include/wad.h index d349cf4a3a..bf5a2af1da 100644 --- a/doomsday/engine/portable/include/wad.h +++ b/doomsday/engine/portable/include/wad.h @@ -78,17 +78,17 @@ class Wad : public File1 PathTree::Node& lumpDirectoryNode(int lumpIdx) const; /** - * Compose the absolute VFS path to a lump contained by this file. + * Compose an absolute URI to a lump contained by this file. * - * @note Always returns a valid string object. If @a lumpIdx is not valid a - * zero-length string is returned. + * @note Always returns a valid Uri object. If @a lumpIdx is not valid a + * zero-length Uri is returned. * * @param lumpIdx Logical index for the lump. * @param delimiter Delimit directory separators using this character. * - * @return String containing the absolute path. + * @return The absolute URI. */ - String composeLumpPath(int lumpIdx, char delimiter = '/'); + de::Uri composeLumpUri(int lumpIdx, QChar delimiter = '/'); /** * Retrieve a lump contained by this file. diff --git a/doomsday/engine/portable/include/zip.h b/doomsday/engine/portable/include/zip.h index c639c6b697..2eadc6cae1 100644 --- a/doomsday/engine/portable/include/zip.h +++ b/doomsday/engine/portable/include/zip.h @@ -81,17 +81,17 @@ class Zip : public File1 PathTree::Node& lumpDirectoryNode(int lumpIdx) const; /** - * Compose the absolute VFS path to a lump contained by this file. + * Compose an absolute URI to a lump contained by this file. * - * @note Always returns a valid string object. If @a lumpIdx is not valid a - * zero-length string is returned. + * @note Always returns a valid Uri object. If @a lumpIdx is not valid a + * zero-length Uri is returned. * * @param lumpIdx Logical index for the lump. * @param delimiter Delimit directory separators using this character. * - * @return String containing the absolute path. + * @return The absolute URI. */ - String composeLumpPath(int lumpIdx, char delimiter = '/'); + de::Uri composeLumpUri(int lumpIdx, QChar delimiter = '/'); /** * Retrieve a lump contained by this file. diff --git a/doomsday/engine/portable/src/con_data.cpp b/doomsday/engine/portable/src/con_data.cpp index a7c90d4695..3eafedb321 100644 --- a/doomsday/engine/portable/src/con_data.cpp +++ b/doomsday/engine/portable/src/con_data.cpp @@ -443,7 +443,8 @@ int CVar_Flags(const cvar_t* var) AutoStr* CVar_ComposePath(cvar_t const* var) { DENG_ASSERT(var); - QByteArray path = reinterpret_cast(var->directoryNode)->composePath(CVARDIRECTORY_DELIMITER).toUtf8(); + CVarDirectory::Node& node = *reinterpret_cast(var->directoryNode); + QByteArray path = node.composePath(CVARDIRECTORY_DELIMITER).toUtf8(); return AutoStr_FromTextStd(path.constData()); } diff --git a/doomsday/engine/portable/src/dd_main.cpp b/doomsday/engine/portable/src/dd_main.cpp index db805afbe8..41229c897e 100644 --- a/doomsday/engine/portable/src/dd_main.cpp +++ b/doomsday/engine/portable/src/dd_main.cpp @@ -2338,7 +2338,8 @@ static bool tryLoadFile(String path, size_t baseOffset, de::File1** file) { de::FileHandle& hndl = App_FileSystem()->openFile(path, "rb", baseOffset, false /* no duplicates */); - QByteArray pathUtf8 = NativePath(hndl.file().composePath()).pretty().toUtf8(); + de::Uri uri = hndl.file().composeUri(); + QByteArray pathUtf8 = NativePath(uri.asText()).pretty().toUtf8(); VERBOSE( Con_Message("Loading \"%s\"...\n", pathUtf8.constData()) ) App_FileSystem()->index(hndl.file()); @@ -2363,7 +2364,8 @@ static bool tryUnloadFile(String path) try { de::File1& file = App_FileSystem()->find(path); - QByteArray pathUtf8 = de::NativePath(file.composePath()).pretty().toUtf8(); + de::Uri uri = file.composeUri(); + QByteArray pathUtf8 = NativePath(uri.asText()).pretty().toUtf8(); // Do not attempt to unload a resource required by the current game. if(games->currentGame().isRequiredFile(file)) diff --git a/doomsday/engine/portable/src/def_main.cpp b/doomsday/engine/portable/src/def_main.cpp index cc7c2b005f..20755f9cc5 100644 --- a/doomsday/engine/portable/src/def_main.cpp +++ b/doomsday/engine/portable/src/def_main.cpp @@ -25,6 +25,8 @@ #include #include +#include + #include "de_base.h" #include "de_system.h" #include "de_platform.h" @@ -760,7 +762,7 @@ void Def_ReadLumpDefs(void) if(!DED_ReadLump(&defs, lump.info().lumpIdx)) { - QByteArray path = lump.container().composePath().toUtf8(); + QByteArray path = de::NativePath(lump.container().composePath()).pretty().toUtf8(); Con_Error("DD_ReadLumpDefs: Parse error reading \"%s:DD_DEFNS\".\n", path.constData()); } } diff --git a/doomsday/engine/portable/src/file.cpp b/doomsday/engine/portable/src/file.cpp index 0da3d4b04d..03f0e32c86 100644 --- a/doomsday/engine/portable/src/file.cpp +++ b/doomsday/engine/portable/src/file.cpp @@ -21,6 +21,8 @@ * 02110-1301 USA */ +#include + #include "de_base.h" #include "de_filesys.h" @@ -57,7 +59,7 @@ bool File1::isContained() const File1& File1::container() const { - if(!container_) throw NotContainedError("File1::container", "File \"" + composePath() + " is not contained"); + if(!container_) throw NotContainedError("File1::container", "File \"" + NativePath(composePath()).pretty() + " is not contained"); return *container_; } @@ -66,10 +68,11 @@ de::FileHandle& File1::handle() return *handle_; } -String File1::composePath(char delimiter) const +de::Uri File1::composeUri(QChar delimiter) const { - String result = path_; - if(delimiter != '/') throw Error("File1::composePath", "Non '/' delimiter not yet implemented"); + QByteArray pathUtf8 = path_.toUtf8(); + de::Uri result = de::Uri(pathUtf8.constData(), RC_NULL); + if(delimiter != '/') throw Error("File1::composeUri", "Non '/' delimiter not yet implemented"); return result; } diff --git a/doomsday/engine/portable/src/fonts.cpp b/doomsday/engine/portable/src/fonts.cpp index 6453300e76..4dfa4cb6c9 100644 --- a/doomsday/engine/portable/src/fonts.cpp +++ b/doomsday/engine/portable/src/fonts.cpp @@ -143,8 +143,7 @@ static inline fontnamespaceid_t namespaceIdForDirectoryNode(FontRepository::Node static de::Uri composeUriForDirectoryNode(FontRepository::Node const& node) { Str const* namespaceName = Fonts_NamespaceName(namespaceIdForDirectoryNode(node)); - QByteArray path = node.composePath(FONTS_PATH_DELIMITER).toUtf8(); - return de::Uri(path.constData(), RC_NULL).setScheme(Str_Text(namespaceName)); + return node.composeUri(FONTS_PATH_DELIMITER).setScheme(Str_Text(namespaceName)); } /// @pre fontIdMap has been initialized and is large enough! @@ -1357,7 +1356,7 @@ static int composeAndCompareDirectoryNodePaths(void const* a, void const* b) QByteArray pathBUtf8 = nodeB.composePath(FONTS_PATH_DELIMITER).toUtf8(); AutoStr* pathA = Str_PercentDecode(AutoStr_FromTextStd(pathAUtf8.constData())); AutoStr* pathB = Str_PercentDecode(AutoStr_FromTextStd(pathBUtf8.constData())); - return stricmp(Str_Text(pathA), Str_Text(pathB)); + return qstricmp(Str_Text(pathA), Str_Text(pathB)); } /** diff --git a/doomsday/engine/portable/src/fs_main.cpp b/doomsday/engine/portable/src/fs_main.cpp index 87b6b06293..744bed4b08 100644 --- a/doomsday/engine/portable/src/fs_main.cpp +++ b/doomsday/engine/portable/src/fs_main.cpp @@ -461,7 +461,7 @@ FS1& FS1::index(de::File1& file) // Ensure this hasn't yet been indexed. FileList::const_iterator found = findListFile(d->loadedFiles, file); if(found != d->loadedFiles.end()) - throw de::Error("FS1::index", "File \"" + file.composePath() + "\" has already been indexed"); + throw de::Error("FS1::index", "File \"" + NativePath(file.composePath()).pretty() + "\" has already been indexed"); #endif // Publish lumps to one or more indexes? diff --git a/doomsday/engine/portable/src/pathtreenode.cpp b/doomsday/engine/portable/src/pathtreenode.cpp index 487308f26a..557b3a1207 100644 --- a/doomsday/engine/portable/src/pathtreenode.cpp +++ b/doomsday/engine/portable/src/pathtreenode.cpp @@ -219,7 +219,7 @@ static size_t maxStackDepth; typedef struct pathconstructorparams_s { size_t length; - String& composedPath; + String composedPath; QChar delimiter; } pathconstructorparams_t; @@ -278,11 +278,9 @@ static void pathConstructor(pathconstructorparams_t& parm, PathTree::Node const& * * Perhaps a fixed size MRU cache? -ds */ -String PathTree::Node::composePath(QChar delimiter) const +Uri PathTree::Node::composeUri(QChar delimiter) const { - String result; - - pathconstructorparams_t parm = { 0, result, delimiter }; + pathconstructorparams_t parm = { 0, String(), delimiter }; #ifdef LIBDENG_STACK_MONITOR stackStart = &parm; #endif @@ -296,16 +294,17 @@ String PathTree::Node::composePath(QChar delimiter) const // Terminating delimiter for branches. if(!delimiter.isNull() && !isLeaf()) - result.append(delimiter); + parm.composedPath += delimiter; - DENG2_ASSERT(result.size() == (int)parm.length); + DENG2_ASSERT(parm.composedPath.length() == (int)parm.length); #ifdef LIBDENG_STACK_MONITOR LOG_AS("pathConstructor"); LOG_INFO("Max stack depth: %1 bytes") << maxStackDepth; #endif - return result; + QByteArray pathUtf8 = parm.composedPath.toUtf8(); + return Uri(pathUtf8.constData(), RC_NULL); } } // namespace de diff --git a/doomsday/engine/portable/src/resource/materials.cpp b/doomsday/engine/portable/src/resource/materials.cpp index be632a9b8b..4981851524 100644 --- a/doomsday/engine/portable/src/resource/materials.cpp +++ b/doomsday/engine/portable/src/resource/materials.cpp @@ -264,8 +264,7 @@ static materialnamespaceid_t namespaceIdForDirectory(MaterialRepository& directo static de::Uri composeUriForDirectoryNode(MaterialRepository::Node const& node) { Str const* namespaceName = Materials_NamespaceName(namespaceIdForDirectory(node.tree())); - QByteArray path = node.composePath(MATERIALS_PATH_DELIMITER).toUtf8(); - return de::Uri(path.constData(), RC_NULL).setScheme(Str_Text(namespaceName)); + return node.composeUri(MATERIALS_PATH_DELIMITER).setScheme(Str_Text(namespaceName)); } static MaterialAnim* getAnimGroup(int number) diff --git a/doomsday/engine/portable/src/resource/textures.cpp b/doomsday/engine/portable/src/resource/textures.cpp index 09fd0cd66b..404aa3cc27 100644 --- a/doomsday/engine/portable/src/resource/textures.cpp +++ b/doomsday/engine/portable/src/resource/textures.cpp @@ -151,8 +151,7 @@ static inline texturenamespaceid_t namespaceIdForDirectoryNode(TextureRepository static de::Uri composeUriForDirectoryNode(TextureRepository::Node const& node) { Str const* namespaceName = Textures_NamespaceName(namespaceIdForDirectoryNode(node)); - QByteArray path = node.composePath(TEXTURES_PATH_DELIMITER).toUtf8(); - return de::Uri(path.constData(), RC_NULL).setScheme(Str_Text(namespaceName)); + return node.composeUri(TEXTURES_PATH_DELIMITER).setScheme(Str_Text(namespaceName)); } /// @pre textureIdMap has been initialized and is large enough! diff --git a/doomsday/engine/portable/src/uri.cpp b/doomsday/engine/portable/src/uri.cpp index bbead1c9dc..90172d3a2f 100644 --- a/doomsday/engine/portable/src/uri.cpp +++ b/doomsday/engine/portable/src/uri.cpp @@ -373,21 +373,22 @@ Uri& Uri::setUri(ddstring_t const* path) return setUri(path != 0? Str_Text(path) : 0); } -AutoStr* Uri::compose() const +String Uri::compose() const { - AutoStr* out = AutoStr_New(); if(!Str_IsEmpty(&d->scheme)) - Str_Appendf(out, "%s:", Str_Text(&d->scheme)); - Str_Append(out, Str_Text(&d->path)); - return out; + { + return String(Str_Text(&d->scheme)) + ":" + String(Str_Text(&d->path)); + } + return String(Str_Text(&d->path)); } String Uri::asText() const { // Just compose it for now, we can worry about making it 'pretty' later. - AutoStr* path = compose(); + QByteArray composed = compose().toUtf8(); + AutoStr* path = AutoStr_FromTextStd(composed.constData()); Str_PercentDecode(path); - return QString::fromUtf8(Str_Text(path)); + return String(Str_Text(path)); } static void writeUri(ddstring_t const* scheme, ddstring_t const* path, struct writer_s& writer) @@ -572,7 +573,8 @@ Uri* Uri_SetUriStr(Uri* uri, ddstring_t const* path) AutoStr* Uri_Compose(Uri const* uri) { SELF_CONST(uri); - return self->compose(); + QByteArray composed = self->compose().toUtf8(); + return AutoStr_FromTextStd(composed.constData()); } AutoStr* Uri_ToString(Uri const* uri) diff --git a/doomsday/engine/portable/src/wad.cpp b/doomsday/engine/portable/src/wad.cpp index d40bff93b9..cb266b19ae 100644 --- a/doomsday/engine/portable/src/wad.cpp +++ b/doomsday/engine/portable/src/wad.cpp @@ -70,15 +70,15 @@ class WadFile : public File1 } /** - * Compose the absolute VFS path to this file. + * Compose an absolute URI to this file. * * @param delimiter Delimit directory using this character. * - * @return String containing the absolute path. + * @return The absolute URI. */ - String composePath(char delimiter = '/') const + Uri composeUri(QChar delimiter = '/') const { - return dynamic_cast(container()).composeLumpPath(info_.lumpIdx, delimiter); + return dynamic_cast(container()).composeLumpUri(info_.lumpIdx, delimiter); } /** @@ -407,10 +407,10 @@ PathTree::Node& Wad::lumpDirectoryNode(int lumpIdx) const return *((*d->lumpNodeLut)[lumpIdx]); } -String Wad::composeLumpPath(int lumpIdx, char delimiter) +de::Uri Wad::composeLumpUri(int lumpIdx, QChar delimiter) { - if(!isValidIndex(lumpIdx)) return String(); - return lump(lumpIdx).directoryNode().composePath(delimiter); + if(!isValidIndex(lumpIdx)) return de::Uri(); + return lump(lumpIdx).directoryNode().composeUri(delimiter); } File1& Wad::lump(int lumpIdx) diff --git a/doomsday/engine/portable/src/zip.cpp b/doomsday/engine/portable/src/zip.cpp index c6c9b09d33..8ed06f8a89 100644 --- a/doomsday/engine/portable/src/zip.cpp +++ b/doomsday/engine/portable/src/zip.cpp @@ -153,15 +153,15 @@ class ZipFile : public File1 } /** - * Compose the absolute VFS path to this file. + * Compose an absolute URI to this file. * * @param delimiter Delimit directory using this character. * - * @return String containing the absolute path. + * @return The absolute URI. */ - String composePath(char delimiter = '/') const + Uri composePath(QChar delimiter = '/') const { - return dynamic_cast(container()).composeLumpPath(info_.lumpIdx, delimiter); + return dynamic_cast(container()).composeLumpUri(info_.lumpIdx, delimiter); } /** @@ -348,7 +348,7 @@ struct Zip::Instance // Scan the end of the file for the central directory end record. if(!locateCentralDirectory()) - throw FormatError("Zip::readLumpDirectory", String("Central directory in %1 not found").arg(de::NativePath(self->composePath()).pretty())); + throw FormatError("Zip::readLumpDirectory", "Central directory in \"" + NativePath(self->composePath()).pretty() + "\" not found"); // Read the central directory end record. centralend_t summary; @@ -356,7 +356,7 @@ struct Zip::Instance // Does the summary say something we don't like? if(summary.diskEntryCount != summary.totalEntryCount) - throw FormatError("Zip::readLumpDirectory", String("Multipart zip file \"%1\" not supported").arg(de::NativePath(self->composePath()).pretty())); + throw FormatError("Zip::readLumpDirectory", "Multipart zip file \"" + NativePath(self->composePath()).pretty() + "\" not supported"); // We'll load the file directory using one continous read into a temporary // local buffer before we process it into our runtime representation. @@ -577,10 +577,10 @@ PathTree::Node& Zip::lumpDirectoryNode(int lumpIdx) const return *((*d->lumpNodeLut)[lumpIdx]); } -String Zip::composeLumpPath(int lumpIdx, char delimiter) +de::Uri Zip::composeLumpUri(int lumpIdx, QChar delimiter) { - if(!isValidIndex(lumpIdx)) return String(); - return lump(lumpIdx).directoryNode().composePath(delimiter); + if(!isValidIndex(lumpIdx)) return de::Uri(); + return lump(lumpIdx).directoryNode().composeUri(delimiter); } File1& Zip::lump(int lumpIdx)