Skip to content

A
angle
Commit 98c5ff6b by Jamie Madill Committed by Commit Bot
Options

BinaryStream: Preserve 64-bit integer data.

Previously the code would truncate 64-bit data to fit in 32-bits. This ran into a serialization bug when expanding a 64-bit mask. The new blend state masks for extended range were out of range for the 32-bit promotion that was happening before. Also refactors how we capture bools and enums to be more consistent. size_t is now correctly saved and loaded as 64-bits. Bug: angleproject:5247 Change-Id: I452a98c1b0add4c0cf45493032e9310e7d8321b2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2497561Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent 5cbf54da
Showing with 55 additions and 38 deletions
src/libANGLE/BinaryStream.h
...@@ -19,6 +19,14 @@ ...@@ -19,6 +19,14 @@
namespace gl namespace gl
{ {
template <typename IntT>
struct PromotedIntegerType
{
using type = typename std::conditional<
std::is_signed<IntT>::value,
typename std::conditional<sizeof(IntT) <= 4, int32_t, int64_t>::type,
typename std::conditional<sizeof(IntT) <= 4, uint32_t, uint64_t>::type>::type;
};
class BinaryInputStream : angle::NonCopyable class BinaryInputStream : angle::NonCopyable
{ {
...@@ -35,8 +43,11 @@ class BinaryInputStream : angle::NonCopyable ...@@ -35,8 +43,11 @@ class BinaryInputStream : angle::NonCopyable
template <class IntT> template <class IntT>
IntT readInt() IntT readInt()
{ {
int value = 0; static_assert(!std::is_same<bool, std::remove_cv<IntT>()>(), "Use readBool");
using PromotedIntT = typename PromotedIntegerType<IntT>::type;
PromotedIntT value = 0;
read(&value); read(&value);
ASSERT(angle::IsValueInRangeForNumericType<IntT>(value));
return static_cast<IntT>(value); return static_cast<IntT>(value);
} }
...@@ -49,8 +60,8 @@ class BinaryInputStream : angle::NonCopyable ...@@ -49,8 +60,8 @@ class BinaryInputStream : angle::NonCopyable
template <class IntT, class VectorElementT> template <class IntT, class VectorElementT>
void readIntVector(std::vector<VectorElementT> *param) void readIntVector(std::vector<VectorElementT> *param)
{ {
unsigned int size = readInt<unsigned int>(); size_t size = readInt<size_t>();
for (unsigned int index = 0; index < size; ++index) for (size_t index = 0; index < size; ++index)
{ {
param->push_back(readInt<IntT>()); param->push_back(readInt<IntT>());
} }
...@@ -186,8 +197,8 @@ class BinaryOutputStream : angle::NonCopyable ...@@ -186,8 +197,8 @@ class BinaryOutputStream : angle::NonCopyable
template <class IntT> template <class IntT>
void writeInt(IntT param) void writeInt(IntT param)
{ {
using PromotedIntT = static_assert(!std::is_same<bool, std::remove_cv<IntT>()>(), "Use writeBool");
typename std::conditional<std::is_signed<IntT>::value, int, unsigned>::type; using PromotedIntT = typename PromotedIntegerType<IntT>::type;
ASSERT(angle::IsValueInRangeForNumericType<PromotedIntT>(param)); ASSERT(angle::IsValueInRangeForNumericType<PromotedIntT>(param));
PromotedIntT intValue = static_cast<PromotedIntT>(param); PromotedIntT intValue = static_cast<PromotedIntT>(param);
write(&intValue, 1); write(&intValue, 1);
...@@ -232,6 +243,12 @@ class BinaryOutputStream : angle::NonCopyable ...@@ -232,6 +243,12 @@ class BinaryOutputStream : angle::NonCopyable
void writeBytes(const unsigned char *bytes, size_t count) { write(bytes, count); } void writeBytes(const unsigned char *bytes, size_t count) { write(bytes, count); }
void writeBool(bool value)
{
int intValue = value ? 1 : 0;
write(&intValue, 1);
}
size_t length() const { return mData.size(); } size_t length() const { return mData.size(); }
const void *data() const { return mData.size() ? &mData[0] : nullptr; } const void *data() const { return mData.size() ? &mData[0] : nullptr; }
...@@ -239,8 +256,6 @@ class BinaryOutputStream : angle::NonCopyable ...@@ -239,8 +256,6 @@ class BinaryOutputStream : angle::NonCopyable
const std::vector<uint8_t> &getData() const { return mData; } const std::vector<uint8_t> &getData() const { return mData; }
private: private:
std::vector<uint8_t> mData;
template <typename T> template <typename T>
void write(const T *v, size_t num) void write(const T *v, size_t num)
{ {
...@@ -248,6 +263,8 @@ class BinaryOutputStream : angle::NonCopyable ...@@ -248,6 +263,8 @@ class BinaryOutputStream : angle::NonCopyable
const char *asBytes = reinterpret_cast<const char *>(v); const char *asBytes = reinterpret_cast<const char *>(v);
mData.insert(mData.end(), asBytes, asBytes + num * sizeof(T)); mData.insert(mData.end(), asBytes, asBytes + num * sizeof(T));
} }
std::vector<uint8_t> mData;
}; };
inline BinaryOutputStream::BinaryOutputStream() {} inline BinaryOutputStream::BinaryOutputStream() {}
......
src/libANGLE/ProgramExecutable.cpp
...@@ -139,8 +139,8 @@ void ProgramExecutable::load(gl::BinaryInputStream *stream) ...@@ -139,8 +139,8 @@ void ProgramExecutable::load(gl::BinaryInputStream *stream)
"Too many vertex attribs for mask: All bits of mAttributesTypeMask types and " "Too many vertex attribs for mask: All bits of mAttributesTypeMask types and "
"mask fit into 32 bits each"); "mask fit into 32 bits each");
mAttributesTypeMask = gl::ComponentTypeMask(stream->readInt<uint32_t>()); mAttributesTypeMask = gl::ComponentTypeMask(stream->readInt<uint32_t>());
mAttributesMask = stream->readInt<gl::AttributesMask>(); mAttributesMask = gl::AttributesMask(stream->readInt<uint32_t>());
mActiveAttribLocationsMask = stream->readInt<gl::AttributesMask>(); mActiveAttribLocationsMask = gl::AttributesMask(stream->readInt<uint32_t>());
mMaxActiveAttribLocation = stream->readInt<unsigned int>(); mMaxActiveAttribLocation = stream->readInt<unsigned int>();
mLinkedGraphicsShaderStages = ShaderBitSet(stream->readInt<uint8_t>()); mLinkedGraphicsShaderStages = ShaderBitSet(stream->readInt<uint8_t>());
...@@ -163,25 +163,25 @@ void ProgramExecutable::save(gl::BinaryOutputStream *stream) const ...@@ -163,25 +163,25 @@ void ProgramExecutable::save(gl::BinaryOutputStream *stream) const
{ {
static_assert(MAX_VERTEX_ATTRIBS * 2 <= sizeof(uint32_t) * 8, static_assert(MAX_VERTEX_ATTRIBS * 2 <= sizeof(uint32_t) * 8,
"All bits of mAttributesTypeMask types and mask fit into 32 bits each"); "All bits of mAttributesTypeMask types and mask fit into 32 bits each");
stream->writeInt(static_cast<int>(mAttributesTypeMask.to_ulong())); stream->writeInt(static_cast<uint32_t>(mAttributesTypeMask.to_ulong()));
stream->writeInt(static_cast<int>(mAttributesMask.to_ulong())); stream->writeInt(static_cast<uint32_t>(mAttributesMask.to_ulong()));
stream->writeInt(mActiveAttribLocationsMask.to_ulong()); stream->writeInt(static_cast<uint32_t>(mActiveAttribLocationsMask.to_ulong()));
stream->writeInt(mMaxActiveAttribLocation); stream->writeInt(mMaxActiveAttribLocation);
stream->writeInt(mLinkedGraphicsShaderStages.bits()); stream->writeInt(mLinkedGraphicsShaderStages.bits());
stream->writeInt(mLinkedComputeShaderStages.bits()); stream->writeInt(mLinkedComputeShaderStages.bits());
stream->writeInt(static_cast<bool>(mIsCompute)); stream->writeBool(mIsCompute);
stream->writeInt(static_cast<bool>(mPipelineHasGraphicsUniformBuffers)); stream->writeBool(mPipelineHasGraphicsUniformBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasComputeUniformBuffers)); stream->writeBool(mPipelineHasComputeUniformBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasGraphicsStorageBuffers)); stream->writeBool(mPipelineHasGraphicsStorageBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasComputeStorageBuffers)); stream->writeBool(mPipelineHasComputeStorageBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasGraphicsAtomicCounterBuffers)); stream->writeBool(mPipelineHasGraphicsAtomicCounterBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasComputeAtomicCounterBuffers)); stream->writeBool(mPipelineHasComputeAtomicCounterBuffers);
stream->writeInt(static_cast<bool>(mPipelineHasGraphicsDefaultUniforms)); stream->writeBool(mPipelineHasGraphicsDefaultUniforms);
stream->writeInt(static_cast<bool>(mPipelineHasComputeDefaultUniforms)); stream->writeBool(mPipelineHasComputeDefaultUniforms);
stream->writeInt(static_cast<bool>(mPipelineHasGraphicsTextures)); stream->writeBool(mPipelineHasGraphicsTextures);
stream->writeInt(static_cast<bool>(mPipelineHasComputeTextures)); stream->writeBool(mPipelineHasComputeTextures);
} }
int ProgramExecutable::getInfoLogLength() const int ProgramExecutable::getInfoLogLength() const
......
src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp
...@@ -260,23 +260,23 @@ void ProgramExecutableVk::save(gl::BinaryOutputStream *stream) ...@@ -260,23 +260,23 @@ void ProgramExecutableVk::save(gl::BinaryOutputStream *stream)
{ {
for (gl::ShaderType shaderType : gl::AllShaderTypes()) for (gl::ShaderType shaderType : gl::AllShaderTypes())
{ {
stream->writeInt<size_t>(mVariableInfoMap[shaderType].size()); stream->writeInt(mVariableInfoMap[shaderType].size());
for (const auto &it : mVariableInfoMap[shaderType]) for (const auto &it : mVariableInfoMap[shaderType])
{ {
stream->writeString(it.first); stream->writeString(it.first);
stream->writeInt<uint32_t>(it.second.descriptorSet); stream->writeInt(it.second.descriptorSet);
stream->writeInt<uint32_t>(it.second.binding); stream->writeInt(it.second.binding);
stream->writeInt<uint32_t>(it.second.location); stream->writeInt(it.second.location);
stream->writeInt<uint32_t>(it.second.component); stream->writeInt(it.second.component);
// PackedEnumBitSet uses uint8_t // PackedEnumBitSet uses uint8_t
stream->writeInt<uint8_t>(it.second.activeStages.bits()); stream->writeInt(it.second.activeStages.bits());
stream->writeInt<uint32_t>(it.second.xfbBuffer); stream->writeInt(it.second.xfbBuffer);
stream->writeInt<uint32_t>(it.second.xfbOffset); stream->writeInt(it.second.xfbOffset);
stream->writeInt<uint32_t>(it.second.xfbStride); stream->writeInt(it.second.xfbStride);
stream->writeInt<uint8_t>(it.second.useRelaxedPrecision); stream->writeBool(it.second.useRelaxedPrecision);
stream->writeInt<uint8_t>(it.second.varyingIsOutput); stream->writeBool(it.second.varyingIsOutput);
stream->writeInt<uint8_t>(it.second.attributeComponentCount); stream->writeInt(it.second.attributeComponentCount);
stream->writeInt<uint8_t>(it.second.attributeLocationCount); stream->writeInt(it.second.attributeLocationCount);
} }
} }
} }
......
src/libANGLE/renderer/vulkan/ProgramVk.cpp
...@@ -212,7 +212,7 @@ void ProgramVk::save(const gl::Context *context, gl::BinaryOutputStream *stream) ...@@ -212,7 +212,7 @@ void ProgramVk::save(const gl::Context *context, gl::BinaryOutputStream *stream)
for (gl::ShaderType shaderType : gl::AllShaderTypes()) for (gl::ShaderType shaderType : gl::AllShaderTypes())
{ {
const size_t uniformCount = mDefaultUniformBlocks[shaderType].uniformLayout.size(); const size_t uniformCount = mDefaultUniformBlocks[shaderType].uniformLayout.size();
stream->writeInt<size_t>(uniformCount); stream->writeInt(uniformCount);
for (unsigned int uniformIndex = 0; uniformIndex < uniformCount; ++uniformIndex) for (unsigned int uniformIndex = 0; uniformIndex < uniformCount; ++uniformIndex)
{ {
sh::BlockMemberInfo &blockInfo = sh::BlockMemberInfo &blockInfo =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment