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

Constructors for union structs #304

Closed
serenity4 opened this issue Jun 27, 2021 · 8 comments
Closed

Constructors for union structs #304

serenity4 opened this issue Jun 27, 2021 · 8 comments

Comments

@serenity4
Copy link
Contributor

The current master generates union structs like so:

struct VkClearColorValue
    data::NTuple{16, UInt8} # can hold homogeneous 4-tuples with type uint32, int32 or float32
end

where the data field is reinterpreted when accessed to match C unions.

As mentioned in this comment, because union structs hold raw bytes, it's not so convenient to instantiate them so we could define a constructor for union structs that help populate their bytes. We could generate something very similar to the snippet in the comment:

const __U_VkClearColorValue  = Union{NTuple{4,Float32},NTuple{4,Int32},NTuple{4,UInt32}}
VkClearColorValue(data::__U_VkClearColorValue) = VkClearColorValue(reinterpret(NTuple{16, UInt8}, [data])[])

So I am thinking of extending this method to emit the constructor method along with the getproperty/setproperty! methods. Would you accept a PR for adding this feature?

@Gnimuc
Copy link
Member

Gnimuc commented Jun 27, 2021

I think this is a special case where all 3 fields have the same size, and the size is equal to the size of the union.

struct VkClearColorValue
    data::NTuple{16, UInt8}
end

function Base.getproperty(x::Ptr{VkClearColorValue}, f::Symbol)
    f === :float32 && return Ptr{NTuple{4, Cfloat}}(x + 0)
    f === :int32 && return Ptr{NTuple{4, Int32}}(x + 0)
    f === :uint32 && return Ptr{NTuple{4, UInt32}}(x + 0)
    return getfield(x, f)
end

@Gnimuc
Copy link
Member

Gnimuc commented Jun 27, 2021

By using the current generated field access methods, the simplest way to init a VkClearColorValue on the Julia side would be:

julia> x = Base.unsafe_convert(Ptr{VkClearColorValue}, Ref{VkClearColorValue}())
Ptr{VkClearColorValue} @0x000000013c2db190

julia> x.float32 = NTuple{4,Cfloat}((1,2,3,4))
(1.0f0, 2.0f0, 3.0f0, 4.0f0)

julia> unsafe_load(x)
VkClearColorValue((0x00, 0x00, 0x80, 0x3f, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x40, 0x40, 0x00, 0x00, 0x80, 0x40))

julia> unsafe_load(x.float32)
(1.0f0, 2.0f0, 3.0f0, 4.0f0)

@serenity4
Copy link
Contributor Author

Right, we could take the example of VkClearValue instead, which should be more general:

struct VkClearValue
    data::NTuple{16, UInt8}
end

function Base.getproperty(x::Ptr{VkClearValue}, f::Symbol)
    f === :color && return Ptr{VkClearColorValue}(x + 0)
    f === :depthStencil && return Ptr{VkClearDepthStencilValue}(x + 0)
    return getfield(x, f)
end

There, the two fields are 8 and 16 bytes respectively. Although to be general we should probably take into account that some fields may start at different offsets.

Instead of the previously mentioned constructor we could generate a pointer to the provided data, reinterpret it with setproperty!(::Ptr, ...) and dereference it just like in your snippet above:

const __U_VkClearValue  = Union{VkClearColorValue,VkClearDepthStencilValue}
function VkClearValue(data::__U_VkClearValue)
    ref = Ref{VkClearValue}()
    x = Base.unsafe_convert(Ptr{VkClearValue}, ref)
    if data isa VkClearColorValue
        x.color = data
    elseif data isa VkClearDepthStencilValue
        x.depthStencil = data
    end
    GC.@preserve ref unsafe_load(x)
end

@Gnimuc
Copy link
Member

Gnimuc commented Jun 27, 2021

yeah, that sounds like a good solution.

@serenity4
Copy link
Contributor Author

Or maybe we could just dereference ref in the function below instead of the unsafe_load operation? Since they both should point to the same memory.

@Gnimuc
Copy link
Member

Gnimuc commented Jun 27, 2021

Yep. ref[] should be exactly unsafe_load(x).

@serenity4
Copy link
Contributor Author

serenity4 commented Jun 27, 2021

Maybe there is a possibility for a small refactor in the currently generated code then my bad, it uses fptr which is not ptr.

function Base.getproperty(x::VkClearValue, f::Symbol)
    r = Ref{VkClearValue}(x)
    ptr = Base.unsafe_convert(Ptr{VkClearValue}, r)
    fptr = getproperty(ptr, f)
    GC.@preserve r unsafe_load(fptr) # change to r[]
end

@serenity4
Copy link
Contributor Author

Implemented in #305.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants