Why 1 item and not 2 items is returned by vkGetPhysicalDeviceSurfacePresentModesKHR?

[sunbearc22]

Hi realitix,

I noticed that for function vkGetPhysicalDeviceSurfacePresentModesKHR, it only returns the presentModes. I was expecting it to return both presentModes and presentModeCount as defined in Spec. Is is possible to return 2 items instead of one item so it follows Vulkan?

I did noticed in line 4463, you defined pPresentModes = ffi.new('VkPresentModeKHR[]', pPresentModeCount[0]) . Did you make pPresentModes a tuple or generator containing the presentMode and the presentModeCount? Pardon me I don't understand ffi.

At the moment, I am using a loop to expose the presentMode and len to expose presentModeCount.

    self.surface_presentModes = self.fnp['vkGetPhysicalDeviceSurfacePresentModesKHR'](
        physicalDevice=self.physical_device, surface=self.surface)
    print('self.surface_presentModes = ', self.surface_presentModes)
    print('len(self.surface_presentModes) = ', len(self.surface_presentModes))
    for mode in self.surface_presentModes:
        print('- available_surface_present_mode = {}'.format(mode) )

Output:

self.surface_presentModes =  <cdata 'enum VkPresentModeKHR[]' owning 12 bytes>
len(self.surface_presentModes) =  3
- available_surface_present_mode = 2
- available_surface_present_mode = 3
- available_surface_present_mode = 0

Appreciate your explanation.

[Berserker66]

I've seen this too, but I liked this behavior. in python we have len() - C does not and I've always had to iterate over the returned lists anyway, I've never needed only the count.

As for what it is doing. ffi.new(<insert type here>, <insert amount here>) gives you an array of amount items of type.

[sunbearc22]

I was thinking that if I see a Vulkan function returning multiple pointers like below,

VkResult vkGetPhysicalDeviceSurfacePresentModesKHR(
    VkPhysicalDevice                            physicalDevice,
    VkSurfaceKHR                                surface,
    uint32_t*                                   pPresentModeCount,
    VkPresentModeKHR*                           pPresentModes);

then with vulkan, I could access them by

self.PresentModeCount, self.PresentModes = vk.vkGetPhysicalDeviceSurfacePresentModesKHR(
        physicalDevice=self.physical_device, surface=self.surface)

Does this format make sense?

[Berserker66]

Yeah, that'd be the pythonic way of doing it, assuming of course, that we ignore that fact that python supports lists/arrays as objects.

II feel like the proper answer here is more philosophical than technical. It would be more "pure" to return the amount, but I don't see a technical reason to do so as it's implicated in the array object.

Then there is the aspect of expected outcome, which for some people would be the expected outcome of the spec, which is fair enough. For me I've learned that people don't always update their documentation and so I've come to accept what my IDE tells me as expected outcome, which in this case just tells me it is one ffi object (but does tell me it is not a tuple).
Would be nice if there were some docstrings in this module, but that'd be icing on a cake, nice to have but not necessary.

Anyway, in the end, up to realitix to decide if anything needs changing here.

[realitix]

Hello @sunbearc22, like @Berserker66 told you, it's on purpose. Indeed it's philosophical. The goal is to be a very thin wrapper BUT it's Python and we can't ignore that. So I had to make a choice to know how far we pythonize this module. The advantage of a small wrapper like that (same functions names, struct names, constant names as the original C version) is that you can easily google the code and find plenty of documentation. It's still a Python module so we have to take advantage of that, it would be useless to return the len when you can write len(return_value). That's why it will stay as is.
I invite you to read the documentation because it's clearly written here.

Thanks, I can close this issue now.

[Berserker66]

Hm. By the same token though, should it not also work this way the other way around then?
You know, for consistency.
So in this example:

instance_create_info = vk.VkInstanceCreateInfo(
    sType=vk.VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO,
    pNext=None,
    flags=0,
    pApplicationInfo=application_info,
    enabledLayerCount=len(layers),
    ppEnabledLayerNames=layers,
    enabledExtensionCount=len(extensions),
    ppEnabledExtensionNames=extensions,
)

enabledLayerCount and enabledExtensionCount should then be automatic - or optional?

[sunbearc22]

@Berserker66 Thanks for sharing your thought.

@realitix,

I read the vkEnumerateDeviceExtensionProperties example. This function returns 3 pointers. So in your convention, how do I access pLayerName, pPropertyCount and pProperties? The document say a list of object is returned. So for

available_logical_device_extensions = vk.vkEnumerateDeviceExtensionProperties(physicalDevice=self.physical_device, pLayerName=None)

I tried

#print('available_logical_device_extensions[0]=',available_logical_device_extensions[0])
#print('available_logical_device_extensions[1]=',available_logical_device_extensions[1])
#print('available_logical_device_extensions[2]=',available_logical_device_extensions[2])

But got an error saying the object is a generator. I then tried below to expose the item

 print(next(available_logical_device_extensions))
 print(next(available_logical_device_extensions))
 print(next(available_logical_device_extensions))

but got a 'StopIteration'.

Your example uses a list comprehension to expose the names of the extensions.
extensionsNames = [e.extensionName for e in available_logical_device_extensions]

But .extensionName was not mentioned in documentation. So how do I discover the name of the attributes of the return object?

Pardon me for asking these questions. My intention is not to advocate pure pythonic way of coding with vulkan. Instead, I want to know how to use it better.

Hope you can advice me.

VkResult vkEnumerateDeviceExtensionProperties(
    VkPhysicalDevice                            physicalDevice,
    const char*                                 pLayerName,
    uint32_t*                                   pPropertyCount,
    VkExtensionProperties*                      pProperties);

[Berserker66]

What you are getting returned from vkEnumerateDeviceExtensionProperties is - assuming it was called correctly - pProperties, which is an array of VkExtensionProperties. If you look that structure up, :
https://www.khronos.org/registry/vulkan/specs/1.0/man/html/VkExtensionProperties.html

You can find that it has the extensionName and specVersion fields.
So you can do:
vkEnumerateDeviceExtensionProperties([....])[0].extensionName.
Assuming you get at least one extensionproperty.

[Berserker66]

Slightly unrelated, but will likely help you. This exists:
https://gist.github.com/inactivist/4ef7058c2132fa16759d
You can tell it to convert any cffi structure to a dict and then pretty print that to get an idea of what you have. It certainly helped me. However, the version as posted there did not work for me, likely due to a different version of cffi, so here's the one fixed up by me, which may work better for you:
https://pastebin.com/SC3gtnas

[sunbearc22]

@Berserker66 Thanks for your explanations. Also for the conversion scripts. I will try it out later in the day.

[realitix]

Good point @Berserker66 about the other way around. Indeed the api is not consistent. I will fix it. The default value will be the len.

[sunbearc22]

@Berserker66 I was trying your script but encountered error. Can you explain?
I had saved your script into a file called convertstruct.py.
In my python code, my import statement is:
from convertstruct.py import *
Within my class and one of its function, I wrote:

Class Setup:

    def _createSwapChain(self):
        self.surface_formats = self.fnp['vkGetPhysicalDeviceSurfaceFormatsKHR'](
            physicalDevice = self.physical_device,
            surface = self.surface )
        surfaceformat = ['format','colorSpace']
         __convert_struct_field( self.surface_formats, surfaceformat )

However, I got:

__convert_struct_field(self.surface_formats,surfaceformat)
NameError: name '_Setup__convert_struct_field' is not defined

I also tried

        for value in __convert_struct_field(self.surface_formats,surfaceformat):
            print(value)

But also got the same error:

    for value in __convert_struct_field(self.surface_formats,surfaceformat):
NameError: name '_Setup__convert_struct_field' is not defined

However, simple print statement works:

        for f in self.surface_formats:
            print('    surface_format = ', f )
            print('    surface_format.format = ', f.format )
            print('    surface_format.colorSpace = ', f.colorSpace )

I got:

    surface_format =  <cdata 'struct VkSurfaceFormatKHR &' 0x2204ad0>
    surface_format.format =  44
    surface_format.colorSpace =  0
    surface_format =  <cdata 'struct VkSurfaceFormatKHR &' 0x2204ad8>
    surface_format.format =  50
    surface_format.colorSpace =  0

How do I resolve the NameError i am getting? I believe it should be '_Setup.__convert_struct_field'. I am sure it is something silly I am missing out but can't seem to figure it?

[Berserker66]

If you look at the pastebin again, you can see that __convert_struct_field is the first function it defines. That is what you need to call. I don't get where the "_Setup" bit comes from at all, it seems you're including the class name for some reason when this is a function, not a method.

Edit: derp - I just noticed. You're not meant to call "__convert_struct_field" yourself, this is meant to be called by convert_to_python only. You use convert_to_python(<cffi type>) to get a python dict.

It is python convention - I think PEP 8 - that _ and __ are the closest thing to a private function in python, that is not meant to be called by a "user application" of a module.

[sunbearc22]

@Berserker66 Noted your 3 points. `convert_to_python(s) worked partially. I tried

         self.surface_capabilities = self.fnp[
            'vkGetPhysicalDeviceSurfaceCapabilitiesKHR'](
                physicalDevice=self.physical_device, surface=self.surface)
        for x in convert_to_python(self.surface_capabilities):
            print(x)
        for x in convert_to_python(self.surface_capabilities.currentExtent):
            print(x)

Output is given below and is correct:

maxImageArrayLayers
minImageCount
supportedTransforms
minImageExtent
supportedCompositeAlpha
currentTransform
currentExtent
maxImageExtent
supportedUsageFlags
maxImageCount
width
height

However, it did not work on surface Format:

        self.surface_formats = self.fnp['vkGetPhysicalDeviceSurfaceFormatsKHR'](
            physicalDevice = self.physical_device,  surface = self.surface )
        for x in convert_to_python(self.surface_formats):
            print(x)

Error message:

    return [ convert_to_python(s[i]) for i in range(ffitype.length) ]
TypeError: 'NoneType' object cannot be interpreted as an integer

Do you know what is causing this error?

The reason I did not try convert_to_python( getattr( s, field ) was because I thought it was the child of __convert_struct_field( s, fields ) because of the line yield (field, convert_to_python( getattr( s, field ) )) it has, i.e. __convert_struct_field( s, fields ) was calling convert_to_python(s).

The other thing I do not understand about function convert_to_python(s) is why it called itself in the line return [ convert_to_python(s[i]) for i in range(ffitype.length) ]. I did not think it is possible. Typo?

[Berserker66]

Mh, that is an interesting anomaly. ffitype.length for the surface formats is None. despite it reporting it is a type of array.
This might be a bug in cffi. It feels wrong. The data is instead in item.. it is rather weird. I can hack a patch that makes it work, but not sure if we may have found a cffi bug.

The structure, using PyCharm debugger:
https://i.imgur.com/fALm8F3.png

[Berserker66]

This is the patch that makes it work:

def __convert_struct_field( s, fields ):
    for field,fieldtype in fields:
        if fieldtype.type.kind == 'primitive':
            yield (field,getattr( s, field ))
        else:
            yield (field, convert_to_python( getattr( s, field ) ))

def convert_to_python(s):
    if type(s) == int:
        return s
    ffitype=ffi.typeof(s)
    if ffitype.kind == 'struct':
        return dict(__convert_struct_field( s, ffitype.fields ) )
    elif ffitype.kind == 'array':
        if ffitype.item.kind == 'primitive':
            if ffitype.item.cname == 'char':
                return ffi.string(s)
            else:
                return [ s[i] for i in range(ffitype.length) ]
        else:
            if ffitype.length != None:
                return [ convert_to_python(s[i]) for i in range(ffitype.length) ]
            else:
                return [convert_to_python(s[0])]
    elif ffitype.kind == 'primitive':
        return int(s)

So yeah, it is an array with 1 item, but it says the length of the array is None instead of 1.

Edit: Is this how cffi handles a *void ?

[sunbearc22]

@Berserker66 Thanks. Patch works. Noticed the changes here:

        else:
            if ffitype.length != None:
                return [ convert_to_python(s[i]) for i in range(ffitype.length) ]
            else:
                return convert_to_python(s[0])

Can you tell me why it is possible for a function to call itself? Would this not result in an infinite loop? Why not write:

        else:
            if ffitype.length != None:
                return [ s[i] for i in range(ffitype.length) ]
            else:
                returns s[0]

?

[Berserker66]

This is basically programming 101 - but anyway.

Functions can call themselves, this is called recursion:
https://en.wikipedia.org/wiki/Recursion_(computer_science)

Yes, you can end up with an infinite loop. Here's some things why you don't:

1. Python protects you against infinite loops and has a maximum recursion depth, you can should it ever happen, gracefully try - except out of it.
2. There is no reason for a vulkan structure ever to reference itself to create such a loop
3. The function would then encounter a dict object, something it can't handle and would raise an exception, long before going infinite.

Your code just returns back the original as it was, unchanged. You still end up with a structure, just the same (or parts of a struct). The intended result of that function is to "convert to python". Python, by default, does not use C structures, it would therefore then fail its intended functionality.

[sunbearc22]

@Berserker66 Hey, thanks for your explanations. It brightened up my day. ;)
@realitix I found the helper functions shared by @Berserker66 useful, I like to suggest that they be appended to vulkan to help users, such as in a module of helper functions. What do you think? @Berserker66 are you ok with this idea?.

[Berserker66]

Well, that is a much more complex discussion. Maybe we should open a new issue for that.
Since it was asked here, I'll give my thoughts.

So far, this module has been a very faithful and direct vulkan implementation, no bells and whistles (apart from some pythonizing of exceptions and i/o). However, as you call it, there are no helper functions.

We can look at other projects that do this, for example pyglet, which is a close comparison.

Pyglet exposes the entire OpenGL interface, and you can use it like that. But really, you'll likely dig into its convenience functions, like a framerate counter, shader handling, batching etc. and you can intermix its stuff with your own.

I've used pyglet in the past for two projects and it was nice. Admittedly I implemented shaders on my own before pyglet did - but anyway - such convenience functions can be nice, but if such a road is taken it should be split off well. A submodule or another project entirely that has this project as dependency.

There is merit to a pure vulkan copy, especially when you get to people porting code from python to something else or from something else to python, porting is a lot easier if you just need to port the language and not also the api. In other words, I don't want convenience functions to "pollute" the pure vulkan interface.

A collection of helper functions is something I don't think any user is going to say no to, it's always a nice to have, but it is also work to maintain and document it all. As it stands, this module can just point at the vulkan spec and say - just do this and doesn't need much documentation of its own.

At the end of the day, I would support a helper module, but caution that; that would require someone to maintain it, make unittests and documentation for it.

[sunbearc22]

@realitix has provided an excellent vulkan wrapper to Vulkan. It is what it calms to be. Also, I know vulkan does serves as the basis for vulk and there I saw many helper functions relating to math and graphics, which gives enthusiast like myself better understanding on how to use vulkan to write a Vulkan app.

As to my suggestion, I am limiting my consideration to functions that help expose the cffi data returned by vulkan; not so much about helper functions that could be useful in vulk. As you both know, I am not conversant in cffi and so thought that the functions shared by @Berserker66 can help non-literate cffi users like myself to better appreciate and use vulkan.

After reading @Berserker66 opinions, I do appreciate that it does take effort to maintain helper functions. If @realitix does agree to go this route, I can help do a short write-up on the functions, which I think can be added to the section on function in the readme document.

[realitix]

Hi @sunbearc22, indeed it's nice to have helper functions. My vulk project can help for that. Like @Berserker66 said, it's better to create a separate project dedicated to these helper functions.

[sunbearc22]

Noted @realitix. Thank @Berserker66 again for your helper functions.