From ad57d50967e98759ebcace6625ecf9d247a0ec34 Mon Sep 17 00:00:00 2001 From: danij Date: Tue, 5 Mar 2013 18:06:42 +0000 Subject: [PATCH] Refactor|Textures: Refactored away the unnecessary UriValidationError(s) --- doomsday/client/include/gl/texturecontent.h | 2 + doomsday/client/include/resource/textures.h | 25 ++----- .../client/include/resource/texturescheme.h | 19 ++++- doomsday/client/src/resource/r_data.cpp | 68 +++++++++++------- doomsday/client/src/resource/textures.cpp | 71 ------------------- .../client/src/resource/texturescheme.cpp | 51 ++++++++++++- 6 files changed, 118 insertions(+), 118 deletions(-) diff --git a/doomsday/client/include/gl/texturecontent.h b/doomsday/client/include/gl/texturecontent.h index 71d6e44f68..3b9e6cf0ae 100644 --- a/doomsday/client/include/gl/texturecontent.h +++ b/doomsday/client/include/gl/texturecontent.h @@ -22,6 +22,8 @@ #ifndef LIBDENG_TEXTURECONTENT_H #define LIBDENG_TEXTURECONTENT_H +#include "api_gl.h" + #ifdef __cplusplus extern "C" { #endif diff --git a/doomsday/client/include/resource/textures.h b/doomsday/client/include/resource/textures.h index c96ca7adc5..8492678ffd 100644 --- a/doomsday/client/include/resource/textures.h +++ b/doomsday/client/include/resource/textures.h @@ -69,21 +69,6 @@ class Textures : DENG2_OBSERVES(TextureScheme, ManifestDefined), /// An unknown scheme was referenced. @ingroup errors DENG2_ERROR(UnknownSchemeError); - /// Base class for all URI validation errors. @ingroup errors - DENG2_ERROR(UriValidationError); - - /// The validation URI is missing the scheme component. @ingroup errors - DENG2_SUB_ERROR(UriValidationError, UriMissingSchemeError); - - /// The validation URI is missing the path component. @ingroup errors - DENG2_SUB_ERROR(UriValidationError, UriMissingPathError); - - /// The validation URI specifies an unknown scheme. @ingroup errors - DENG2_SUB_ERROR(UriValidationError, UriUnknownSchemeError); - - /// The validation URI is a URN. @ingroup errors - DENG2_SUB_ERROR(UriValidationError, UriIsUrnError); - /** * ResourceClass encapsulates the properties and logics belonging to a logical * class of resource. @@ -198,9 +183,13 @@ class Textures : DENG2_OBSERVES(TextureScheme, ManifestDefined), * * @return Manifest for this URI; otherwise @c 0 if @a uri is invalid. */ - Manifest *declare(Uri const &uri, de::Texture::Flags flags, - Vector2i const &dimensions, Vector2i const &origin, int uniqueId, - de::Uri const *resourceUri = 0); + inline Manifest &declare(Uri const &uri, de::Texture::Flags flags, + Vector2i const &dimensions, Vector2i const &origin, int uniqueId, + de::Uri const *resourceUri = 0) + { + return scheme(uri.scheme()).declare(uri.path(), flags, dimensions, + origin, uniqueId, resourceUri); + } /** * Returns a list of all the unique texture instances in the collection, diff --git a/doomsday/client/include/resource/texturescheme.h b/doomsday/client/include/resource/texturescheme.h index 03389bdd85..2f79294bbb 100644 --- a/doomsday/client/include/resource/texturescheme.h +++ b/doomsday/client/include/resource/texturescheme.h @@ -82,12 +82,25 @@ class TextureScheme : DENG2_OBSERVES(TextureManifest, UniqueIdChanged), /** * Insert a new manifest at the given @a path into the scheme. * If a manifest already exists at this path, the existing manifest is - * returned and this is a no-op. + * returned. + * + * If any of the property values (flags, dimensions, etc...) differ from + * that which is already defined in the pre-existing manifest, any texture + * which is currently associated is released (any GL-textures acquired for + * it are deleted). + * + * @param path Virtual path for the resultant manifest. + * @param flags Texture flags property. + * @param dimensions Logical dimensions property. + * @param origin World origin offset property. + * @param uniqueId Unique identifier property. + * @param resourceUri Resource URI property. * - * @param path Virtual path for the resultant manifest. * @return The (possibly newly created) manifest at @a path. */ - Manifest &declare(Path const &path); + Manifest &declare(Path const &path, Texture::Flags flags, + Vector2i const &dimensions, Vector2i const &origin, + int uniqueId, de::Uri const *resourceUri); /** * Determines if a manifest exists on the given @a path. diff --git a/doomsday/client/src/resource/r_data.cpp b/doomsday/client/src/resource/r_data.cpp index 5c6f8ea1a4..ff57c9c904 100644 --- a/doomsday/client/src/resource/r_data.cpp +++ b/doomsday/client/src/resource/r_data.cpp @@ -237,13 +237,20 @@ DENG_EXTERN_C patchid_t R_DeclarePatch(char const *encodedName) int uniqueId = textures.scheme("Patches").count() + 1; // 1-based index. de::Uri resourceUri = composeLumpIndexResourceUrn(lumpNum); - TextureManifest *manifest = textures.declare(uri, flags, dimensions, origin, uniqueId, &resourceUri); - if(!manifest) return 0; // Invalid uri? + try + { + TextureManifest &manifest = textures.declare(uri, flags, dimensions, origin, uniqueId, &resourceUri); - /// @todo Defer until necessary (manifest texture is first referenced). - deriveTexture(*manifest); + /// @todo Defer until necessary (manifest texture is first referenced). + deriveTexture(manifest); - return uniqueId; + return uniqueId; + } + catch(TextureScheme::InvalidPathError const &er) + { + LOG_WARNING(er.asText() + ". Failed declaring texture \"%s\", ignoring.") << uri; + } + return 0; } #undef R_GetPatchInfo @@ -644,15 +651,15 @@ static void processCompositeTextureDefs(CompositeTextures &defs) */ if(def.origIndex() == 0) flags |= Texture::NoDraw; - - TextureManifest *manifest = App_Textures().declare(uri, flags, def.logicalDimensions(), Vector2i(), def.origIndex()); - if(manifest) + try { + TextureManifest &manifest = App_Textures().declare(uri, flags, def.logicalDimensions(), Vector2i(), def.origIndex()); + // Are we redefining an existing texture? - if(manifest->hasTexture()) + if(manifest.hasTexture()) { // Yes. Destroy the existing definition (*should* exist). - Texture &tex = manifest->texture(); + Texture &tex = manifest.texture(); CompositeTexture *oldDef = reinterpret_cast(tex.userDataPointer()); if(oldDef) { @@ -666,15 +673,16 @@ static void processCompositeTextureDefs(CompositeTextures &defs) continue; } // A new texture. - else if(Texture *tex = manifest->derive()) + else if(Texture *tex = manifest.derive()) { tex->setUserDataPointer((void *)&def); continue; } } - - /// @todo Defer until necessary (manifest texture is first referenced). - LOG_WARNING("Failed defining Texture for patch composite \"%s\", ignoring.") << uri; + catch(TextureScheme::InvalidPathError const &er) + { + LOG_WARNING(er.asText() + ". Failed declaring texture \"%s\", ignoring.") << uri; + } delete &def; } @@ -831,7 +839,7 @@ void R_InitSpriteTextures() continue; } - de::Uri uri = de::Uri("Sprites", Path(fileName)); + de::Uri uri("Sprites", Path(fileName)); Texture::Flags flags = 0; // If this is from an add-on flag it as "custom". @@ -866,10 +874,15 @@ void R_InitSpriteTextures() file.unlock(); de::Uri resourceUri = composeLumpIndexResourceUrn(i); - TextureManifest *manifest = App_Textures().declare(uri, flags, dimensions, origin, uniqueId, &resourceUri); - if(!manifest) continue; // Invalid uri? - - uniqueId++; + try + { + App_Textures().declare(uri, flags, dimensions, origin, uniqueId, &resourceUri); + uniqueId++; + } + catch(TextureScheme::InvalidPathError const &er) + { + LOG_WARNING(er.asText() + ". Failed declaring texture \"%s\", ignoring.") << uri; + } } while(Stack_Height(stack)) @@ -911,12 +924,19 @@ Texture *R_DefineTexture(String schemeName, de::Uri const &resourceUri, } de::Uri uri(scheme.name(), Path(String("%1").arg(uniqueId, 8, 10, QChar('0')))); - TextureManifest *manifest = App_Textures().declare(uri, Texture::Custom, dimensions, - Vector2i(), uniqueId, &resourceUri); - if(!manifest) return 0; // Invalid URI? + try + { + TextureManifest &manifest = App_Textures().declare(uri, Texture::Custom, dimensions, + Vector2i(), uniqueId, &resourceUri); - /// @todo Defer until necessary (manifest texture is first referenced). - return deriveTexture(*manifest); + /// @todo Defer until necessary (manifest texture is first referenced). + return deriveTexture(manifest); + } + catch(TextureScheme::InvalidPathError const &er) + { + LOG_WARNING(er.asText() + ". Failed declaring texture \"%s\", ignoring.") << uri; + } + return 0; } Texture *R_DefineTexture(String schemeName, de::Uri const &resourceUri) diff --git a/doomsday/client/src/resource/textures.cpp b/doomsday/client/src/resource/textures.cpp index 6e5d71db8d..254d9929a2 100644 --- a/doomsday/client/src/resource/textures.cpp +++ b/doomsday/client/src/resource/textures.cpp @@ -236,77 +236,6 @@ Textures::All const &Textures::all() const return d->textures; } -TextureManifest *Textures::declare(Uri const &uri, de::Texture::Flags flags, - Vector2i const &dimensions, Vector2i const &origin, int uniqueId, de::Uri const *resourceUri) -{ - LOG_AS("Textures::declare"); - - // Ensure we have a properly formed URI (but not a URN - this is a resource path). - if(uri.isEmpty()) - { - /// @throw UriMissingPathError The URI is missing the required path component. - throw UriMissingPathError("Textures::declare", "Missing path in URI \"" + uri.asText() + "\""); - } - if(uri.scheme().isEmpty()) - { - /// @throw UriMissingSchemeError The URI is missing the required scheme component. - throw UriMissingSchemeError("Textures::declare", "Missing scheme in URI \"" + uri.asText() + "\""); - } - else if(!knownScheme(uri.scheme())) - { - /// @throw UriUnknownSchemeError The URI specifies an unknown scheme. - throw UriUnknownSchemeError("Textures::declare", "Unknown scheme in URI \"" + uri.asText() + "\""); - } - - // Have we already created a manifest for this? - TextureManifest *manifest = 0; - try - { - manifest = &find(uri); - } - catch(NotFoundError const &) - { - manifest = &scheme(uri.scheme()).declare(uri.path()); - } - - /* - * (Re)configure the manifest. - */ - bool mustRelease = false; - - manifest->flags() = flags; - manifest->setOrigin(origin); - - if(manifest->setLogicalDimensions(dimensions)) - { - mustRelease = true; - } - - // We don't care whether these identfiers are truely unique. Our only - // responsibility is to release textures when they change. - if(manifest->setUniqueId(uniqueId)) - { - mustRelease = true; - } - - if(resourceUri && manifest->setResourceUri(*resourceUri)) - { - // The mapped resource is being replaced, so release any existing Texture. - /// @todo Only release if this Texture is bound to only this binding. - mustRelease = true; - } - - if(mustRelease && manifest->hasTexture()) - { -#ifdef __CLIENT__ - /// @todo Update any Materials (and thus Surfaces) which reference this. - GL_ReleaseGLTexturesByTexture(manifest->texture()); -#endif - } - - return manifest; -} - static int iterateManifests(TextureScheme const &scheme, int (*callback)(TextureManifest &manifest, void *parameters), void *parameters) { diff --git a/doomsday/client/src/resource/texturescheme.cpp b/doomsday/client/src/resource/texturescheme.cpp index 71d5d1ac13..da6f964771 100644 --- a/doomsday/client/src/resource/texturescheme.cpp +++ b/doomsday/client/src/resource/texturescheme.cpp @@ -18,7 +18,9 @@ */ #include "TextureManifest" - +#ifdef __CLIENT__ +# include "gl/gl_texmanager.h" +#endif #include "resource/texturescheme.h" using namespace de; @@ -172,8 +174,18 @@ String const &TextureScheme::name() const return d->name; } -TextureManifest &TextureScheme::declare(Path const &path) +TextureManifest &TextureScheme::declare(Path const &path, + Texture::Flags flags, Vector2i const &dimensions, Vector2i const &origin, + int uniqueId, de::Uri const *resourceUri) { + LOG_AS("TextureScheme::declare"); + + if(path.isEmpty()) + { + /// @throw InvalidPathError An empty path was specified. + throw InvalidPathError("TextureScheme::declare", "Missing/zero-length path was supplied"); + } + int const sizeBefore = d->index.size(); Manifest *newManifest = &d->index.insert(path); DENG2_ASSERT(newManifest); @@ -193,6 +205,41 @@ TextureManifest &TextureScheme::declare(Path const &path) DENG2_FOR_AUDIENCE(ManifestDefined, i) i->schemeManifestDefined(*this, *newManifest); } + /* + * (Re)configure the manifest. + */ + bool mustRelease = false; + + newManifest->flags() = flags; + newManifest->setOrigin(origin); + + if(newManifest->setLogicalDimensions(dimensions)) + { + mustRelease = true; + } + + // We don't care whether these identfiers are truely unique. Our only + // responsibility is to release textures when they change. + if(newManifest->setUniqueId(uniqueId)) + { + mustRelease = true; + } + + if(resourceUri && newManifest->setResourceUri(*resourceUri)) + { + // The mapped resource is being replaced, so release any existing Texture. + /// @todo Only release if this Texture is bound to only this binding. + mustRelease = true; + } + + if(mustRelease && newManifest->hasTexture()) + { +#ifdef __CLIENT__ + /// @todo Update any Materials (and thus Surfaces) which reference this. + GL_ReleaseGLTexturesByTexture(newManifest->texture()); +#endif + } + return *newManifest; }