Update sdlthread.inc to version 2.26.2 #114
Conversation
| function SDL_TLSSet(id: TSDL_TLSID; value: Pointer; destructor_: Pointer): cint32 cdecl; external SDL_LibName {$IFDEF DELPHI} {$IFDEF MACOS} name '_SDL_TLSSet' {$ENDIF} {$ENDIF}; | ||
| * Set the current thread's value associated with a thread local storage ID. | ||
| * | ||
| * The function prototype for `destructor` is: |
There was a problem hiding this comment.
Should we introduce a function-pointer type for the destructor? We'd deviate slightly from the SDL API, but get better type checking in return.
There was a problem hiding this comment.
Something along the lines as following?
TTLSDestructor = procedure(value: Pointer); cdecl;
function SDL_TLSSet(id: TSDL_TLSID; const value: Pointer; destructor_: TTLSDestructor): cint; cdecl; external SDL_LibName {$IFDEF DELPHI} {$IFDEF MACOS} name '_SDL_TLSSet' {$ENDIF} {$ENDIF};
Would accompany this by a comment to explain why we did it this way.
There was a problem hiding this comment.
Thanks for your replies :). I have not a lot of time, so I reply only to this part at the moment.
On reconsidering this, I fear I do not like the idea. I fully agree that it would be beneficial to have type-checking, but 1) we generally try to stay as close as possible to the original C header code, 2) someone using threads should be able to make sure it is really a destructor function pointer by himself, 3) this would raise the question about other function pointers, how should we treat them, always introduce a specific type (or is there a special reason why we should introduce a type-check explicitly for this function pointer)?
Best regards
Matthias
There was a problem hiding this comment.
On closer examination I saw that the original headers use a function pointer (in contrast to a simple pointer), too. Therefore I implemented the function pointer as discussed here.
units/sdlthread.inc
Outdated
| { SDL2-for-Pascal #todo : Explanation needed } | ||
| TpfnSDL_CurrentBeginThread = function( | ||
| SecurityAttributes: Pointer; StackSize: LongWord; ThreadFunc: TThreadFunc; | ||
| Parameter: Pointer {arg}; CreationFlags: LongWord; var ThreadId: TThreadID {threadID}): cuintptr_t; cdecl; |
There was a problem hiding this comment.
- The signature here uses Pascal's LongWord. Should we use
cuintinstead? In the SDL headers (and Microsoft documentation) these arguments are of typeunsigned. - In the C headers, the last argument is a pointer. Should we replace the pass-by-reference?
There was a problem hiding this comment.
- I agree and fixed this.
- I'm not sure why
ThreadFuncandThreadIDrely on Pascal system definitions (TThreadFuncfrom RTL'sthreadh.incandTThreadIDfrom RTL'ssysosh.inc) There is no pointer type of TThreadID defined, which means we would need to introduce it by ourselves which would be kind of ugly to mix in a pointer type definition for a RTL type within the SDL unit.
I wonder if we should decouple this altogether like:
type
TpfnSDL_CurrentBeginThreadFunc = function(params: Pointer):cuint; cdecl;
TpfnSDL_CurrentBeginThread = function(
SecurityAttributes: Pointer; StackSize: cuint; func: TpfnSDL_CurrentBeginThreadFunc;
Parameter: Pointer {arg}; CreationFlags: cuint; ThreadId: pcuint {threadID}): cuintptr_t; cdecl;
|
|
||
| { SDL2-For-Pascal: #note : In the C header are two versions | ||
| of these macro functions. One for SDL's dynamic API and one without. | ||
| We can go with this for the moment. Improvement surely possible. } |
There was a problem hiding this comment.
Looking at SDL's source code, the library by default is built in the "dynamic API" mode, and building without it requires manually editing the source. Based on that, I think we should translate the "dynamic API" versions of SDL_CreateThread() and SDL_CreateThreadWithStackSize(). Since said macros call SDL_CreateThread_REAL() and SDL_CreateThreadWithStackSize_REAL(), we could avoid having to overload the identifiers.
| function SDL_TLSSet(id: TSDL_TLSID; value: Pointer; destructor_: Pointer): cint32 cdecl; external SDL_LibName {$IFDEF DELPHI} {$IFDEF MACOS} name '_SDL_TLSSet' {$ENDIF} {$ENDIF}; | ||
| * Set the current thread's value associated with a thread local storage ID. | ||
| * | ||
| * The function prototype for `destructor` is: |
|
@Free-Pascal-meets-SDL-Website Can you sync this PR with the latest |
I sync'd it. No new functions got introduced. |
|
Looks ok, though my comment regarding the dynamic API stuff for Windows still stands. I can take care of that in a follow-up PR, if you'd like. |
Sure, would be great. |
TpfnSDL_CurrentBeginThread,TThreadID); nobody ever complained about these structures => raises the question, if anybody uses SDL threads in Pascal anyway, since Pascal provides high quality procedural and object-oriented thread management