Vector of bit-packed structs not measuring correctly?

[Net5F]

Hello, me again with a measurement problem. This time, I've learned my lesson and boiled it down to a simple example before bringing it here :D

I have the following code, where an EntityUpdate struct contains a vector of Input structs. Input contains an array that gets bit-packed on serialization.

#include "bitsery/bitsery.h"
#include "bitsery/adapter/buffer.h"
#include "bitsery/adapter/stream.h"
#include "bitsery/adapter/measure_size.h"
#include "bitsery/traits/vector.h"
#include "bitsery/ext/value_range.h"

struct Input {
public:
    enum Type : uint8_t { XUp, XDown, YUp, YDown, ZUp, ZDown, NumTypes, None };
    enum State : uint8_t { Released = 0, Pressed = 1 };

    typedef std::array<State, Type::NumTypes> StateArr;
    StateArr inputStates{};
};

struct EntityUpdate {
    std::vector<Input> inputs;
};

template<typename S>
void serialize(S& serializer, EntityUpdate& entityUpdate)
{
    serializer.container(entityUpdate.inputs,
                         static_cast<std::size_t>(1000));
}

template<typename S>
void serialize(S& serializer, Input& input)
{
    serializer.enableBitPacking([&input](typename S::BPEnabledType& sbp) {
        sbp.container(input.inputStates, [](typename S::BPEnabledType& sbp,
                                            Input::State& inputState) {
            constexpr bitsery::ext::ValueRange<Input::State> range{
                Input::State::Released, Input::State::Pressed};
            sbp.ext(inputState, range);
        });
    });
}

using OutputAdapter = bitsery::OutputBufferAdapter<std::vector<uint8_t>>;

int main()
{
    EntityUpdate entityUpdate{};
    for (unsigned int i = 0; i < 4; ++i) {
        entityUpdate.inputs.push_back(Input{});
    }

    // Measure the size.
    std::size_t measuredSize{bitsery::quickSerialization(
        bitsery::MeasureSize{}, entityUpdate)};

    // Serialize and get the real size.
    std::vector<uint8_t> buffer{};
    std::size_t finalSize{bitsery::quickSerialization<OutputAdapter>(
        buffer, entityUpdate)};

    assert(measuredSize == finalSize);

    return 0;
}

The issue is, when the array is filled with more than 4 inputs, I'm getting different values between the measured size and the serialized size. In the example, I'm getting 4B for measured and 5B for final. 5B makes sense, since each Input can fit in 1B and the array will use 1B for its size.

Am I doing something incorrectly, or is there a bug?

[fraillt]

Thanks for example!
It's not a bug, but looks like a bug, as there's one subtle thing here.
The problem is that, MeasureSize adapter is already BitPackingEnabled = true by default, which means that serializer.enableBitPacking is no-op for it.
On the other hand, OutputBufferAdapter is not BitPackingEnabled, so when you call serializer.enableBitPacking, it will wrap your existing OutputBufferAdapter into a OutputAdapterBitPackingWrapper, and once you exit from this scope, it will be destroyed and will align data to full byte!

The solution would be to change MeasureSize by making it not BitPackingEnabled

    template<typename Config>
    class BasicMeasureSize {
    public:

        static constexpr bool BitPackingEnabled = false;
        using TConfig = Config;
        using TValue = uint8_t;

Or, you can also try enableBitPacking in outer scope, if it makes sense to you

template<typename S>
void serialize(S& serializer, EntityUpdate& entityUpdate)
{
    serializer.enableBitPacking([&entityUpdate](typename S::BPEnabledType& sbp) {
        sbp.container(entityUpdate.inputs,
                         static_cast<std::size_t>(1000));
    });
}

Just for the record.
The reason why MeasureSize is BitPackingEnabled by default is that it provides efficient writeBits implementation (simply increment written bits), otherwise it would be wrapped in OutputAdapterBitPackingWrapper which actually does all the bit shifting and writing.

[Net5F]

That makes sense, thanks for the answer!
Is it true that, if I want Input to be bit packed, the latter approach (adding enableBitPacking to the outer scope) would be the way to go? Is there any downside to doing so?
If I have further outer layers (say, another struct that has a vector of EntityUpdate), would those also need to have that member similarly wrapped with enableBitPacking?

[fraillt]

in this particular example, it makes sense to add enableBitPacking in outer scope, because it allows writing inputs tightly, without aligning after each container element, and you save from 1byte (5 to 4). However if you would also write more data that doesn't require bit packing, it might be less efficient, because data might not be aligned to whole byte. (although you can call align method in enableBitPacking scope, to solve this).
So it's up to you to decide, what is more important:

• speed (enableBitPacking where required)
• space (pay extra CPU cycles, when whole-byte data is written in non-aligned buffer)

Btw, I'm planning to change MeasureSize, so it is not EnableBitPacking by default, as it definitely confuses users.

[Net5F]

I think I'm starting to understand. So if I had something like the following structs:

struct Input {
public:
    enum Type : uint8_t { XUp, XDown, YUp, YDown, ZUp, ZDown, NumTypes, None };
    enum State : uint8_t { Released = 0, Pressed = 1 };

    // trying to pack this into 1B
    typedef std::array<State, Type::NumTypes> StateArr;
    StateArr inputStates{};

    // new
    std::string foo{""};
};

struct EntityUpdate {
    // new
    unsigned int bar{0};

    std::vector<Input> inputs;
};

Would I be able to get Input::inputStates to be packed properly while letting the other fields be as fast as normal by using align as so?

template<typename S>
void serialize(S& serializer, Input& input)
{
    serializer.enableBitPacking([&input](typename S::BPEnabledType& sbp) {
        sbp.container(input.inputStates, [](typename S::BPEnabledType& sbp,
                                            Input::State& inputState) {
            constexpr bitsery::ext::ValueRange<Input::State> range{
                Input::State::Released, Input::State::Pressed};
            sbp.ext(inputState, range);
        });
    });

    serializer.align();

    serializer.text1b(foo, 100);
}

template<typename S>
void serialize(S& serializer, EntityUpdate& entityUpdate)
{
    serializer.value4b(entityUpdate.bar);

    serializer.enableBitPacking([&entityUpdate](typename S::BPEnabledType& sbp) {
        sbp.container(entityUpdate.inputs,
                         static_cast<std::size_t>(1000));
    });
}

[fraillt]

Yes, that's right.
Also, if you use void serialize(S& serializer, Input& input) only from bit packing enabled scope, then you don't need to add enableBitPacking here, as it's already enabled, by the caller.

// only works if called with BPEnabled serializer/deserializer
template<typename S>
void serialize(S& serializer, Input& input)
{
    serializer.container(input.inputStates, [](S& s,
                                        Input::State& inputState) {
        constexpr bitsery::ext::ValueRange<Input::State> range{
            Input::State::Released, Input::State::Pressed};
        s.ext(inputState, range);
    });

    serializer.adapter().align();

    serializer.text1b(input.foo, 100);
}

template<typename S>
void serialize(S& serializer, EntityUpdate& entityUpdate)
{
    serializer.value4b(entityUpdate.bar);

    serializer.enableBitPacking([&entityUpdate](typename S::BPEnabledType& sbp) {
        sbp.container(entityUpdate.inputs,
                         static_cast<std::size_t>(1000));
    });
}

[Net5F]

Ah, good to know. Thank you very much, that clears up my questions! :D