Skip to content

Commit

Permalink
[emapp] fixed a bug of loading the texture with ambiguous path (#299)
Browse files Browse the repository at this point in the history 
This commit fixes a bug of loading the texture with ambiguous path
(`./texture.png` and `texture.png`).

The bug causes the texture filename treats as key and cannot fetch
`IImageView` properly, so implements hash function to `URI` and uses it
not to include ambiguous path in `m_imageHandles`.
  • Loading branch information
@hkrn
hkrn authored Mar 29, 2023
1 parent 1af8162 commit d669051
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 36 deletions.
2 changes: 1 addition & 1 deletion emapp/include/emapp/Model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ class Model NANOEM_DECL_SEALED : public IDrawable, private NonCopyable {
struct LoadingImageItem;
typedef tinystl::vector<sg::LineVertexUnit, TinySTLAllocator> LineVertexList;
typedef tinystl::vector<LoadingImageItem *, TinySTLAllocator> LoadingImageItemList;
typedef tinystl::unordered_map<String, Image *, TinySTLAllocator> ImageMap;
typedef tinystl::unordered_map<URI, Image *, TinySTLAllocator> ImageMap;
typedef tinystl::unordered_map<String, const nanoem_model_bone_t *, TinySTLAllocator> BoneHashMap;
typedef tinystl::unordered_map<String, const nanoem_model_morph_t *, TinySTLAllocator> MorphHashMap;
typedef tinystl::unordered_map<const nanoem_model_bone_t *, const nanoem_model_constraint_t *, TinySTLAllocator>
Expand Down
4 changes: 4 additions & 0 deletions emapp/include/emapp/URI.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,17 @@ class URI NANOEM_DECL_SEALED {
String lastPathComponent() const;
const char *lastPathComponentConstString() const NANOEM_DECL_NOEXCEPT;
String pathExtension() const;
nanoem_u32_t hash() const NANOEM_DECL_NOEXCEPT;
bool isEmpty() const NANOEM_DECL_NOEXCEPT;
bool hasFragment() const;
bool equalsTo(const URI &other) const NANOEM_DECL_NOEXCEPT;
bool equalsToAbsolutePath(const String &other) const NANOEM_DECL_NOEXCEPT;
bool equalsToAbsolutePathConstString(const char *other) const NANOEM_DECL_NOEXCEPT;
bool equalsToFilenameConstString(const char *other) const NANOEM_DECL_NOEXCEPT;

bool operator==(const URI &value) const NANOEM_DECL_NOEXCEPT;
operator size_t() const NANOEM_DECL_NOEXCEPT { return hash(); }

private:
URI(const String &path, const String &fragment);

Expand Down
75 changes: 40 additions & 35 deletions emapp/src/Model.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2926,19 +2926,19 @@ Model::createImage(const nanoem_unicode_string_t *path, sg_wrap wrap, nanoem_u32
FileUtils::canonicalizePathSeparator(reinterpret_cast<const char *>(utf8Path), filename);
nanoemUnicodeStringFactoryDestroyByteArray(factory, utf8Path);
if (!filename.empty()) {
ImageMap::const_iterator it = m_imageHandles.find(filename);
const URI imageURI(Project::resolveArchiveURI(resolvedFileURI(), filename));
ImageMap::const_iterator it = m_imageHandles.find(imageURI);
if (it != m_imageHandles.end()) {
imagePtr = it->second;
}
else {
const URI imageURI(Project::resolveArchiveURI(resolvedFileURI(), filename));
Image *image = nanoem_new(Image);
image->setFilename(filename);
m_imageHandles.insert(tinystl::make_pair(filename, image));
m_imageURIs.insert(tinystl::make_pair(filename, imageURI));
m_imageHandles.insert(tinystl::make_pair(imageURI, image));
m_loadingImageItems.push_back(nanoem_new(LoadingImageItem(imageURI, filename, wrap, flags)));
imagePtr = image;
}
m_imageURIs.insert(tinystl::make_pair(filename, imageURI));
}
}
return imagePtr;
Expand All @@ -2950,39 +2950,43 @@ Model::internalUploadImage(const String &filename, const sg_image_desc &desc, bo
SG_PUSH_GROUPF("Model::internalUploadImage(name=%s, width=%d, height=%d, fileExist=%d)", filename.c_str(),
desc.width, desc.height, fileExist);
Image *image = nullptr;
ImageMap::iterator it = m_imageHandles.find(filename);
if (it != m_imageHandles.end()) {
image = it->second;
ImageLoader::copyImageDescrption(desc, image);
if (Inline::isDebugLabelEnabled()) {
char label[Inline::kMarkerStringLength];
StringUtils::format(label, sizeof(label), "Models/%s/%s", canonicalNameConstString(), filename.c_str());
image->setLabel(label);
}
image->setFileExist(fileExist);
image->create();
nanoem_unicode_string_factory_t *factory = m_project->unicodeStringFactory();
StringUtils::UnicodeStringScope scope(factory);
StringUtils::tryGetString(factory, filename, scope);
nanoem_rsize_t numMaterials;
nanoem_model_material_t *const *materials = nanoemModelGetAllMaterialObjects(m_opaque, &numMaterials);
for (nanoem_rsize_t i = 0; i < numMaterials; i++) {
const nanoem_model_material_t *materialPtr = materials[i];
if (!nanoemModelMaterialIsToonShared(materialPtr)) {
const nanoem_model_texture_t *texture = nanoemModelMaterialGetToonTextureObject(materialPtr);
const nanoem_unicode_string_t *texturePath = nanoemModelTextureGetPath(texture);
if (nanoemUnicodeStringFactoryCompareString(factory, scope.value(), texturePath) == 0) {
if (model::Material *material = model::Material::cast(materialPtr)) {
/* fetch left-bottom corner pixel */
const nanoem_rsize_t offset = nanoem_rsize_t(glm::max(desc.height - 1, 0)) * desc.width * 4;
const nanoem_u8_t *dataPtr = static_cast<const nanoem_u8_t *>(desc.data.subimage[0][0].ptr);
const Vector4 toonColor(glm::make_vec4(dataPtr + offset));
material->setToonColor(toonColor / Vector4(0xff));
FileEntityMap::const_iterator it = m_imageURIs.find(filename.c_str());
if (it != m_imageURIs.end()) {
const URI &imageURI = it->second;
ImageMap::iterator it2 = m_imageHandles.find(imageURI);
if (it2 != m_imageHandles.end()) {
image = it2->second;
ImageLoader::copyImageDescrption(desc, image);
if (Inline::isDebugLabelEnabled()) {
char label[Inline::kMarkerStringLength];
StringUtils::format(label, sizeof(label), "Models/%s/%s", canonicalNameConstString(), filename.c_str());
image->setLabel(label);
}
image->setFileExist(fileExist);
image->create();
nanoem_unicode_string_factory_t *factory = m_project->unicodeStringFactory();
StringUtils::UnicodeStringScope scope(factory);
StringUtils::tryGetString(factory, filename, scope);
nanoem_rsize_t numMaterials;
nanoem_model_material_t *const *materials = nanoemModelGetAllMaterialObjects(m_opaque, &numMaterials);
for (nanoem_rsize_t i = 0; i < numMaterials; i++) {
const nanoem_model_material_t *materialPtr = materials[i];
if (!nanoemModelMaterialIsToonShared(materialPtr)) {
const nanoem_model_texture_t *texture = nanoemModelMaterialGetToonTextureObject(materialPtr);
const nanoem_unicode_string_t *texturePath = nanoemModelTextureGetPath(texture);
if (nanoemUnicodeStringFactoryCompareString(factory, scope.value(), texturePath) == 0) {
if (model::Material *material = model::Material::cast(materialPtr)) {
/* fetch left-bottom corner pixel */
const nanoem_rsize_t offset = nanoem_rsize_t(glm::max(desc.height - 1, 0)) * desc.width * 4;
const nanoem_u8_t *dataPtr = static_cast<const nanoem_u8_t *>(desc.data.subimage[0][0].ptr);
const Vector4 toonColor(glm::make_vec4(dataPtr + offset));
material->setToonColor(toonColor / Vector4(0xff));
}
}
}
}
EMLOG_DEBUG("The image is allocated: name={} ID={}", filename.c_str(), image->handle().id);
}
EMLOG_DEBUG("The image is allocated: name={} ID={}", filename.c_str(), image->handle().id);
}
SG_POP_GROUP();
return image;
Expand Down Expand Up @@ -3209,7 +3213,7 @@ Model::internalClear()
}
for (ImageMap::const_iterator it = m_imageHandles.begin(), end = m_imageHandles.end(); it != end; ++it) {
Image *image = it->second;
SG_INSERT_MARKERF("Model::internalClear(image=%d, name=%s)", image->handle().id, it->first.c_str());
SG_INSERT_MARKERF("Model::internalClear(image=%d, name=%s)", image->handle().id, image->filenameConstString());
image->destroy();
nanoem_delete(image);
}
Expand Down Expand Up @@ -5404,7 +5408,8 @@ Model::getAllImageViews(ImageViewMap &value) const
{
value.clear();
for (ImageMap::const_iterator it = m_imageHandles.begin(), end = m_imageHandles.end(); it != end; ++it) {
value.insert(tinystl::make_pair(it->first, static_cast<IImageView *>(it->second)));
IImageView *view = static_cast<IImageView *>(it->second);
value.insert(tinystl::make_pair(view->filename(), view));
}
}

Expand Down
19 changes: 19 additions & 0 deletions emapp/src/URI.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "emapp/StringUtils.h"

#include "bx/hash.h"

namespace nanoem {

String
Expand Down Expand Up @@ -145,6 +147,16 @@ URI::pathExtension() const
return pathExtension(m_absolutePath);
}

nanoem_u32_t
URI::hash() const NANOEM_DECL_NOEXCEPT
{
bx::HashMurmur2A hasher;
hasher.begin();
hasher.add(m_absolutePath.c_str(), m_absolutePath.size());
hasher.add(m_fragment.c_str(), m_fragment.size());
return hasher.end();
}

bool
URI::isEmpty() const NANOEM_DECL_NOEXCEPT
{
Expand Down Expand Up @@ -182,6 +194,13 @@ URI::equalsToFilenameConstString(const char *other) const NANOEM_DECL_NOEXCEPT
return p ? StringUtils::equalsIgnoreCase(p, other) : false;
}

bool
URI::operator==(const URI &value) const NANOEM_DECL_NOEXCEPT
{
return StringUtils::equals(absolutePathConstString(), value.absolutePathConstString()) &&
StringUtils::equals(fragmentConstString(), value.fragmentConstString());
}

URI::URI(const String &path, const String &fragment)
: m_absolutePath(path)
, m_fragment(fragment)
Expand Down

0 comments on commit d669051

Please sign in to comment.