Fix various errors discovered by clang-tidy (#1351)

- Move the static function declared in MaterialVariants.h to the .cpp
- Fix several use-after-move that were horribly wrong
This commit is contained in:
Romain Guy
2019-06-26 15:25:58 -07:00
committed by GitHub
parent 10dc3a24e8
commit 847d7dd71a
10 changed files with 80 additions and 44 deletions

View File

@@ -41,6 +41,7 @@ set(COMMON_SRCS
src/shaders/ShaderGenerator.cpp
src/Enums.cpp
src/MaterialBuilder.cpp
src/MaterialVariants.cpp
src/PostprocessMaterialBuilder.cpp)
# Sources and headers for filamat

View File

@@ -581,8 +581,10 @@ bool MaterialBuilder::generateShaders(const std::vector<Variant>& variants, Chun
// Emit GLSL chunks (TextDictionaryReader and MaterialTextChunk).
if (!glslEntries.empty()) {
container.addChild<filamat::DictionaryTextChunk>(glslDictionary, ChunkType::DictionaryGlsl);
container.addChild<MaterialTextChunk>(std::move(glslEntries), glslDictionary, ChunkType::MaterialGlsl);
const auto& dictionaryChunk = container.addChild<filamat::DictionaryTextChunk>(
std::move(glslDictionary), ChunkType::DictionaryGlsl);
container.addChild<MaterialTextChunk>(std::move(glslEntries),
dictionaryChunk.getDictionary(), ChunkType::MaterialGlsl);
}
// Emit SPIRV chunks (SpirvDictionaryReader and MaterialSpirvChunk).
@@ -594,8 +596,10 @@ bool MaterialBuilder::generateShaders(const std::vector<Variant>& variants, Chun
// Emit Metal chunks (MetalDictionaryReader and MaterialMetalChunk).
if (!metalEntries.empty()) {
container.addChild<filamat::DictionaryTextChunk>(metalDictionary, ChunkType::DictionaryMetal);
container.addChild<MaterialTextChunk>(std::move(metalEntries), metalDictionary, ChunkType::MaterialMetal);
const auto& dictionaryChunk = container.addChild<filamat::DictionaryTextChunk>(
std::move(metalDictionary), ChunkType::DictionaryMetal);
container.addChild<MaterialTextChunk>(std::move(metalEntries),
dictionaryChunk.getDictionary(), ChunkType::MaterialMetal);
}
#endif
@@ -636,8 +640,7 @@ Package MaterialBuilder::build() noexcept {
bool success = generateShaders(variants, container, info);
// Flatten all chunks in the container into a Package.
size_t packageSize = container.getSize();
Package package(packageSize);
Package package(container.getSize());
Flattener f(package);
container.flatten(f);
package.setValid(success);

View File

@@ -0,0 +1,45 @@
/*
* Copyright (C) 2017 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "MaterialVariants.h"
namespace filamat {
std::vector<Variant> determineVariants(uint8_t variantFilter, bool isLit,
bool shadowMultiplier) {
std::vector<Variant> variants;
uint8_t variantMask = ~variantFilter;
for (uint8_t k = 0; k < filament::VARIANT_COUNT; k++) {
if (filament::Variant::isReserved(k)) {
continue;
}
// Remove variants for unlit materials
uint8_t v = filament::Variant::filterVariant(
k & variantMask, isLit || shadowMultiplier);
if (filament::Variant::filterVariantVertex(v) == k) {
variants.emplace_back(k, filament::backend::ShaderType::VERTEX);
}
if (filament::Variant::filterVariantFragment(v) == k) {
variants.emplace_back(k, filament::backend::ShaderType::FRAGMENT);
}
}
return variants;
}
} // namespace filamat

View File

@@ -19,6 +19,8 @@
#include <private/filament/Variant.h>
#include <backend/DriverEnums.h>
#include <vector>
namespace filamat {
@@ -32,29 +34,7 @@ struct Variant {
Stage stage;
};
std::vector<Variant> determineVariants(uint8_t variantFilter, bool isLit,
bool shadowMultiplier) {
std::vector<Variant> variants;
uint8_t variantMask = ~variantFilter;
for (uint8_t k = 0; k < filament::VARIANT_COUNT; k++) {
if (filament::Variant::isReserved(k)) {
continue;
}
// Remove variants for unlit materials
uint8_t v = filament::Variant::filterVariant(
k & variantMask, isLit || shadowMultiplier);
if (filament::Variant::filterVariantVertex(v) == k) {
variants.emplace_back(k, filament::backend::ShaderType::VERTEX);
}
if (filament::Variant::filterVariantFragment(v) == k) {
variants.emplace_back(k, filament::backend::ShaderType::FRAGMENT);
}
}
return variants;
}
std::vector<Variant> determineVariants(uint8_t variantFilter, bool isLit, bool shadowMultiplier);
} // namespace filamat

View File

@@ -196,8 +196,10 @@ Package PostprocessMaterialBuilder::build() {
// Emit GLSL chunks
if (!glslEntries.empty()) {
container.addChild<filamat::DictionaryTextChunk>(std::move(glslDictionary), ChunkType::DictionaryGlsl);
container.addChild<MaterialTextChunk>(std::move(glslEntries), std::move(glslDictionary), ChunkType::MaterialGlsl);
const auto& dictionaryChunk = container.addChild<filamat::DictionaryTextChunk>(
std::move(glslDictionary), ChunkType::DictionaryGlsl);
container.addChild<MaterialTextChunk>(std::move(glslEntries),
dictionaryChunk.getDictionary(), ChunkType::MaterialGlsl);
}
#ifndef FILAMAT_LITE
@@ -209,14 +211,15 @@ Package PostprocessMaterialBuilder::build() {
// Emit Metal chunks
if (!metalEntries.empty()) {
container.addChild<filamat::DictionaryTextChunk>(std::move(metalDictionary), ChunkType::DictionaryMetal);
container.addChild<MaterialTextChunk>(std::move(metalEntries), std::move(metalDictionary), ChunkType::MaterialMetal);
const auto& dictionaryChunk = container.addChild<filamat::DictionaryTextChunk>(
std::move(metalDictionary), ChunkType::DictionaryMetal);
container.addChild<MaterialTextChunk>(std::move(metalEntries),
dictionaryChunk.getDictionary(), ChunkType::MaterialMetal);
}
#endif
// Flatten all chunks in the container into a Package.
size_t packageSize = container.getSize();
Package package(packageSize);
Package package(container.getSize());
Flattener f(package);
container.flatten(f);
package.setValid(!errorOccured);

View File

@@ -37,14 +37,16 @@ public:
template <typename T,
std::enable_if_t<std::is_base_of<Chunk, T>::value, int> = 0,
typename... Args>
void addChild(Args&&... args) {
mChildren.emplace_back(new T(std::forward<Args>(args)...));
const T& addChild(Args&&... args) {
T* chunk = new T(std::forward<Args>(args)...);
mChildren.emplace_back(chunk);
return *chunk;
}
// Helper method to add a SimpleFieldChunk to this ChunkContainer.
template <typename T, typename... Args>
void addSimpleChild(Args&&... args) {
addChild<SimpleFieldChunk<T>>(std::forward<Args>(args)...);
const SimpleFieldChunk<T>& addSimpleChild(Args&&... args) {
return addChild<SimpleFieldChunk<T>>(std::forward<Args>(args)...);
}
size_t getSize() const;

View File

@@ -18,7 +18,7 @@
namespace filamat {
DictionaryTextChunk::DictionaryTextChunk(const LineDictionary& dictionary, ChunkType chunkType) :
DictionaryTextChunk::DictionaryTextChunk(LineDictionary&& dictionary, ChunkType chunkType) :
Chunk(chunkType), mDictionary(dictionary) {
}

View File

@@ -28,13 +28,15 @@ namespace filamat {
class DictionaryTextChunk final : public Chunk {
public:
DictionaryTextChunk(const LineDictionary& dictionary, ChunkType chunkType);
DictionaryTextChunk(LineDictionary&& dictionary, ChunkType chunkType);
~DictionaryTextChunk() = default;
const LineDictionary& getDictionary() const noexcept { return mDictionary; }
private:
void flatten(Flattener& f) override;
LineDictionary mDictionary;
const LineDictionary mDictionary;
};
} // namespace filamat

View File

@@ -49,7 +49,7 @@ private:
std::unordered_map<std::string, size_t> mLineIndices;
std::vector<std::string> mStrings;
size_t mStorageSize;
size_t mStorageSize = 0;
};
} // namespace filamat

View File

@@ -46,7 +46,7 @@ private:
std::vector<ShaderAttribute> mDuplicateMap;
const std::vector<TextEntry> mEntries;
const LineDictionary mDictionary;
const LineDictionary& mDictionary;
};
} // namespace filamat