Skip to content

Conversation

@nicoabie
Copy link
Contributor

@nicoabie nicoabie commented Apr 7, 2025

@Interrupt I'm learning zig and I found delve very nice so wanted to contribute

This is missing emscripten for lua natecraddock/ziglua#86
I'll ask again if they can add that flag so we don't need to use your fork anymore}

for changes in the shaders
https://floooh.github.io/2024/11/04/sokol-fall-2024-update.html

});

const zmesh_lib = if (options.shared) blk: {
// TODO addLibrary
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addSharedLibrary is deprecated

stride: usize, // 0 == automatically determined by accessor
view_type: BufferViewType,
data: ?*anyopaque, // overrides buffer.data if present, filled by extensions
override_data: ?*anyopaque, // overrides buffer.data if present, filled by extensions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new zig v0.14 didn't like that variable and function having the same name

build.zig Outdated
.optimize = optimize,
.lang = .lua54,
.can_use_jmp = !target.result.isWasm(),
// .can_use_jmp = !target.result.cpu.arch.isWasm(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be fixed to have the same funcitonality as with v0.13

}
}

// TODO check this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check this

max: Vec3,
center: Vec3,
transform: Mat4 = math.Mat4.identity,
transform_matrix: Mat4 = math.Mat4.identity,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new zig v0.14 didn't like that variable and function having the same name

debug.info("Creating shader: {d}", .{graphics.next_shader_handle});
const shader = sg.makeShader(shader_desc);

// TODO check this
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


var enable_debug_logging = false;

pub var test_me: bool = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

breaks findLibraryFunctions

@@ -0,0 +1 @@
Download sokol-shdc from https://github.com/floooh/sokol-tools-bin/tree/bindings-cleanup No newline at end of file
Copy link
Owner

@Interrupt Interrupt Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the bindings-cleanup branch explicitly required, hasn't this been merged in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the latest tag. I though it made sense to link to the specific version of the bin which generated the shaders.

checked with latest master https://github.com/floooh/sokol-tools-bin/tree/9b5a3e2b57fe9783ba4d1f3249059bc4720b592f and the output is the same

@nii236
Copy link

nii236 commented May 16, 2025

Hey @nicoabie I had a quick test of your fork and none of the examples run correctly when compiled for WASM. I'm brand new to zig so not sure how to fix:

image

@nicoabie
Copy link
Contributor Author

Hi! Thanks for reporting, I'll try to check it out in the following weeks. I'm new to zig as well :)

@Interrupt
Copy link
Owner

Sorry for the delay! Just digging into this now again.

When running the stresstest, something seems to be leaking vertex buffer bindslots. Getting the following error.

...
Skinned mesh example module initializing
Assertion failed: (a_state->buffer_index < SG_MAX_VERTEXBUFFER_BI

@Interrupt
Copy link
Owner

Still tracking this down, but it looks like if I stop caching the common pipelines in the platform/graphics.zig shader init functions, then the passes and stresstest run correctly:

    pub fn initFromBuiltin(cfg: ShaderConfig, comptime builtin: anytype) !Shader {
        const shader = try ShaderImpl.initFromBuiltin(cfg, builtin);
        // shader.makeCommonPipelines();
        return shader;
    }

@Interrupt
Copy link
Owner

Aha, I think it was a use-after-free in makeCommonPipelines - this seems to have fixed it:

    pub fn makeCommonPipelines(self: *Shader) void {
        for (common_vertex_layouts) |l| {
            _ = self.impl.makePipeline(l);
        }
    }

@Interrupt Interrupt marked this pull request as ready for review October 21, 2025 16:30
@Interrupt
Copy link
Owner

Made some final changes to this branch to disable the builtin scripting support when building for Emscripten. Merging in now!

@Interrupt Interrupt merged commit dce251d into Interrupt:main Oct 21, 2025
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

Successfully merging this pull request may close these issues.

3 participants