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

sg_blend_state: use sg_color_mask instead of uint8_t #448

Closed
lithiumtoast opened this issue Dec 31, 2020 · 11 comments
Closed

sg_blend_state: use sg_color_mask instead of uint8_t #448

lithiumtoast opened this issue Dec 31, 2020 · 11 comments
Assignees

Comments

@lithiumtoast
Copy link

I found this when creating automatic C# bindings using libclang. sg_blend_state doesn't make use of sg_color_mask but instead uses uint8_t even though the documentation comment says otherwise.

Is there a reason for this?

I found this because the way I parse the .h file using libclang is I start with extern functions and recurse down on all in/out function parameters including when parameters are nested structs.

@lithiumtoast
Copy link
Author

Btw, the current padding for sg_blend_state.color_write_mask is 3 bytes in the struct layout of sg_blend_state so just changing its type to sg_color_mask has no consequence even if sg_color_mask is forced to be 4 bytes.

@floooh
Copy link
Owner

floooh commented Jan 2, 2021

Is there a reason for this?

Yes, the main reason is C++ basically (and IFIR some C compilers also warn in that case in higher warning levels):

https://godbolt.org/z/M6Wros

C doesn't have a proper way to describe such a 'flag group' through its type system unfortunately.

The C standard doesn't really define an underlying type for enums (that's why the sokol-enums all have this FORCE_U32 at the end - but because of this it would actually be better to change the uint8_t to uint32_t in the sg_blend_state struct).

@floooh floooh self-assigned this Jan 2, 2021
@lithiumtoast
Copy link
Author

lithiumtoast commented Jan 3, 2021

That's unfortunate 😒 I won't be able to have 100% automatic generation of the bindings from just the exported functions.

Thanks for the answer!

@floooh
Copy link
Owner

floooh commented Jan 3, 2021

What tool are you using for the binding generation, and how does this tool treat "regular" enum fields versus "bitflag fields"? (e.g. how does it know whether to use a regular C# enum or a FlagAttribute (https://docs.microsoft.com/en-us/dotnet/api/system.flagsattribute?view=net-5.0)?

PS: one thing I'm considering for the Zig bindings is a language-specific config-define. For instance for Zig I will add a "SOKOL_ZIG_WORKAROUNDS" config define which will add some zig-specific workarounds to the public Sokol API declarations.

@lithiumtoast
Copy link
Author

What tool are you using for the binding generation

I'm using a custom tool I wrote that uses libclang to navigate and extract information from the abstract syntax tree of a translation unit and Rosyln (C# compiler API) to generate C# code. The process looks something like this:

  1. Parse a translation unit using clang_parseTranslationUnit.
  2. Loop over each cursor in the translation unit.
  3. If a cursor is a function declaration and it's exported, go to step 4. Note that for creating bindings to higher-level languages it is assumed that sokol_gfx + sokol_app + ... are compiled as one or multiple shared object(s).
  4. For the exported function declaration, look at the return type and it's 0-to-many parameter declarations for their types.
  5. Apply recursion when necessary for each return type and parameter type of an exported function declaration until bottoming out at built-in types such as (uint8_t, unsigned int, etc). Note that recursion can be quite long-winded for the human mind especially with typedefs and nested typedefs.

At every step above we can extract the necessary information to transpile the functions and structs to C# (or any other language) including the exact bit representation of each struct with data alignment and size.

As of today (Janurary 2021) with C#, the data alignment and size are important because any array of structs (AOS) that are nested inside a struct is non-trivial. Take for example sg_shader_stage_desc which uniform_blocks is an array of sg_shader_uniform_block_desc which then uniforms is also an array of sg_shader_uniform_desc. In C#, the built-in type for an array is actually a class. The memory for a class in C# is always allocated and tracked by the garbage collector (GC); this is what it means to be "managed". The memory for a struct in C# is not necessarily managed. However, for games and other real-time systems, using the garbage collector is not really desirable for obvious reasons. So, the structs should be blittable to C structs meaning they have the same bit representation in both languages. This also helps prevent any hits to performance when marshalling data across the native boundary for interoperability. The good news is that C# does allow for C-style arrays for fields of a struct. The only problem is that types that can be used are limited to built-in types like int, float, etc. So, it's not possible to have nested blittable structs in C# without compromise. Maybe C# language will evolve in the future to allow for this. Anyways most people at this stage would probably quit, cave to using the garbage collector, or use a more appropriate language like Zig (I love Zig so far), but I'm stubborn 😉. There exists ugly workarounds to solve this problem but it requires knowing the exact data alignment and size of each struct when trans-compiling the C structs to C#.

Knowing the data alignment and size of all the structs in C is also neat for other reasons such as knowing how exactly the data will fit into cache lines. The information could be used to reorganize the members of a struct to achieve tighter packing. E.g. I programmed my tool to create comments about the padding of each struct member. Here I use the same struct I used in my other issue about struct packing: #295

...
    [StructLayout(LayoutKind.Explicit, Size = 1688, Pack = 8)]
    public struct sg_image_desc
    {
        [FieldOffset(0)] /* size = 4, padding = 0 */
        public uint _start_canary;

        [FieldOffset(4)] /* size = 4, padding = 0 */
        public sg_image_type type;

        [FieldOffset(8)] /* size = 1, padding = 3 */
        public CBool render_target;

        [FieldOffset(12)] /* size = 4, padding = 0 */
        public int width;

        [FieldOffset(16)] /* size = 4, padding = 0 */
        public int height;

        [FieldOffset(20)] /* size = 4, padding = 0 */
        public int num_slices;

        [FieldOffset(24)] /* size = 4, padding = 0 */
        public int num_mipmaps;

        [FieldOffset(28)] /* size = 4, padding = 0 */
        public sg_usage usage;

        [FieldOffset(32)] /* size = 4, padding = 0 */
        public sg_pixel_format pixel_format;

        [FieldOffset(36)] /* size = 4, padding = 0 */
        public int sample_count;

        [FieldOffset(40)] /* size = 4, padding = 0 */
        public sg_filter min_filter;

        [FieldOffset(44)] /* size = 4, padding = 0 */
        public sg_filter mag_filter;

        [FieldOffset(48)] /* size = 4, padding = 0 */
        public sg_wrap wrap_u;

        [FieldOffset(52)] /* size = 4, padding = 0 */
        public sg_wrap wrap_v;

        [FieldOffset(56)] /* size = 4, padding = 0 */
        public sg_wrap wrap_w;

        [FieldOffset(60)] /* size = 4, padding = 0 */
        public sg_border_color border_color;

        [FieldOffset(64)] /* size = 4, padding = 0 */
        public uint max_anisotropy;

        [FieldOffset(68)] /* size = 4, padding = 0 */
        public float min_lod;

        [FieldOffset(72)] /* size = 4, padding = 4 */
        public float max_lod;

        [FieldOffset(80)] /* size = 1536, padding = 0 */
        public sg_image_content content;

        [FieldOffset(1616)] /* size = 8, padding = 0 */
        public byte* label;

        [FieldOffset(1624)] /* size = 8, padding = 0 */
        public fixed uint gl_textures[2]; /* original type is `uint32_t [2]` */

        [FieldOffset(1632)] /* size = 4, padding = 4 */
        public uint gl_texture_target;

        [FieldOffset(1640)] /* size = 16, padding = 0 */
        public fixed ulong _mtl_textures[16 / 8]; /* original type is `const void *[2]` */

        public ref void* mtl_textures(int index = 0)
        {
            fixed (sg_image_desc* @this = &this)
            {
                var pointer = (void* * )&@this->_mtl_textures[0];
                var pointerOffset = index;
                return ref *(pointer + pointerOffset);
            }
        }

        [FieldOffset(1656)] /* size = 8, padding = 0 */
        public void* d3d11_texture;

        [FieldOffset(1664)] /* size = 8, padding = 0 */
        public void* d3d11_shader_resource_view;

        [FieldOffset(1672)] /* size = 8, padding = 0 */
        public void* wgpu_texture;

        [FieldOffset(1680)] /* size = 4, padding = 4 */
        public uint _end_canary;
    }

how does this tool treat "regular" enum fields versus "bitflag fields"

It doesn't. One option is to use a heuristic by looking at the members of a C enum and saying "that's probably flags". Another option is to use clang attributes to mark an enum in C source code to be used as flags. From there libclang might be able to tell me some good information, I have not investigated this yet.

@floooh
Copy link
Owner

floooh commented Jan 4, 2021

I've started working on a separate branch 'bindgen-friendly' where I'm implementing some API changes mentioned at the end of this blog post: https://floooh.github.io/2020/08/23/sokol-bindgen.html

One idea I have with your feedback is hinting the bindings generator with a typedef for generating such bitmask fields. For instance:

Each 'flag bits enum' could get a specifically named bitmask typedef so that a bindings generator would know that a struct field is supposed to hold combinations of enum items. For instance:

// a flag-bit enum:
typedef enum sg_color_mask {
    ...
} sg_color_mask;

// the matching flagfield type:
typedef uint32_t sg_color_mask_bitfield;

// ...and down in the sg_blend_state:
typedef struct sg_blend_state {
    ...
    sg_color_mask_bitfield color_write_mask;
    ....
} sg_blend_state;

The bindings generator could then look for *_bitfield typedefs and match the name against an enum by the typename. Would that help?

@lithiumtoast
Copy link
Author

Hacking an existing C compiler or using libclang to extract the required type information. This falls into the category ‘big software project on its own’, definitely more work than I want to invest into a solution.

Oops. Should have read your blog before spending a good week or two using libclang. Well, it's done now at least. I have project over at: https://github.com/lithiumtoast/c2cs. Could probably separate out the libclang part from the Rosyln part or even move the libclang part over to use Zig instead of C#. (I don't want to write and maintain C code lol.) Would that be something of interest? I'm personally thinking of moving all my future projects to Zig anyway; I'm slowly starting to swallow the hard pill that C# is the wrong tool for games.

I see that even in your Zig bindings experiment you have the same problem as I described in the first post of this issue.

The bindings generator could then look for *_bitfield typedefs and match the name against an enum by the typename. Would that help?

Maybe; it would kind of be a solution in the same space as adding special attributes or comments as you mention about about "Annotating the C-API with keywords understood by a simple parser but ignored by the C compiler". What I would really like as a solution is creating bindings for any C API out in the wild. I know this is not realistic as you already mentioned some problems that make C APIs not very friendly for bindings. (ExplicitLayout in C# allows for unions which is why I have not complained about it yet here in feedback.) However, I think the ability to take any decent C API out there and use it in like C#, Python or w/e from just an entry point of a single .h file and a shared library object is a worthwhile pursuit. It would really open up a bunch of known solid C libraries like libuv, SDL2, raylib and sokol_gfx 😉 from being accessible and save developer time from people writing and maintaining different flavoured wheels.

How about changing the type to sg_color_mask with a compiler directive like BINDGEN_FRIENDLY? I can simply make sure I have the macro defined when parsing the translation unit with libclang. Your example of a failing compilation wouldn't matter in this case because the source of where the compilation fails is irrelevant; only the exported functions and their return types + parameter types are relevant.

@floooh
Copy link
Owner

floooh commented Jan 17, 2021

Btw, change of plans, I simply "unrolled" all the possible bit combinations in the sg_color_mask enum, so that the "sg_blend_state.color_write_mask" can be a regular enum field like all the others:

dc6cb4a

...still in the bindgen-friendly branch, I hope to merge this in a week or so.

@floooh floooh closed this as completed Jan 17, 2021
@lithiumtoast
Copy link
Author

lithiumtoast commented Jan 19, 2021

simply "unrolled" all the possible bit combinations in the sg_color_mask enum, so that the "sg_blend_state.color_write_mask" can be a regular enum field like all the others

I guess that is an option here which solves this problem. Kinda passes the buck for creating auto-generated idiomatic bindings though. But one problem and solution at a time so I'll take it 😃

change of plans

Will you explain your thoughts on this in a blog post?

@floooh
Copy link
Owner

floooh commented Jan 19, 2021

Will you explain your thoughts on this in a blog post?

Yes I'm planning to write a blog post explaining the upcoming API changes.

One point is simplification in enums to make AST-parsing with clang / libclang easier (for instance all enum items are simple integer literals instead of constant expressions).

@lithiumtoast
Copy link
Author

Turns out that libclang can give you information about clang attributes for a cursor. So for the libclang route of autogen bindings, this kind of thing is an option:

enum __attribute__((enum_extensibility(closed),flag_enum)) MyEnum
{
  F1 = 1 << 0, 
  F2 = 1 << 1,
  F3 = 1 << 2
};

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