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

sokol-header: Assertion failed #5

Closed
Tracked by #3
kassane opened this issue Dec 29, 2023 · 18 comments
Closed
Tracked by #3

sokol-header: Assertion failed #5

kassane opened this issue Dec 29, 2023 · 18 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@kassane
Copy link
Owner

kassane commented Dec 29, 2023

Occurs in ReleaseSafe and Debug build mode during run:

$> zig build clear          
$> ./zig-out/bin/clear      
Backend: Glcore33
clear: /home/kassane/sokol-d/src/sokol/c/sokol_gfx.h:8995: void _sg_gl_commit(void): Assertion '!_sg.gl.in_pass' failed.
[1]    12762 IOT instruction (core dumped)  ./zig-out/bin/clear

$> zig build debugtext_print
$> ./zig-out/bin/debugtext_print 
debugtext_print: /home/kassane/sokol-d/src/sokol/c/sokol_debugtext.h:3780: sdtx_context_desc_t _sdtx_context_desc_defaults(const sdtx_context_desc_t *): Assertion 'res.canvas_width > 0.0f' failed.
[1]    13331 IOT instruction (core dumped)  ./zig-out/bin/debugtext_print

sgl_context no assertion issue.

cc: @floooh

@kassane kassane added bug Something isn't working help wanted Extra attention is needed labels Dec 29, 2023
@floooh
Copy link
Collaborator

floooh commented Dec 30, 2023

First one is easy:

sg.commit();
sg.endPass();

The order of sg.commit() and sg.endPass() must be reversed (basically, sg.commit() must be the last thing in a frame).

The second one is tricky, it's this line:

https://github.com/floooh/sokol/blob/adf1f83657ac011168412eeb8f98f56dcd2969fe/util/sokol_debugtext.h#L3780

...but this assert should actually be impossible to hit, because the 0.0f value should have been replaced with _SDTX_DEFAULT_CANVAS_WIDTH (which is 640) here:

https://github.com/floooh/sokol/blob/adf1f83657ac011168412eeb8f98f56dcd2969fe/util/sokol_debugtext.h#L3775

This could be some sort of memory corruption or ABI issue. What operating system is this on? Also, stepping to the code in the debugger might help identifying the issue.

@kassane
Copy link
Owner Author

kassane commented Dec 30, 2023

The order of sg.commit() and sg.endPass() must be reversed (basically, sg.commit() must be the last thing in a frame).

fixed!!

This could be some sort of memory corruption or ABI issue. What operating system is this on? Also, stepping to the code in the debugger might help identifying the issue.

I'm running on Linux. I'll debug it.
Thanks for the clarification.

Also on the other examples, except clear. Although it was replicated line by line. I haven't had the same result... I'll have to see how the LDC2 compiler affects compared to the zig compiler.

@kassane kassane mentioned this issue Jan 2, 2024
13 tasks
@kassane
Copy link
Owner Author

kassane commented Jan 2, 2024

WiP (based on rust example):
image

@floooh
Copy link
Collaborator

floooh commented Jan 3, 2024

How did you fix it? FWIW, sometimes I've been seeing weird platform-specific ABI related issues in language bindings where the struct params were corrupted on their way to the other language (for instance in Zig on Intel Macs).

I'll take care of a couple smaller requests and PRs over the next few days, and will most likely have time to look into the D bindings starting this weekend (unless you need more time for sample code).

(in any case I'll try to get a bit familiar with the D toolchain and tinker around with the bindings)

@kassane
Copy link
Owner Author

kassane commented Jan 3, 2024

I''ve been seeing weird platform-specific ABI related issues in language bindings where the struct params were corrupted on their way to the other language (for instance in Zig on Intel Macs).

Yeah, I'm getting these issues in D, including betterC enabled (deactivating D runtime + part of phobos(std) and garbage collector).
At the moment, I'm debugging the C solutions and the other bindings to track some changes in behavior.

How did you fix it?

The problem in the debug still persists, but I've had to rewrite the example followed by rust, instead of zig example.


Edit

Also, I'm adding a few more examples so that refactoring will also impact the bindgen!!! (e.g.: cube and blend)

Any help with revision is welcome!

@kassane
Copy link
Owner Author

kassane commented Jan 3, 2024

module mymodule;

extern(C)
@nogc
nothrow {

    void my_c_function();

}

I find it to look cleaner, minimizing risks of mistakes

Bindgen already translates with a similar interface.

sokol-d/src/sokol/gfx.d

Lines 1797 to 1808 in c42a959

extern(C) scope const(void)* sg_d3d11_device() @system @nogc nothrow;
scope const(void)* d3d11Device() @trusted nothrow {
return sg_d3d11_device();
}
extern(C) scope const(void)* sg_d3d11_device_context() @system @nogc nothrow;
scope const(void)* d3d11DeviceContext() @trusted nothrow {
return sg_d3d11_device_context();
}
extern(C) D3d11BufferInfo sg_d3d11_query_buffer_info(Buffer) @system @nogc nothrow;
D3d11BufferInfo d3d11QueryBufferInfo(Buffer buf) @trusted nothrow {
return sg_d3d11_query_buffer_info(buf);
}

Only the examples are not auto-generated, allowing you to use what suits you best (with or without GC).

@floooh
Copy link
Collaborator

floooh commented Jan 3, 2024

globals in D are threadlocal, i don't remember if sokol is threadsafe or not, so to be safe any global you use, you should mark them as __gshared to match C globals

This shouldn't matter for the sample code I would think. Personally I would prefer sample code that isn't annotated like this unless absolutely needed.

@kassane
Copy link
Owner Author

kassane commented Jan 6, 2024

This won't work in -betterC, you have the same function in bunch of modules and seem to be only used in the app one, it's completely useless imo, just remove it, better to leave it to the user how to handle the ownership/lifetime of that c string, plus D has slices, [0 .. nullTermPos] is enough

example:

string cStrTod(inout(char)*  c_str) {
    auto start = c_str;
    auto end = cast(char*) c_str;
    for (; *end; end++){}
    return cast(string) c_str[0 .. end - start];
}

Fixed on 0684058
https://github.com/kassane/sokol-d/actions/runs/7433435082

Issue solved on betterC [Windows/MacOS only]

error: undefined reference to symbol __D4core8internal5array11duplication__T8_dupCtfeTaTyaZQpFNaNbNiNfMAaZAya
    note: referenced in /Users/runner/work/sokol-d/sokol-d/zig-cache/o/8441520852fd3bf30fcee699975c9b06/debugtext_print.o
error: undefined reference to symbol __D4core8internal5array11duplication__T8_dupCtfeTxaTaZQpFNaNbNiNfMAxaZAa
    note: referenced in /Users/runner/work/sokol-d/sokol-d/zig-cache/o/8441520852fd3bf30fcee699975c9b06/debugtext_print.o

Thank you!!

@kassane
Copy link
Owner Author

kassane commented Feb 1, 2024

Back to the original topic of the problem. After running some debugging tests, I realized that the error in the assert is related to the fact that the float/double in D is nan instead of 0.0f by default.

sokol_debugtext.h:3780: sdtx_context_desc_t _sdtx_context_desc_defaults(const sdtx_context_desc_t *): Assertion `res.canvas_width > 0.0f` failed.

References

@kassane
Copy link
Owner Author

kassane commented Feb 1, 2024

New brief test in sokol_debugtext.h:

diff --git a/src/sokol/c/sokol_debugtext.h b/src/sokol/c/sokol_debugtext.h
index 09da601..a44a673 100644
--- a/src/sokol/c/sokol_debugtext.h
+++ b/src/sokol/c/sokol_debugtext.h
@@ -3777,6 +3777,7 @@ static sdtx_context_desc_t _sdtx_context_desc_defaults(const sdtx_context_desc_t
     res.tab_width = _sdtx_def(res.tab_width, _SDTX_DEFAULT_TAB_WIDTH);
     // keep pixel format attrs are passed as is into pipeline creation
     SOKOL_ASSERT(res.char_buf_size > 0);
+    SOKOL_ASSERT(!isnan(res.canvas_width));
     SOKOL_ASSERT(res.canvas_width > 0.0f);
     SOKOL_ASSERT(res.canvas_height > 0.0f);
     return res;

Output:

steps [2/4] debugtext_print... debugtext_print: /home/kassane/sokol-d/src/sokol/c/sokol_debugtext.h:3780: sdtx_context_desc_t _sdtx_context_desc_defaults(const sdtx_context_desc_t *): Assertion `!__builtin_isnan (res.canvas_width)' failed.
run-debugtext_print
└─ run /home/kassane/sokol-d/zig-out/bin/debugtext_print failure
error: the following command terminated unexpectedly:
/home/kassane/sokol-d/zig-out/bin/debugtext_print

@kassane
Copy link
Owner Author

kassane commented Feb 2, 2024

Finally fix! Added default values to structs number fields as opposed to init (nan).

image

@kassane kassane closed this as completed in b27dc82 Feb 2, 2024
@floooh
Copy link
Collaborator

floooh commented Feb 3, 2024

Yeah that's what I also do in the Zig bindings:

https://github.com/floooh/sokol-zig/blob/7c941144c8b303ed58f1c9a47e3b4206d7c3fce3/src/sokol/gfx.zig#L468-L489

...it's important that non-initialized struct items are set to zero.

@kassane
Copy link
Owner Author

kassane commented Feb 3, 2024

Yeah that's what I also do in the Zig bindings:

https://github.com/floooh/sokol-zig/blob/7c941144c8b303ed58f1c9a47e3b4206d7c3fce3/src/sokol/gfx.zig#L468-L489

...it's important that non-initialized struct items are set to zero.

Yes, when I started porting the zig script to D, it removed all default values, in consideration of the C concept.
Now I've had a look at how to do float/double array filling without GC.

Maybe this solves the pipeline shader problem in sokol-tools.
https://d.godbolt.org/z/W39ofzKqc (fill static array)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants
@floooh @kassane and others