Gnimuc / CImGui.jl

Julia wrapper for cimgui
https://github.com/cimgui/cimgui
MIT License
253 stars 25 forks source link

the CImGui.Text(text) function segfaults on aarm (v1.82) #74

Open ghyatzo opened 1 year ago

ghyatzo commented 1 year ago

Hi,

I have recently begun to switch my project to the newer version. And albeit for the most part it was a pretty painless transition, strings just don't want to behave properly.

the demo app include(joinpath(pathof(CImGui), "..", "..", "demo", "demo.jl")) segafults with

signal (11): Segmentation fault: 11
in expression starting at /Users/user/.local/julia/packages/CImGui/EBKak/demo/demo.jl:66
_platform_strlen at /usr/lib/system/libsystem_platform.dylib (unknown line)
Allocations: 8846575 (Pool: 8841676; Big: 4899); GC: 9

I tried all sort of combinations (yes also @sprintf) but the only one that works is if I override with the following CImGui.Text(text) = CImGui.TextUnformatted(text) (basically we force to input the \0 at the end)

If I add a \0 manually, unsafe_convert gets angry

┌ Error: Error in renderloop!
│   exception = ArgumentError: embedded NULs are not allowed in C strings: "This is some useful text.\0"
└ @ Main ~/.local/julia/packages/CImGui/EBKak/demo/demo.jl:125

Stacktrace:
  [1] unsafe_convert
    @ ./c.jl:216 [inlined]
  [2] igText(text::String)
    @ LibCImGui ~/.local/julia/packages/LibCImGui/DfBwq/src/LibCImGui.jl:41
  [3] Text
    @ ~/.local/julia/packages/CImGui/EBKak/src/wrapper.jl:1030 [inlined]
  [4] top-level scope
    @ ~/.local/julia/packages/CImGui/EBKak/demo/demo.jl:84

but at least it shows that it is doing what it is supposed to do (i think).

when running everything through rosetta, it works. I am not even sure this is an issue of this package, could it be an issue of julia on aarm?

Gnimuc commented 1 year ago

Does the following vararg version of the text API work on aarch macOS?

@ccall libcimgui.igText("Hello %s %s"::CString; "world"::CString, "!"::CString)::Cvoid

Gnimuc commented 1 year ago

BTW, CString is not mandatory. You can use Ptr{Cuchar} instead.

ghyatzo commented 1 year ago

I have tried a couple of variations, in steps towards the normal CImGui.Text(text).

here's what happens

 @ccall libcimgui.igText("Hello %s %s"::Cstring, "world"::Cstring, "!"::Cstring)::Cvoid    # Hello jl_fptr_args ??N?
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring, Cstring), "Hello %s %s", "world", "!") # Hello jl_fptr_args ??N?
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring), "test pre %s", "test") # test pre jl_fptr_args
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring), "%s", "hello") # segfaults

the ??N? would change to other random stuff (sometimes also the jl_fptr_args part) every time I started a new julia sessions. (different location in memory?)

Gnimuc commented 1 year ago

The first , in the ccall macro version should be ;

Gnimuc commented 1 year ago

https://docs.julialang.org/en/v1/manual/calling-c-and-fortran-code/#Mapping-C-Functions-to-Julia

ghyatzo commented 1 year ago

duh... sorry its morning here. I see, does @ccall supports varargs while ccall doesnt? ok now it's like this

 @ccall libcimgui.igText("Hello %s %s"::Cstring; "world"::Cstring, "!"::Cstring)::Cvoid    # Hello world !
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring, Cstring), "Hello %s %s", "world", "!") # Hello world !
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring), "test pre %s", "test") # test pre world
 ccall((:igText, libcimgui), Cvoid, (Cstring, Cstring), "%s", "hello") # world
Gnimuc commented 1 year ago

ccall only support a limited version of varadic arguments where each vararg should be of the same type.

ghyatzo commented 1 year ago

I noticed there is also a (probably) related issue with ImPlot annotations, as they only show as ??. I am toying around by redefining the implot calls using @ccall, there is a problem. I don't understand how to unpack tuples into varargs for example

function Annotate(x::Real, y::Real, pix_offset::ImVec2, fmt::String...)
    @ccall libcimgui.ImPlot_Annotate_Str(x::Cdouble, y::Cdouble, pix_offset::ImVec2; fmt::Cstring)::Cvoid
end

complains, and

@ccall libcimgui.ImPlot_Annotate_Str(x::Cdouble, y::Cdouble, pix_offset::ImVec2; fmt::Cstring...)::Cvoid

complains as well.

Do you know if it is possible to splat into the call? I guess I could splat before the macro expansion with an eval, but eww...

Gnimuc commented 1 year ago

Clang.jl has a prototype for wrapping vararg functions: https://github.com/JuliaInterop/Clang.jl/blob/9a519baa45000d2afe1262ae7463cc0795671721/gen/generator.toml#L163

But I never use that. Isn't the @ccall macro already concise to write?

ghyatzo commented 1 year ago

I meant that if I were to call Annotate(1, 2, ImVec2(0, 0), "a", "b", "c") in the first example typeof(fmt) == Tuple{String} and it complains it can't convert it into a Cstring, while in the second example it does not recognise ::Cstring... as a valid argument type and complains that fmt does not have one.

fmt...::Cstring... does not work as well. found this as well: https://discourse.julialang.org/t/why-is-ccall-syntax/72427/6

Anyhow, I tested with

function Annotate(x::Real, y::Real, pix_offset::ImVec2, fmt::String)
    @ccall libcimgui.ImPlot_Annotate_Str(x::Cdouble, y::Cdouble, pix_offset::ImVec2; fmt::Cstring)::Cvoid
end

so accepting only one string, but it still shows ?? in annotations

Gnimuc commented 1 year ago

Does it work if you directly use @ccall libcimgui.ImPlot_Annotate_Str(1::Cdouble, 2::Cdouble, ImVec2(0, 0)::ImVec2; "a"::Cstring, "b"::Cstring, "c"::Cstring)::Cvoid?

ghyatzo commented 1 year ago

Nope, still ??

Gnimuc commented 1 year ago

According to https://github.com/cimgui/cimplot/blob/d68fa3abd4fe75312089e9332917a6c326704a58/cimplot.h#L965, the correct way to invoke the API is @ccall libcimgui.ImPlot_Annotate_Str(1::Cdouble, 2::Cdouble, ImVec2(0, 0)::ImVec2, "fmt %s %s"::Cstring; "a"::Cstring, "b"::Cstring)::Cvoid

EDIT: updated the wrong ; position

ghyatzo commented 1 year ago

Got it working with, otherwise it would print fmt as well.

@ccall libcimgui.ImPlot_Annotate_Str(1::Cdouble, 2::Cdouble, ImVec2(0, 0)::ImVec2, "%s %s"::Cstring; "a"::Cstring, "b"::Cstring)::Cvoid

Shall I make an issue to implot? or is that a generator issue?

IanButterworth commented 11 months ago

This is a blocker for me on Apple Silicon. Has there been any progress?

Also, I note that CImGui.Text(text) = CImGui.TextUnformatted(text) does work for me as a workaround