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

Allow to deffer subroutine/constant calling #4

Closed
wants to merge 2 commits into from

Conversation

mnbv09870987
Copy link

I am doing perl binding for SDL2 https://www.libsdl.org/download-2.0.php

in include/SDL_pixels.h there is next enum:

typedef enum
{
    SDL_PIXELFORMAT_UNKNOWN,
    ...
    SDL_PIXELFORMAT_RGBA8888 =
        SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED32, SDL_PACKEDORDER_RGBA,
                               SDL_PACKEDLAYOUT_8888, 32, 4),
   ...
    /* Aliases for RGBA byte arrays of color data, for the current platform */
#if SDL_BYTEORDER == SDL_BIG_ENDIAN
    SDL_PIXELFORMAT_RGBA32 = SDL_PIXELFORMAT_RGBA8888,
    SDL_PIXELFORMAT_ARGB32 = SDL_PIXELFORMAT_ARGB8888,
    SDL_PIXELFORMAT_BGRA32 = SDL_PIXELFORMAT_BGRA8888,
    SDL_PIXELFORMAT_ABGR32 = SDL_PIXELFORMAT_ABGR8888,
#else
    SDL_PIXELFORMAT_RGBA32 = SDL_PIXELFORMAT_ABGR8888,
    SDL_PIXELFORMAT_ARGB32 = SDL_PIXELFORMAT_BGRA8888,
    SDL_PIXELFORMAT_BGRA32 = SDL_PIXELFORMAT_ARGB8888,
    SDL_PIXELFORMAT_ABGR32 = SDL_PIXELFORMAT_RGBA8888,
#endif
    ....
} SDL_PixelFormatEnum;

I convert it like next:

# required forward declaration
sub SDL_PIXELFORMAT_RGBA8888();

$ffi->load_custom_type('::Enum', 'SDL_PixelFormatEnum',
	{ ret => 'int', package => 'SDL2::Pixels' },
	'SDL_PIXELFORMAT_UNKNOWN',
        ....
    [ SDL_PIXELFORMAT_RGBA8888 => SDL_DEFINE_PIXELFORMAT(
		SDL2::Pixels::SDL_PIXELTYPE_PACKED32, SDL2::Pixels::SDL_PACKEDORDER_RGBA,
		SDL2::Pixels::SDL_PACKEDLAYOUT_8888, 32, 4
	)],
	SDL2::Endian::SDL_BYTEORDER == SDL2::Endian::SDL_BIG_ENDIAN? (
	    [SDL_PIXELFORMAT_RGBA32 => SDL_PIXELFORMAT_RGBA8888],
	    [SDL_PIXELFORMAT_ARGB32 => SDL_PIXELFORMAT_ARGB8888],
	    [SDL_PIXELFORMAT_BGRA32 => SDL_PIXELFORMAT_BGRA8888],
	    [SDL_PIXELFORMAT_ABGR32 => SDL_PIXELFORMAT_ABGR8888],
	) : (
	    [SDL_PIXELFORMAT_RGBA32 => SDL_PIXELFORMAT_ABGR8888],
	    [SDL_PIXELFORMAT_ARGB32 => SDL_PIXELFORMAT_BGRA8888],
	    [SDL_PIXELFORMAT_BGRA32 => SDL_PIXELFORMAT_ARGB8888],
	    [SDL_PIXELFORMAT_ABGR32 => SDL_PIXELFORMAT_RGBA8888],
	)
);                           

because of ?: works first before load_custom_type is called. I get error:

Undefined subroutine &SDL2::Pixels::SDL_PIXELFORMAT_ABGR8888 called at ...

This patch allow to defer call to SDL_PIXELFORMAT_ABGR8888. Now declaration looks like:

	SDL2::Endian::SDL_BYTEORDER == SDL2::Endian::SDL_BIG_ENDIAN? (
	    sub{ [SDL_PIXELFORMAT_RGBA32 => SDL_PIXELFORMAT_RGBA8888] },
	    sub{ [SDL_PIXELFORMAT_ARGB32 => SDL_PIXELFORMAT_ARGB8888] },
	    sub{ [SDL_PIXELFORMAT_BGRA32 => SDL_PIXELFORMAT_BGRA8888] },
	    sub{ [SDL_PIXELFORMAT_ABGR32 => SDL_PIXELFORMAT_ABGR8888] },
	) : (
	    sub{ [SDL_PIXELFORMAT_RGBA32 => SDL_PIXELFORMAT_ABGR8888] },
	    sub{ [SDL_PIXELFORMAT_ARGB32 => SDL_PIXELFORMAT_BGRA8888] },
	    sub{ [SDL_PIXELFORMAT_BGRA32 => SDL_PIXELFORMAT_ARGB8888] },
	    sub{ [SDL_PIXELFORMAT_ABGR32 => SDL_PIXELFORMAT_RGBA8888] },
	)

and works without problem

@@ -167,11 +167,6 @@ subtest 'define errors' => sub {
match qr/rev must be either 'int', or 'str'/,
);

is(
Copy link
Member

Choose a reason for hiding this comment

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

Instead of removing this test, please substitute another unsupported type, like a scalar ref.

@@ -202,6 +202,9 @@ sub ffi_custom_type_api_1
foreach my $value (@values)
{
my $name;
if( ref $value eq 'CODE' ) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if( ref $value eq 'CODE' ) {
if(is_coderef $value) {

You will need to import is_coderef from Ref::Util

Copy link
Member

@plicease plicease left a comment

Choose a reason for hiding this comment

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

Need to document the new usage.

@plicease
Copy link
Member

I'm not 100% sure this is the best way to handle enum aliasing tbh, I think there might be a better interface.

Copy link
Member

@plicease plicease left a comment

Choose a reason for hiding this comment

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

Also a test should be added to test passing a subref in as a value.

@plicease
Copy link
Member

I think a more general approach would be something like:

$ffi->load_custom_type('::Enum', 'foo_t',
  'default',
  'better',
  ['best' => 12, alias => ['abc','xyz']],
);

@plicease plicease mentioned this pull request Jun 12, 2020
2 tasks
@plicease
Copy link
Member

I think I prefer #6 which doesn't rely on the load order. I think you should still be able to do what you need.

@plicease plicease changed the base branch from master to main July 1, 2020 18:45
@plicease plicease closed this Sep 27, 2020
@plicease plicease mentioned this pull request Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants