floooh / sokol-nim

nim bindings for https://github.com/floooh/sokol
MIT License
76 stars 14 forks source link

use `addr` instead `unsafeAddr` #21

Closed Angluca closed 1 year ago

Angluca commented 1 year ago
warning:: `unsafeAddr` is a deprecated alias for `addr`, 
use `addr` instead.

nim devel (v1.9+) deprecate unsafeAddr, Nim may upgrade to v2.0 in a few months or next year (maybe)

floooh commented 1 year ago

Ah ok, tests are failing because the Nim version used in CI is tool old.

Can you update the version here?

https://github.com/floooh/sokol-nim/blob/dcf9dd789cc3e1601ab4c989ef21f4d2ee6b8296/.github/workflows/main.yml#L15

(I hope that the PR CI pipeline uses the workflow file from your branch, and not from master)

Angluca commented 1 year ago

Examples add event function (press exitapp)

Angluca commented 1 year ago

nim shader can use the string, Not use array type It's hard to read and too long

# nim use cstring like c code
var str = "12345"
var cstr: cstring = str # string auto change type to cstring
assert cstr[0] == 49 # this cstring like c char[5] 
# shaders/triangle.nim
#[
const fsSourceMetalMacos: array[xxx, uint8] = [
    0x23'u8,0x69,0x6e,0x63,0x6c,0x75,0x64,0x65,0x20,0x3c,0x6d,0x65,0x74,0x61,0x6c,0x5f,
    0x73,0x74,0x64,0x6c,0x69,0x62,0x3e,0x0a,0x23,0x69,0x6e,0x63,0x6c,0x75,0x64,0x65,
    ...
]  use array is too long, hard to read and can't easy to modify T.T
]#
const fsSourceMetalMacos = """
  #include <metal_stdlib>
  #include <simd/simd.h>

  using namespace metal;

  struct main0_out
  {
     float4 frag_color [[color(0)]];
  };

  struct main0_in
  {
     float4 color [[user(locn0)]];
  };

  #line 10 "examples/shaders/triangle.glsl"
  fragment main0_out main0(main0_in in [[stage_in]])
  {
     main0_out out = {};
  #line 10 "examples/shaders/triangle.glsl"
     out.frag_color = in.color;
     return out;
  }
"""
proc triangleShaderDesc*(backend: sg.Backend): sg.ShaderDesc =
  case backend:
    of ... :
     ...
      # result.fs.source = cast[cstring](addr(fsSourceMetalMacos))  # not use array so not need cast[]
      result.fs.source = fsSourceMetalMacos
      result.fs.entry = "main0"
      result.label = "triangleShader"
    else: discard

Change sokol_shdc generate nim shader pls. (use string, not use array)

floooh commented 1 year ago

...wait wait wait... what's up with the event function? Does quitting through the window close button no longer work? If possible I would like to avoid having to add an event callback to each sample.

As for the sokol-shdc change: shader source code is usually only used for GLSL, because GL doesn't have a binary format. If possible (e.g. MSL and HLSL) the shaders will be included as bytecode. The Nim samples just take the source code shortcut because they include the shader code for all platforms, but binary MSL can only be generated on Macs, and binary HLSL only on Windows (because this requires the Metal and HLSL shader compilers to be available).

As such dumping the shader sources as ASCII would only really make sense for GLSL.

(in the beginning, sokol-shdc actually wrote shaders sources as strings in the C backend, but that caused some sort of trouble, which specifcally I don't currently remember - might be irrelevant for non-C languages though => I'll think about the change, but can you write a reminder ticket here please? https://github.com/floooh/sokol-tools/issues).

Anyway: can you reduce this PR to just the unsafeAddr => addr change please? Then we can discuss whether a separate PR for the event-function makes sense (I'm not a fan if the only effect is that the samples can be quit with a key press, it just adds unnecessary noise to the samples).

Angluca commented 1 year ago
# String mainly depends on the file type, 
# utf8 is best filetype for all code file
#--c-----
char sz[] = {49, 50, 51, 52, 53, 0};
char str[] = "12345";
assert( !strcmp(sz, str) );
#--nim---
var sz = [49'u8, 50, 51, 52, 53, 0] # type is array[6,uint8] or uncheckArray[uint8]
var cstr: cstring = "12345"
var str: string = "12345"
assert c_strcmp(cstr, cast[cstring](sz.addr))
assert cstr == cast[cstring](sz.addr) # all "12345" like c
assert str == cstr # str.cstring == cstr

Maybe you use array u8 value is best way, Because not every one used editor filetype default is utf8 if want modify, I'm a newbie don't know more T.T, Hope sokol will be better. Test on macos, Window close button is ok, I'm lazy T.T so added event function, can remove it .

floooh commented 1 year ago

Ok merged, thanks!

I'll also need to fix sokol-shdc though. I'll try to do that today.

floooh commented 1 year ago

Hmm, I wonder if depending on a development feature was such a great idea, instead of waiting until it makes it into stable. I have a hard time testing on macOS, because the homebrew version doesn't support that unsafeAddr() => addr() fix yet (homebrew install nim 1.6.14)

floooh commented 1 year ago

...and "choosenim" sucks on macOS because it requires a separate path setup...

(ok I got it working, still wondering how useful that change is, is there any ETA when the unsafeAddr() => addr() change will be in Nim stable? I'm a bit concerned because that change already seems to have happened in Feb-2022, but is still stuck in the development version)

floooh commented 1 year ago

Ok, sokol-shdc tool has been updated, shaders have been rebuilt, and I added a note about using the Nim development version to the readme.