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

ADD: Support for dynamic linking first version that can be compiled u… #149

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

PascalCorpsman
Copy link

Hello all,

based on the discussion ev1313/Pascal-SDL-2-Headers#78 (comment) i wrote a little FPC Transpiler and switched now my complete work over to this repository.

In Order to create the least impact possible, i choosed to do "inplace" modifications for the code switchings for dynamic / static linked version.

The static linked version is 99% unchanged (only needed to rename the two overloaded functions from sdl2.pas).

The dynamic linked version does compile under Windows64 and Linux64, under Linux64 i was also able to test the joystick part (and that worked ;) )

So if you choose to migrate this PR then all users who are using the SDL2.pas at least need to do this step:

(*

  • If you get a compiler error with missing file

  • just create a file namend "sdl2_cfg.inc" in your project folder and

  • insert the following content:

  • ---------- Content of file ---------
    {*

  • set this define if you want to use dynamic linking instead of static linking

  • ! Attention !

  • Not all functions are "ported" yet, see existing "working" code on how to

  • port missing functions.
    *}
    {.$DEFINE SDL_DYNAMIC_LINKING}

  • ---------- End content of file ----------

  • ! Attention !

  • If you use the dynamic link feature, don't forget to call the SDL_LoadLib

  • function.
    *)

And yes there is still plenty work to do, to have a fully supported dynamic linked SDL2.pas, but beside the already mentioned differences you could stay at the static linked version with nearly no change and at least the "basic" dynamic linked version does now exist ;).

I think if you merge this code base, maybe other interested users could finish the work i started here (y).

@PascalCorpsman
Copy link
Author

One Question it seems that i upladed the files with the wrong line endings, which one is the one of the project, than i will try to correct this ?

@Free-Pascal-meets-SDL-Website
Copy link
Collaborator

Thanks for this overwhelming PR; wow but a hell of a commit :D

I guess we will need some time to review this.

@suve
Copy link
Collaborator

suve commented Nov 20, 2024

We're using \n line endings in the project; your PR seems to rewrite most of the files to use \r\n instead, hence the humongous diff.

@PascalCorpsman
Copy link
Author

PascalCorpsman commented Nov 21, 2024

ok i reworked the code to change the lineendings, now there are a lot less changed lines :)

@suve
Copy link
Collaborator

suve commented Nov 21, 2024

Why create a separate commit? Please amend the original one, instead.

Looking through the PR, this creates an SDL_DYNAMIC_LINKING define which changes all function definitions to a pair of typedef + function pointer. The idea's simple, but it creates a lot of extra churn in the code, to the point where I'm wondering if it'd make sense to store the definitions in some templated form and have a program generate the final .pas/.inc files.

Another thing is that the nomenclature is wrong - dynamic linking is what we're doing right now. This switches the library to runtime loading.

@PascalCorpsman
Copy link
Author

sry i do not know how to amend a commit.

if i rename

{.$DEFINE SDL_DYNAMIC_LINKING}

to

{.$DEFINE SDL_RUNTIME_LOADING}

then we solved the nomenclature thing.

Your point according the "Churn" thing is surely right, but maybe this should be part of work after you merged the PR ?

@Free-Pascal-meets-SDL-Website
Copy link
Collaborator

@PascalCorpsman May I ask, did you add all the ifdef's by hand? Or did you use some kind of a script?

Could you please change the function pointer suffixes to "func" instead of "fun". I mean, this library is fun indeed, but this suffix choice if just funny :D.

One thing I see as kind of a problem: To stay consistent in the future, merging this (without the proposed template solution) would force the project to add the ifdefs for all new definitions. On the other hand this is a "hot" feature and it may be worth it.

I'm also a little bit concerned about the pasdoc generator, it will not recognize the comments anymore, especially since pasdoc also has no groupBegin/groupEnd feature to my knowledge.

@PascalCorpsman
Copy link
Author

May I ask, did you add all the ifdef's by hand? Or did you use some kind of a script?
Lucky me, the code in the .inc files is really good structured, so i could do this with some easy string manipulation functions, and yes initially this was a program written in Lazarus, i only needed to adjust less than 10 functions by hand to get it compiled.

I attached the program, but be aware that this code is a complete mess, as it was created to run exactly once and then thrown away ..
inc_converter.zip

Could you please change the function pointer suffixes to "func" instead of "fun". I mean, this library is fun indeed, but this suffix choice if just funny :D.
did ;)

as i already suggested, if you come up with a good template solution this should be done, all i wanted to do, is showing how it could be done and that it is doable. And at least for my fpc_atomic project it already does the trick ;)

@Free-Pascal-meets-SDL-Website
Copy link
Collaborator

Looking through the PR, this creates an SDL_DYNAMIC_LINKING define which changes all function definitions to a pair of typedef + function pointer. The idea's simple, but it creates a lot of extra churn in the code, to the point where I'm wondering if it'd make sense to store the definitions in some templated form and have a program generate the final .pas/.inc files.

Maybe our original static definitions should be the template.

@PascalCorpsman I really appreciate your commitment a lot, I still wonder if there is a more elegant way of doing it. As @suve pointed out, the extra definitions overload our translation code. Also it contradicts our principle to just translate the basic headers one-to-one. Extra features should be kind of separate. - I still had no time examining your code though. :(

@PascalCorpsman
Copy link
Author

PascalCorpsman commented Jan 6, 2025

I still had no time examining your code though. :(

i recognised this ;), as i was yesterday able to test the code under Windows and there it crashed due to missing conversions. After fix 1d87255 it now works like intended (at least on Windows and Linux).

And yes if there is a more elegant way to support runtime linking and the orig way i would be really interested in that too, maybe we have luck and some clever guy is reading this and pinpoints the clever solution g

By the way, if you look at the code you will recognize really soon that the port is not completed, as i only needed the joystick parts i only ported them and showed at least one way how the rest could be done, so again finding a elegant way and then porting the complete codebase to this would be the best way (y)

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