zig icon indicating copy to clipboard operation
zig copied to clipboard

C parameters passed in wrong order

Open yujiri8 opened this issue 3 years ago • 6 comments

Zig Version

0.10.0-dev.4418+99c3578f6

Steps to Reproduce

I'm using these raylib bindings: https://github.com/Not-Nik/raylib-zig

And seeing a very strange issue where parameters to some raylib functions seem to be passed in the wrong order.

Here is a minimal main.zig that reproduces it for me:

const raylib = @import("raylib");
pub fn main() !void {
    raylib.InitWindow(600, 600, "blah");
    raylib.SetTargetFPS(60);
    while (true) {
        raylib.BeginDrawing();
        var font = raylib.GetFontDefault();
        raylib.ClearBackground(raylib.BLACK);
        raylib.DrawTextEx(font, "blah", .{ .x = 5, .y = 120 }, 16, 2, raylib.WHITE);
        raylib.EndDrawing();
    }
}

build.zig:

const Builder = @import("std").build.Builder;
const raylib = @import("deps/raylib/lib.zig");

pub fn build(b: *Builder) void {
    const target = b.standardTargetOptions(.{});
    const mode = b.standardReleaseOptions();

    const exe = b.addExecutable("exe", "src/main.zig");
    exe.setTarget(target);
    exe.setBuildMode(mode);
    exe.install();

    raylib.link(exe, false);
    raylib.addAsPackage("raylib", exe);
}

I'm running this on Alpine Linux (musl libc) x86_64.

Expected Behavior

It is supposed to be the position to draw the text at is the 3rd parameter and the font size is 4th (see raylib documentation: https://www.raylib.com/cheatsheet/cheatsheet.html)

Actual Behavior

The .y field of the position controls the font size.

It works as expected in zig 0.9.1.

yujiri8 avatar Oct 17 '22 20:10 yujiri8

Oh, forgot to add:this warning is thrown during compile:

LLD Link... warning(link): unexpected LLD stderr:
ld.lld: warning: /home/yujiri/code/spacestation-defense/zig-cache/o/c3613de9397765a9e1a920bd0e8aa5a8/libraylib.a: archive member '/home/yujiri/code/spacestation-defense/zig-cache/o/319057d551723c5507c915bb89a4df86/raylib.o' is neither ET_REL nor LLVM bitcode

ghost avatar Oct 17 '22 20:10 ghost

Reduction:

pub const Vector2 = extern struct {
    x: f32,
    y: f32,
};
pub extern fn DrawTextEx(position: Vector2, fontSize: f32) void;
pub fn main() void {
    DrawTextEx(.{ .x = 1, .y = 2 }, 12);
}
#include <stdio.h>
typedef struct {
    float x;
    float y;
} Vector2;
void DrawTextEx(Vector2 position, float fontSize) {
    printf("position x %f y %f\n", position.x, position.y);
    printf("fontSize %f\n", fontSize);
}

Vexu avatar Oct 18 '22 08:10 Vexu

I'd like to add, that this also happens when DrawTextEx takes only single argument, Vector2 position (without fontSize). The function then logs: position x 1.000000 y 0.000000 (and so I did observe similiar behaviour in my game multiple times, it's always Y being 0)

It's worth noting, that when Vector has 3rd field, the function works properly as shown below:

pub const Vector = extern struct {
    x: f32,
    y: f32,
    z: f32,
};
pub extern fn DrawTextEx(position: Vector) void;
pub fn main() 
    DrawTextEx(.{ .x = 1, .y = 2, .z = 3 });
}
#include <stdio.h>
typedef struct {
    float x;
    float y;
    float z;
} Vector;
void DrawTextEx(Vector position) {
    printf("position x %f y %f z %f\n", position.x, position.y, position.z);
}

logs: position x 1.000000 y 2.000000 z 3.000000

Natchuz avatar Oct 19 '22 13:10 Natchuz

I have the exact same issue but with a different C library. An important note is that it happens on macOS M1 (aarch64) but not on my Linux box (amd64).

The setup

$ zig version
0.10.0-dev.4212+34835bbbc

I'm linking to a C library (cimgui, a C binding of the C++ DearImGui library) which contains an ImVec2 type containing two floats.

  • The C definition in the .h file:
typedef struct ImVec2 ImVec2;
struct ImVec2
{
    float x, y;
};
  • The corresponding Zig cimport generated result:
pub const struct_ImVec2 = extern struct {
    x: f32,
    y: f32,
};
pub const ImVec2 = struct_ImVec2;

Details

I call a method with this signature:

C:
extern "C" void ImDrawList_AddRectFilled(ImDrawList* self,const ImVec2 p_min,const ImVec2 p_max,ImU32 col,float rounding,ImDrawFlags flags);

Zig:
pub extern fn ImDrawList_AddRectFilled(self: [*c]ImDrawList, top_left: ImVec2, top_right: ImVec2, col: ImU32, rounding: f32, flags: ImDrawFlags) void;

Let's say I call it with:

var top_left: = ImVec2{ .x = 50.0, .y = 250.0 };
var bottom_right = ImVec2{ .x = 500.0, .y = 1000.0 };
draw_list.AddRectFilled(top_left, bottom_right, 0xFFFFFFFF, 2.0, 0);

if I modify the C code to immediately print the value of top_left and bottom_right:

(50.0, 500.0) (2.0, -0.0)

You can see that in the first C ImVec2, I have (top_left.x, bottom_right.x), and the second ImVec2 also contains a mix of the function parameters.

Note that if I unset use_stage1=true in my build.zig, I don't have the bug. I guess this is a bug in the stage1 compiler only and not in the stage2 one.

HTH.

remeh avatar Oct 22 '22 17:10 remeh

~~0.10.0-dev.4212+34835bbbc is quite old, can you try a newer version?~~ Oh and you said you're using stage1, never mind this entire reply then. ~~Also amd64 and aarch64 should be the same.~~ misread as arm64.

Vexu avatar Oct 22 '22 18:10 Vexu

Also amd64 and aarch64 should be the same. misread as arm64.

If I'm not mistaken, aarch64 is basically the same as arm64 (and not amd64), and I've seen your commit above, but it seems to target amd64 though? Just to make sure: I observe the bug with stage1 on aarch64/arm64 only and I can't reproduce it on amd64, but @yujiri8 having opened this thread is observing it on amd64.

Edit: I can also reproduce with stage2 on amd64/Linux.

remeh avatar Oct 22 '22 20:10 remeh

Hey folks. Just a small note that it is still broken with 0.10.0-dev.4560+828735ac0 (zig-macos-aarch64-0.10.0-dev.4212+34835bbbc.tar.xz from the website) on macOS M1 when using stage1. Let me know if that deserves a new issue and if you want me to open it with details.

remeh avatar Oct 25 '22 11:10 remeh

stage1 is expected to be broken in many ways and does not handle non-x86_64 C ABI at all.

Vexu avatar Oct 25 '22 11:10 Vexu