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

WIP sample cleanup. #15

Merged
merged 29 commits into from
May 12, 2024
Merged

WIP sample cleanup. #15

merged 29 commits into from
May 12, 2024

Conversation

floooh
Copy link
Collaborator

@floooh floooh commented May 11, 2024

...just FYI, I'm going through the samples, make them work, and do some subjective code cleanup.

I'll let you know when I consider them 'done'.

PS: I would also actually prefer to drastically simplify the build.zig, for instance kick out the WebAssembly stuff (which IMHO is just distracting).

I would also prefer a build process that's more aligned with what's usual for the D ecosystem (e.g. if possible without Zig involvement, or at least use Zig only to build the C sources if D can't do that on its own).

Is there actually such a thing like a standard "D build system"? PS: nvm, it seems to be the same mess as in the C/C++ world :/

fix: #13

@floooh
Copy link
Collaborator Author

floooh commented May 11, 2024

PS: I'm also thinking about removing the range helper function alltogether, using D's .ptr and .sizeof works well enough IMHO, and it's less obscure.

@kassane kassane added the enhancement New feature or request label May 11, 2024
@kassane
Copy link
Owner

kassane commented May 11, 2024

I would also prefer a build process that's more aligned with what's usual for the D ecosystem (e.g. if possible without Zig involvement, or at least use Zig only to build the C sources if D can't do that on its own).

Is there actually such a thing like a standard "D build system"? PS: nvm, it seems to be the same mess as in the C/C++ world :/

Sadly, dub only allows the management of packages (like cargo), but it is not suitable for building systems.
Up to now, it has not been possible (without a workaround) to bind C code to D code in dub.

There's the reggae build-system and meson which offer official support for dlang.

@floooh
Copy link
Collaborator Author

floooh commented May 11, 2024

Ok, then let's stick with Zig :)

Still not sure about the whole WebAssembly thing though ;)

src/examples/debugtext.d Outdated Show resolved Hide resolved
@floooh
Copy link
Collaborator Author

floooh commented May 11, 2024

Hmm, I'm now at a point again where I need debugging to work, and I can't get it to work on macOS. I'll try to continue on Linux tomorrow.

(I could have sworn I had debugging working on Mac at one point, but don't remember how :/)

PPS: ah right, good thing I was writing everything down: floooh/sokol#955 (comment)

@floooh
Copy link
Collaborator Author

floooh commented May 11, 2024

Is this necessary? It seems to prevent debugging in vscode on macOS:

sokol-d/build.zig

Lines 363 to 365 in c20da5b

// remove object files after success build, and put them in a unique temp directory
if (options.kind != .obj)
try cmds.append("-cleanup-obj");

...when removing this code, debugging via CodeLLDB "just works". Should I remove it?

@kassane
Copy link
Owner

kassane commented May 11, 2024

Is this necessary? It seems to prevent debugging in vscode on macOS:

Remove it.


The goal was to decrease the zig-cache's size. However, ldc2 already has cache-build enabled in build.zig.

http://johanengelen.github.io/ldc/2016/09/17/LDC-object-file-caching.html

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

I also stumbled again over D setting undefined floats to NaN, I'm now changing the sokol-shdc code generator to default-zero-initialize all primitive types.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Btw what's the purpose of this Zig file?

https://github.com/kassane/sokol-d/blob/main/src/handmade/math.zig

Does D not have an equivalent of those math functions?

...I'm going to look at the math functions next, since it looks like at least the matrix functions produce some funky results.

PS: ok, seems to be about WebAssembly support...

@kassane
Copy link
Owner

kassane commented May 12, 2024

...cannot pass rvalue argument...

remove -preview=all - experimental features D standard. Like clang -fexperimental-library.
DIP 1000 (safe rules) affects rvalues.
https://github.com/dlang/DIPs/blob/master/DIPs/other/DIP1000.md

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Ok, when I change this code:

sg.makeShader(shd.triangle_shader_desc(sg.queryBackend()));

...so that it uses an intermediate variable like this:

auto shd_desc = shd.triangle_shader_desc(sg.queryBackend());
sg.makeShader(shd_desc);

...it works. Blargh, that sucks :D

I also get a linker error, but I guess that's because I moved the formerly platform-specific embedded shader code in the triangle sample into a .glsl shader.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

...do I need to declare an imported source file (e.g. import shd = shaders.triangle;) also as a linker dependency somewhere in the build file?

@kassane
Copy link
Owner

kassane commented May 12, 2024

...do I need to declare an imported source file (e.g. import shd = shaders.triangle;) also as a linker dependency somewhere in the build file?

-i[=<pattern>] - Include imported modules in the compilation


I'm not mac user, but like zig, ldc2/ldmd2 have --link-internally - Use internal LLD for linking

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Another problem I encounter with dub :/

src/examples/cube.d(128,26): Error: cannot take address of local `vsParams` in `@safe` function `frame`

...for code like this:

shd.VsParams vsParams = { mvp: computeMvp(state.rx, state.ry) };
sg.Range r = { ptr: &vsParams, size: vsParams.sizeof };

...why suddenly all those errors? Is this a "D 1.0" vs "D 2.0" thing?

@kassane
Copy link
Owner

kassane commented May 12, 2024

betwen zig and dub have dflags "-preview=all"?

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Since -preview=all makes a lot more sense than vanilla D (to me at least lol), shouldn't we enable it in the dub build too?

@kassane
Copy link
Owner

kassane commented May 12, 2024

Another problem I encounter with dub :/

Fix dub here. add dflags "-preview=all" "-i" on all examples

LANG=C dub build -b release --compiler=$HOME/zig-bootstrap/out/host/bin/ldc2 :cube
    Starting Performing "release" build using /home/kassane/zig-bootstrap/out/host/bin/ldc2 for x86_64.
    Building sokol-d:cube 0.1.0: building configuration [application]
   Pre-build Running commands
     Linking sokol-d_cube
kassane:~/sokol-d $ LANG=C dub build -b release --compiler=$HOME/zig-bootstrap/out/host/bin/ldc2 :mrt
    Starting Performing "release" build using /home/kassane/zig-bootstrap/out/host/bin/ldc2 for x86_64.
    Building sokol-d:mrt 0.1.0: building configuration [application]
   Pre-build Running commands
     Linking sokol-d_mrt
kassane:~/sokol-d $ git diff
diff --git a/dub.sdl b/dub.sdl
index 5015679..66cb6a2 100644
--- a/dub.sdl
+++ b/dub.sdl
@@ -34,6 +34,7 @@ subPackage {
        targetPath "build"
        sourceFiles "src/examples/clear.d"
        libs "sokol"
+       dflags "-preview=all" "-i"
        lflags "-Lzig-out/lib" platform="posix"
        lflags "/LIBPATH:zig-out/lib" platform="windows"
        excludedSourceFiles "src/examples/sgl_context.d" "src/examples/triangle.d" "src/examples/sgl_points.d" "src/examples/saudio.d" "src/examples/debugtext.d" "src/examples/mrt.d" "src/examples/user_data.d" "src/examples/cube.d" "src/examples/blend.d" "src/shaders/*.d"

@kassane
Copy link
Owner

kassane commented May 12, 2024

a lot more sense than vanilla D (to me at least lol), shouldn't we enable it in the dub build too?

When we use zig only, it is responsible for ldc2 execution.
However, with dub enabled, zig is only responsible for libsokol (clang) only.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Ok, the dub builds work with those additional flags.

But next problem: it looks like the dub builds want to use a libsokol dynamic link library is this intended?

On macOS I get this when I want to run one of the dub-generated exes:

sokol-d ➤ build/sokol-d_triangle
dyld[9896]: Library not loaded: @rpath/libsokol.dylib

@kassane
Copy link
Owner

kassane commented May 12, 2024

ref.: #13

The Windows target using the sokol dynamic library does not work. Only works in static library using zig build by default.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Can't we use static linking instead?

@kassane
Copy link
Owner

kassane commented May 12, 2024

Can't we use static linking instead?

need add some sokol config on dub. On zig sokol share with ldc2Config.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Hmm ok, I committed the dub.sdl changes (-preview=all -i), and then let me also quickly change the naming convention for the create-shader function... I guess that's all I can do for now.

@kassane
Copy link
Owner

kassane commented May 12, 2024

Can't we use static linking instead?

On Linux e.g.:

LANG=C dub build -b release --compiler=$HOME/zig-bootstrap/out/host/bin/ldc2 :cube
    Starting Performing "release" build using /home/kassane/zig-bootstrap/out/host/bin/ldc2 for x86_64.
    Building sokol-d:cube 0.1.0: building configuration [application]
   Pre-build Running commands
     Linking sokol-d_cube
/usr/bin/ld: zig-out/lib/libsokol.a(/home/kassane/sokol-d/zig-cache/o/a72e20601cb44fb59375c61f7cb39bec/sokol_app.o): in function `_sapp_linux_run':
sokol_app.c:(.text+0x4bd): undefined reference to `XInitThreads'
/usr/bin/ld: sokol_app.c:(.text+0x4c2): undefined reference to `XrmInitialize'
/usr/bin/ld: sokol_app.c:(.text+0x4c9): undefined reference to `XOpenDisplay'
/usr/bin/ld: sokol_app.c:(.text+0x548): undefined reference to `XkbSetDetectableAutoRepeat'
/usr/bin/ld: sokol_app.c:(.text+0x554): undefined reference to `XResourceManagerString'
/usr/bin/ld: sokol_app.c:(.text+0x561): undefined reference to `XrmGetStringDatabase'
/usr/bin/ld: sokol_app.c:(.text+0x598): undefined reference to `XrmGetResource'
/usr/bin/ld: sokol_app.c:(.text+0x5c5): undefined reference to `XrmDestroyDatabase'
/usr/bin/ld: sokol_app.c:(.text+0x634): undefined reference to `XInternAtom'
/usr/bin/ld: sokol_app.c:(.text+0x650): undefined reference to `XInternAtom'
/usr/bin/ld: sokol_app.c:(.text+0x66c): undefined reference to `XInternAtom'
/usr/bin/ld: sokol_app.c:(.text+0x688): undefined reference to `XInternAtom'
/usr/bin/ld: sokol_app.c:(.text+0x6a4): undefined reference to `XInternAtom'
/usr/bin/ld: zig-out/lib/libsokol.a(/home/kassane/sokol-d/zig-cache/o/a72e20601cb44fb59375c61f7cb39bec/sokol_app.o):sokol_app.c:(.text+0x6c0): more undefined references to `XInternAtom' follow
/usr/bin/ld: zig-out/lib/libsokol.a(/home/kassane/sokol-d/zig-cache/o/a72e20601cb44fb59375c61f7cb39bec/sokol_app.o): in function `_sapp_linux_run':

@kassane
Copy link
Owner

kassane commented May 12, 2024

dub static-lib support superset this PR. Add in TODO/Roadmap

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

On Linux:

undefined reference to `XInternAtom'

...those look like the system DLL dependencies are missing, e.g. (in the sokol-zig build-zig):

        if (options.use_x11) {
            lib.linkSystemLibrary("X11");
            lib.linkSystemLibrary("Xi");
            lib.linkSystemLibrary("Xcursor");
        }

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

I fixed the shader-desc function names btw, so the examples source code should be fine now. I'll merge the sokol-shdc PR now and create binaries, so at least that part would be done.

@kassane
Copy link
Owner

kassane commented May 12, 2024

On Linux:

undefined reference to `XInternAtom'

...those look like the system DLL dependencies are missing, e.g. (in the sokol-zig build-zig):

        if (options.use_x11) {
            lib.linkSystemLibrary("X11");
            lib.linkSystemLibrary("Xi");
            lib.linkSystemLibrary("Xcursor");
        }

Add in dub

lflags "-Lzig-out/lib" platform="posix"
lflags "/LIBPATH:zig-out/lib" platform="windows"
lflags "/LIBPATH:zig-out/lib" platform="osx"

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Ok, the sokol-shdc branch has been merged: floooh/sokol-tools#128

The executables in https://github.com/floooh/sokol-tools-bin will be uptodate when this CI pipeline goes green:

https://github.com/floooh/sokol-tools/actions/runs/9053315598

I'll log off for today then :)

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

PS: I would prefer to merge this PR first, and then you can figure out how to fix the dub files. Since I don't have any knowledge how this works it's better if you take care of that directly ;)

In general I would prefer if we could put out the first version of the bindings soon-ish even if the extra things (like WASM support or building via dub) doesn't work yet. What should work though before a release is building via build.zig on Windows, macOS and Linux (I can also do some tests tomorrow on Windows and Linux).

@kassane
Copy link
Owner

kassane commented May 12, 2024

done for merge?

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

done for merge?

Yep, I think that's all :)

@kassane kassane merged commit 9198de3 into kassane:main May 12, 2024
@floooh floooh deleted the floooh-fixes-1 branch May 12, 2024 18:19
@kassane
Copy link
Owner

kassane commented May 12, 2024

Thanks for help.

@floooh
Copy link
Collaborator Author

floooh commented May 12, 2024

Btw, if you need to re-build the bindings via gen_all.py keep in mind that I have this inflight branch here where I dabbled with the scope keyword in function args (it's only a minimal change though and if needed I can discard this inflight-branch again):

https://github.com/floooh/sokol/tree/kassane-dlang_bindgen

...hopefully no more changes will be needed to the codegen python scripts before the release (e.g. ideally only changes to the sokol-d repo).

@kassane
Copy link
Owner

kassane commented May 12, 2024

@floooh, discard my PR and merge your branch.

Dub fixed for static-lib on v0.1.1

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

Successfully merging this pull request may close these issues.

Improvements and fixes
2 participants