From a78990a4ee0d3295b1697b502351064ea44ad0ba Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 9 Dec 2022 16:33:13 -0800 Subject: [PATCH 1/2] [Impeller Scene] Convert vertex positions to the correct space --- impeller/scene/importer/importer_gltf.cc | 13 +- impeller/scene/importer/importer_unittests.cc | 2 +- impeller/scene/importer/vertices_builder.cc | 153 +++++++++++------- impeller/scene/importer/vertices_builder.h | 52 +++--- 4 files changed, 133 insertions(+), 87 deletions(-) diff --git a/impeller/scene/importer/importer_gltf.cc b/impeller/scene/importer/importer_gltf.cc index 96149cf0b34d6..ff1969350416a 100644 --- a/impeller/scene/importer/importer_gltf.cc +++ b/impeller/scene/importer/importer_gltf.cc @@ -22,13 +22,12 @@ namespace impeller { namespace scene { namespace importer { -static const std::map kAttributes = { - {"POSITION", VerticesBuilder::Attribute::kPosition}, - {"NORMAL", VerticesBuilder::Attribute::kNormal}, - {"TANGENT", VerticesBuilder::Attribute::kTangent}, - {"TEXCOORD_0", VerticesBuilder::Attribute::kTextureCoords}, - {"COLOR_0", VerticesBuilder::Attribute::kColor}, -}; +static const std::map kAttributes = + {{"POSITION", VerticesBuilder::AttributeType::kPosition}, + {"NORMAL", VerticesBuilder::AttributeType::kNormal}, + {"TANGENT", VerticesBuilder::AttributeType::kTangent}, + {"TEXCOORD_0", VerticesBuilder::AttributeType::kTextureCoords}, + {"COLOR_0", VerticesBuilder::AttributeType::kColor}}; static bool WithinRange(int index, size_t size) { return index >= 0 && static_cast(index) < size; diff --git a/impeller/scene/importer/importer_unittests.cc b/impeller/scene/importer/importer_unittests.cc index 652c97ade272a..68f481b588757 100644 --- a/impeller/scene/importer/importer_unittests.cc +++ b/impeller/scene/importer/importer_unittests.cc @@ -38,7 +38,7 @@ TEST(ImporterTest, CanParseGLTF) { auto& vertex = mesh.vertices[0]; Vector3 position = ToVector3(vertex.position()); - ASSERT_VECTOR3_NEAR(position, Vector3(-0.0100185, -0.522907, 0.133178)); + ASSERT_VECTOR3_NEAR(position, Vector3(-0.0100185, -0.522907, -0.133178)); Vector3 normal = ToVector3(vertex.normal()); ASSERT_VECTOR3_NEAR(normal, Vector3(0.556997, -0.810833, 0.179733)); diff --git a/impeller/scene/importer/vertices_builder.cc b/impeller/scene/importer/vertices_builder.cc index d08268df4fdf9..dc606f1614901 100644 --- a/impeller/scene/importer/vertices_builder.cc +++ b/impeller/scene/importer/vertices_builder.cc @@ -8,6 +8,7 @@ #include #include +#include "flutter/fml/logging.h" #include "impeller/scene/importer/conversions.h" #include "impeller/scene/importer/scene_flatbuffers.h" @@ -17,29 +18,6 @@ namespace importer { VerticesBuilder::VerticesBuilder() = default; -std::map - VerticesBuilder::kAttributes = { - {VerticesBuilder::Attribute::kPosition, - {.offset_bytes = offsetof(Vertex, position), - .size_bytes = sizeof(Vertex::position), - .component_count = 3}}, - {VerticesBuilder::Attribute::kNormal, - {.offset_bytes = offsetof(Vertex, normal), - .size_bytes = sizeof(Vertex::normal), - .component_count = 3}}, - {VerticesBuilder::Attribute::kTangent, - {.offset_bytes = offsetof(Vertex, tangent), - .size_bytes = sizeof(Vertex::tangent), - .component_count = 4}}, - {VerticesBuilder::Attribute::kTextureCoords, - {.offset_bytes = offsetof(Vertex, texture_coords), - .size_bytes = sizeof(Vertex::texture_coords), - .component_count = 2}}, - {VerticesBuilder::Attribute::kColor, - {.offset_bytes = offsetof(Vertex, color), - .size_bytes = sizeof(Vertex::color), - .component_count = 4}}}; - void VerticesBuilder::WriteFBVertices(std::vector& vertices) const { vertices.resize(0); for (auto& v : vertices_) { @@ -49,64 +27,119 @@ void VerticesBuilder::WriteFBVertices(std::vector& vertices) const { } } -/// @brief Reads a contiguous sequence of numeric components from `source` and -/// writes them to `destination` as 32bit floats. Signed SourceTypes -/// convert to a range of -1 to 1, and unsigned SourceTypes convert to a -/// range of 0 to 1. +/// @brief Reads a numeric component from `source` and returns a 32bit float. +/// Signed SourceTypes convert to a range of -1 to 1, and unsigned +/// SourceTypes convert to a range of 0 to 1. template -static void WriteComponentsAsScalars(void* destination, - const void* source, - size_t component_count) { +static Scalar ToNormalizedScalar(const void* source, size_t index) { constexpr SourceType divisor = std::is_integral_v ? std::numeric_limits::max() : 1; - for (size_t i = 0; i < component_count; i++) { - const SourceType* s = reinterpret_cast(source) + i; - Scalar v = static_cast(*s) / static_cast(divisor); - Scalar* dest = reinterpret_cast(destination) + i; - *dest = v; + const SourceType* s = reinterpret_cast(source) + index; + return static_cast(*s) / static_cast(divisor); +} + +/// @brief A ComponentWriter which simply converts all of an attribute's +/// components to normalized scalar form. +static void PassthroughAttributeWriter( + Scalar* destination, + const void* source, + const VerticesBuilder::ComponentProperties& component, + const VerticesBuilder::AttributeProperties& attribute) { + FML_DCHECK(attribute.size_bytes == + attribute.component_count * sizeof(Scalar)); + for (size_t component_i = 0; component_i < attribute.component_count; + component_i++) { + *(destination + component_i) = component.convert_proc(source, component_i); } } -static std::map< - VerticesBuilder::ComponentType, - std::function< - void(void* destination, const void* source, size_t component_count)>> - kAttributeWriters = { +/// @brief A ComponentWriter which converts a Vector3 position from +/// right-handed GLTF space to left-handed Impeller space. +static void PositionAttributeWriter( + Scalar* destination, + const void* source, + const VerticesBuilder::ComponentProperties& component, + const VerticesBuilder::AttributeProperties& attribute) { + FML_DCHECK(attribute.component_count == 3); + *(destination + 0) = component.convert_proc(source, 0); + *(destination + 1) = component.convert_proc(source, 1); + *(destination + 2) = -component.convert_proc(source, 2); +} + +std::map + VerticesBuilder::kAttributeTypes = { + {VerticesBuilder::AttributeType::kPosition, + {.offset_bytes = offsetof(Vertex, position), + .size_bytes = sizeof(Vertex::position), + .component_count = 3, + .write_proc = PositionAttributeWriter}}, + {VerticesBuilder::AttributeType::kNormal, + {.offset_bytes = offsetof(Vertex, normal), + .size_bytes = sizeof(Vertex::normal), + .component_count = 3, + .write_proc = PassthroughAttributeWriter}}, + {VerticesBuilder::AttributeType::kTangent, + {.offset_bytes = offsetof(Vertex, tangent), + .size_bytes = sizeof(Vertex::tangent), + .component_count = 4, + .write_proc = PassthroughAttributeWriter}}, + {VerticesBuilder::AttributeType::kTextureCoords, + {.offset_bytes = offsetof(Vertex, texture_coords), + .size_bytes = sizeof(Vertex::texture_coords), + .component_count = 2, + .write_proc = PassthroughAttributeWriter}}, + {VerticesBuilder::AttributeType::kColor, + {.offset_bytes = offsetof(Vertex, color), + .size_bytes = sizeof(Vertex::color), + .component_count = 4, + .write_proc = PassthroughAttributeWriter}}}; + +static std::map + kComponentTypes = { {VerticesBuilder::ComponentType::kSignedByte, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int8_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kUnsignedByte, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int8_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kSignedShort, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int16_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kUnsignedShort, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int16_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kSignedInt, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int32_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kUnsignedInt, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(int32_t), + .convert_proc = ToNormalizedScalar}}, {VerticesBuilder::ComponentType::kFloat, - WriteComponentsAsScalars}, + {.size_bytes = sizeof(float), + .convert_proc = ToNormalizedScalar}}, }; -void VerticesBuilder::SetAttributeFromBuffer(Attribute attribute, +void VerticesBuilder::SetAttributeFromBuffer(AttributeType attribute, ComponentType component_type, const void* buffer_start, - size_t stride_bytes, - size_t count) { - if (count > vertices_.size()) { - vertices_.resize(count, Vertex()); + size_t attribute_stride_bytes, + size_t attribute_count) { + if (attribute_count > vertices_.size()) { + vertices_.resize(attribute_count, Vertex()); } - const auto& properties = kAttributes[attribute]; - const auto& writer = kAttributeWriters[component_type]; - for (size_t i = 0; i < count; i++) { - const char* source = - reinterpret_cast(buffer_start) + stride_bytes * i; - char* destination = - reinterpret_cast(&vertices_.data()[i]) + properties.offset_bytes; + const ComponentProperties& component_props = kComponentTypes[component_type]; + const AttributeProperties& attribute_props = kAttributeTypes[attribute]; + for (size_t i = 0; i < attribute_count; i++) { + const uint8_t* source = reinterpret_cast(buffer_start) + + attribute_stride_bytes * i; + uint8_t* destination = reinterpret_cast(&vertices_.data()[i]) + + attribute_props.offset_bytes; - writer(destination, source, properties.component_count); + attribute_props.write_proc(reinterpret_cast(destination), source, + component_props, attribute_props); } } diff --git a/impeller/scene/importer/vertices_builder.h b/impeller/scene/importer/vertices_builder.h index 1ff3d42894964..bd57a52a535ab 100644 --- a/impeller/scene/importer/vertices_builder.h +++ b/impeller/scene/importer/vertices_builder.h @@ -17,14 +17,6 @@ namespace importer { class VerticesBuilder { public: - enum class Attribute { - kPosition, - kNormal, - kTangent, - kTextureCoords, - kColor, - }; - enum class ComponentType { kSignedByte = 5120, kUnsignedByte, @@ -35,26 +27,48 @@ class VerticesBuilder { kFloat, }; - VerticesBuilder(); - - void WriteFBVertices(std::vector& vertices) const; + enum class AttributeType { + kPosition, + kNormal, + kTangent, + kTextureCoords, + kColor, + }; - void SetAttributeFromBuffer(Attribute attribute, - ComponentType component_type, - const void* buffer_start, - size_t stride_bytes, - size_t count); + using ComponentConverter = + std::function; + struct ComponentProperties { + size_t size_bytes = 0; + ComponentConverter convert_proc; + }; - private: + struct AttributeProperties; + using AttributeWriter = + std::function; struct AttributeProperties { size_t offset_bytes; size_t size_bytes; size_t component_count; + AttributeWriter write_proc; }; - static std::map& vertices) const; + + void SetAttributeFromBuffer(AttributeType attribute, + ComponentType component_type, + const void* buffer_start, + size_t attribute_stride_bytes, + size_t attribute_count); + + private: + static std::map - kAttributes; + kAttributeTypes; struct Vertex { Vector3 position; From 0b0d985c95aa7fcd368dd9f90d98c352fe4d50b9 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 9 Dec 2022 18:41:46 -0800 Subject: [PATCH 2/2] Address comment --- impeller/scene/importer/vertices_builder.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/scene/importer/vertices_builder.h b/impeller/scene/importer/vertices_builder.h index bd57a52a535ab..1f2fb84e6639c 100644 --- a/impeller/scene/importer/vertices_builder.h +++ b/impeller/scene/importer/vertices_builder.h @@ -49,9 +49,9 @@ class VerticesBuilder { const ComponentProperties& component_props, const AttributeProperties& attribute_props)>; struct AttributeProperties { - size_t offset_bytes; - size_t size_bytes; - size_t component_count; + size_t offset_bytes = 0; + size_t size_bytes = 0; + size_t component_count = 0; AttributeWriter write_proc; };