The size of godot_variant seems wrong on 32bit builds

[acmepjz]

I need to transfer dozens of arguments of various types in an array from gdscript to gdnative. I found that the Color is transferred incorrectly. Example code based on gdnative c example:

gdscript:

extends Control

# load the Simple library
onready var data = preload("res://bin/simple.gdns").new()

func _on_Button_pressed():
    $Label.text = "Data = " + data.get_data([Color(1, 1, 1, 1), "unused"])

cpp:

#include <stdio.h>

godot_variant simple_get_data(godot_object *p_instance, void *p_method_data,
        void *p_user_data, int p_num_args, godot_variant **p_args) {
    godot_string data;
    godot_variant ret;

    godot_array arr = api->godot_variant_as_array(p_args[0]);
    godot_variant v = api->godot_array_get(&arr, 0);
    godot_color clr = api->godot_variant_as_color(&v);

    char s[1024];
    sprintf(s, "(%g,%g,%g,%g)",
        api->godot_color_get_r(&clr),
        api->godot_color_get_g(&clr),
        api->godot_color_get_b(&clr),
        api->godot_color_get_a(&clr));

    api->godot_string_new(&data);
    api->godot_string_parse_utf8(&data, s);
    api->godot_variant_new_string(&ret, &data);
    api->godot_string_destroy(&data);

    return ret;
}

It's expected to get (1,1,1,1), but it actually returns (1,1,1,random number) on Win32 platform.

I suspect that the size of godot_variant is wrong in https://github.com/GodotNativeTools/godot_headers/blob/master/gdnative/variant.h

#define GODOT_VARIANT_SIZE (16 + sizeof(void *))

#ifndef GODOT_CORE_API_GODOT_VARIANT_TYPE_DEFINED
#define GODOT_CORE_API_GODOT_VARIANT_TYPE_DEFINED
typedef struct {
    uint8_t _dont_touch_that[GODOT_VARIANT_SIZE];
} godot_variant;
#endif

while in https://github.com/godotengine/godot/blob/3.2/core/variant.h it has members

    // Variant takes 20 bytes when real_t is float, and 36 if double
    // it only allocates extra memory for aabb/matrix.

    Type type;

    // ...

    union {
        bool _bool;
        int64_t _int;
        double _real;
        Transform2D *_transform2d;
        ::AABB *_aabb;
        Basis *_basis;
        Transform *_transform;
        void *_ptr; //generic pointer
        uint8_t _mem[sizeof(ObjData) > (sizeof(real_t) * 4) ? sizeof(ObjData) : (sizeof(real_t) * 4)];
    } _data GCC_ALIGNED_8;

I notice the GCC_ALIGNED_8, which implies that godot_variant has size 24, but not 20, even on 32bit platforms.

I also inspected the actual memory layout of godot_variant (if you pass a separate Color as the second argument):

    uint32_t *ptr = (uint32_t*)p_args[1];
    for (int i = 0; i < 6; i++) printf("%08X\n", ptr[i]);

and it seems that the _data actually start at offset 8 even on 32bit platforms.

Anyone can confirm this?

[aaronfranke]

Likely caused by https://github.com/godotengine/godot/pull/26069

[acmepjz]

From the release notes of Godot 3.2.2 it looks like that this is already fixed in Godot's engine code ( https://github.com/godotengine/godot/pull/38799 especially https://github.com/godotengine/godot/pull/38799/files#diff-e8fa10e5ca21c9919f3ebcb6742941c4 ). So this problem should be fixed by syncing the code with latest Godot's engine code?

[BastiaanOlij]

Can we confirm this is fixed so we can close this issue?

[akien-mga]

The headers are up-to-date with 3.3.3-stable so this should be fixed indeed. Please comment if you can still reproduce it.