Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller Scene] Convert vertex positions to match Impeller's clip space orientation #38174

Merged
merged 2 commits into from
Dec 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions impeller/scene/importer/importer_gltf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ namespace impeller {
namespace scene {
namespace importer {

static const std::map<std::string, VerticesBuilder::Attribute> 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<std::string, VerticesBuilder::AttributeType> 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<size_t>(index) < size;
Expand Down
2 changes: 1 addition & 1 deletion impeller/scene/importer/importer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
153 changes: 93 additions & 60 deletions impeller/scene/importer/vertices_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <limits>
#include <type_traits>

#include "flutter/fml/logging.h"
#include "impeller/scene/importer/conversions.h"
#include "impeller/scene/importer/scene_flatbuffers.h"

Expand All @@ -17,29 +18,6 @@ namespace importer {

VerticesBuilder::VerticesBuilder() = default;

std::map<VerticesBuilder::Attribute, VerticesBuilder::AttributeProperties>
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<fb::Vertex>& vertices) const {
vertices.resize(0);
for (auto& v : vertices_) {
Expand All @@ -49,64 +27,119 @@ void VerticesBuilder::WriteFBVertices(std::vector<fb::Vertex>& 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 <typename SourceType>
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<SourceType>
? std::numeric_limits<SourceType>::max()
: 1;
for (size_t i = 0; i < component_count; i++) {
const SourceType* s = reinterpret_cast<const SourceType*>(source) + i;
Scalar v = static_cast<Scalar>(*s) / static_cast<Scalar>(divisor);
Scalar* dest = reinterpret_cast<Scalar*>(destination) + i;
*dest = v;
const SourceType* s = reinterpret_cast<const SourceType*>(source) + index;
return static_cast<Scalar>(*s) / static_cast<Scalar>(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::AttributeType, VerticesBuilder::AttributeProperties>
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<VerticesBuilder::ComponentType,
VerticesBuilder::ComponentProperties>
kComponentTypes = {
{VerticesBuilder::ComponentType::kSignedByte,
WriteComponentsAsScalars<int8_t>},
{.size_bytes = sizeof(int8_t),
.convert_proc = ToNormalizedScalar<int8_t>}},
{VerticesBuilder::ComponentType::kUnsignedByte,
WriteComponentsAsScalars<uint8_t>},
{.size_bytes = sizeof(int8_t),
.convert_proc = ToNormalizedScalar<uint8_t>}},
{VerticesBuilder::ComponentType::kSignedShort,
WriteComponentsAsScalars<int16_t>},
{.size_bytes = sizeof(int16_t),
.convert_proc = ToNormalizedScalar<int16_t>}},
{VerticesBuilder::ComponentType::kUnsignedShort,
WriteComponentsAsScalars<uint16_t>},
{.size_bytes = sizeof(int16_t),
.convert_proc = ToNormalizedScalar<uint16_t>}},
{VerticesBuilder::ComponentType::kSignedInt,
WriteComponentsAsScalars<int32_t>},
{.size_bytes = sizeof(int32_t),
.convert_proc = ToNormalizedScalar<int32_t>}},
{VerticesBuilder::ComponentType::kUnsignedInt,
WriteComponentsAsScalars<uint32_t>},
{.size_bytes = sizeof(int32_t),
.convert_proc = ToNormalizedScalar<uint32_t>}},
{VerticesBuilder::ComponentType::kFloat,
WriteComponentsAsScalars<float>},
{.size_bytes = sizeof(float),
.convert_proc = ToNormalizedScalar<float>}},
};

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<const char*>(buffer_start) + stride_bytes * i;
char* destination =
reinterpret_cast<char*>(&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<const uint8_t*>(buffer_start) +
attribute_stride_bytes * i;
uint8_t* destination = reinterpret_cast<uint8_t*>(&vertices_.data()[i]) +
attribute_props.offset_bytes;

writer(destination, source, properties.component_count);
attribute_props.write_proc(reinterpret_cast<Scalar*>(destination), source,
component_props, attribute_props);
}
}

Expand Down
52 changes: 33 additions & 19 deletions impeller/scene/importer/vertices_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ namespace importer {

class VerticesBuilder {
public:
enum class Attribute {
kPosition,
kNormal,
kTangent,
kTextureCoords,
kColor,
};

enum class ComponentType {
kSignedByte = 5120,
kUnsignedByte,
Expand All @@ -35,26 +27,48 @@ class VerticesBuilder {
kFloat,
};

VerticesBuilder();

void WriteFBVertices(std::vector<fb::Vertex>& 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<Scalar(const void* source, size_t byte_offset)>;
struct ComponentProperties {
size_t size_bytes = 0;
ComponentConverter convert_proc;
};

private:
struct AttributeProperties;
using AttributeWriter =
std::function<void(Scalar* destination,
const void* source,
const ComponentProperties& component_props,
const AttributeProperties& attribute_props)>;
struct AttributeProperties {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this, but, can we zero these out by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I forget to do this a lot, I wonder if there's a clang-format lint that can help.

size_t offset_bytes;
size_t size_bytes;
size_t component_count;
AttributeWriter write_proc;
};

static std::map<VerticesBuilder::Attribute,
VerticesBuilder();

void WriteFBVertices(std::vector<fb::Vertex>& 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<VerticesBuilder::AttributeType,
VerticesBuilder::AttributeProperties>
kAttributes;
kAttributeTypes;

struct Vertex {
Vector3 position;
Expand Down