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

[lua] FFI function binding #4445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

lampysprites
Copy link
Contributor

@lampysprites lampysprites commented May 1, 2024

Hello! I saw #4123, and had something done about that locally by compiling cffi statically with lua. Which is close to what luajit has - alternative to "luaopen_xxxxx" type of shared libraries, and unlike it, it binds normal dll's without initializing them with lua state. It's still a workaround though, which might end up bothersome to support.

Changes:

  • added ffi global to lua, which is the cffi-lua module
  • added libbfi and cffi-lua submodules, both of which i patched a bit - cmake file in libffi, and some overloads in cffi-lua that don't pass msvc. (upd: no longer needed)
  • added ENABLE_FFI compilation flag

Benefits are pretty obvious:

  • compared to loadlibrary, ABI and compiler compatibility becomes irrelevant.
  • compared to interpreter, way way faster execution - namely, for anything that requires reading/writing image data.
  • allows to do a whole lot of things with only limitation that the application state is not shared.

Downsides:

  • no security whatsoever, as pointed out by @ dacap on Provide support for requiring c libraries in scripts #4123:
  • loadlib stil won't work, thus neither will already existing lua modules.
  • it's harder than scripting - requires some basic understanding of C and memory management. And even though it's copy-paste, it's still bothersome to create bindings for libraries that use transparent pointers.
  • crashes are very easy to enjoy in just a few lines of code.
  • dependency on libdl on linux - could that affect distribution?

Example/tests:
Source and compiled version: ffi_test.zip - includes minimal example; image modification; using external library (miniaudio)

overall a binding can look like this:

local ffi_test = ffi.load("ffi_test.dll")
ffi.cdef("void grayscale(unsigned char * bytes, unsigned long length);")
local function grayscale(image)
  local bytes = image.bytes
  local sz = image.rowStride * image.height
  ffi_test.grayscale(ffi.cast("unsigned char *", bytes), ffi.cast("unsigned long", sz))
  image.bytes = bytes
end

I agree that my contributions are licensed under the Individual Contributor License Agreement V4.0 ("CLA") as stated in https://github.com/igarastudio/cla/blob/main/cla.md

I have signed the CLA following the steps given in https://github.com/igarastudio/cla#signing

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

@lampysprites
Copy link
Contributor Author

sorry abt the mess - i used wrong patch of cffi-lua; but macos issue is something else and it'll take a bit of looking

@lampysprites
Copy link
Contributor Author

Issues with compilation fixed, hopefully.
Also figured how to keep build scripts separately (third_party/ffi-cmake) - everything now works with upstream repos without my oonga-boonga patches.
The last remaining addition i can think of is adding a flag for linking to shared libffi, like it is done for some deps. But also it is how it is not done for other deps, so tell me.

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

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.

None yet

2 participants