Skip to content
This repository was archived by the owner on Nov 3, 2023. It is now read-only.

Conversation

@Y-Less
Copy link

@Y-Less Y-Less commented Jun 14, 2019

Major additions:

  1. alias attributes are parsed to generate new versions of commands/structs/enums pointing at the old versions. Usually when an extension has been moved in to the main spec. Also deprecates the old version. Commands will give a warning, structs are implicitly converted, enums reference each other.

Examples:

public void CmdSetDeviceMask (UInt32 deviceMask)
{
	unsafe {
		Interop.NativeMethods.vkCmdSetDeviceMask (this.m, deviceMask);
	}
}

[Obsolete ("CmdSetDeviceMaskKHR is deprecated, please use CmdSetDeviceMask instead.")]
public void CmdSetDeviceMaskKHR (UInt32 deviceMask)
{
	unsafe {
		Interop.NativeMethods.vkCmdSetDeviceMaskKHR (this.m, deviceMask);
	}
}
public enum StructureType : int
{
	...
	PhysicalDeviceBufferDeviceAddressFeaturesExt = 1000244000,
	PhysicalDeviceBufferAddressFeaturesExt = PhysicalDeviceBufferDeviceAddressFeaturesExt,
	...
}
unsafe public partial class PhysicalDeviceFeatures2Khr : PhysicalDeviceFeatures2
{
	public PhysicalDeviceFeatures2Khr () : base () {
	}

	internal PhysicalDeviceFeatures2Khr (NativePointer pointer) : base (pointer) {
	}
}
  1. feature tags are now parsed in much the same way as extension tags.

  2. extnumber attributes are optionally used for determining enum values.

  3. Added a few more basic types: zx_handle_t, DeviceAddress, HMONITOR, GgpStreamDescriptor, and GgpFrameToken.

  4. Recognise the INTEL vendor suffix.

  5. Currently just disables several android- and metal-specific extensions, as they couldn't be generated correctly.

  6. Also disables a few commands that needed pointers to pointers, and fixed pointer arrays (was already listed as TODO in the generator before I touched anything, so this isn't actually a change).

  7. Now pulls the latest vk.xml file from the Vulkan docs, not just pinned to 1.0.

Y-Less added 30 commits June 12, 2019 18:48
We could continue to use the old functions, but currently the code doesn't save any prototype information for functions.  Thus our alias can't have the same prototype as the original, so can't have its full code generated.  Instead, for now we just generate a very very simple import as:

```csharp
[Obsolete("{0} is deprecated, please use {1} instead.", true)]
[DllImport (VulkanLibrary, CallingConvention = CallingConvention.Winapi)]
private static unsafe extern void {0} ();
```

And hope that is sufficient.
I tried for a while to figure out how to support this.  The fundamental issue is that `fixed` must (by definition) be a fixed size array.  But this is an array of pointers, which varies in size by platform.  In C that's not an issue, just compile for both, in C# you can't do that.
One set some types that didn't need to be hard-coded.

The other fixed newlines within functions, but cluttered the resulting generated diff.
@msftclas
Copy link

msftclas commented Jun 14, 2019

CLA assistant check
All CLA requirements met.

@Y-Less Y-Less closed this Jun 17, 2019
@radekdoulik
Copy link
Contributor

@Y-Less did you close the PR by a mistake?

@radekdoulik radekdoulik reopened this Jun 17, 2019
@Y-Less
Copy link
Author

Y-Less commented Jun 17, 2019

No, I realised a bug, I'm just working on fixing it. Bit flags moved from extensions to the main spec didn't generate their aliases properly. I was going to reopen it when that was done.

@Y-Less
Copy link
Author

Y-Less commented Jun 17, 2019

It also doesn't currently generate complete enum aliases, so these lines are ignored:

        <type category="enum" name="VkPeerMemoryFeatureFlagBitsKHR"                alias="VkPeerMemoryFeatureFlagBits"/>

And thus enum PeerMemoryFeatureFlags is generated, but enum PeerMemoryFeatureFlagsKhr isn't, which would be required for backwards-compatibility.

@radekdoulik
Copy link
Contributor

OK, I started fixing the whitespace issues meanwhile, so that it is easier for me to review. Let me know when it is ready and I will get back to it.

Thanks for all the work!

@Y-Less
Copy link
Author

Y-Less commented Jun 17, 2019

Sorry about the whitespace. I've fixed that now, along with I hope the other bits. The main change being that it will now generate:

	[Flags]
	public enum PeerMemoryFeatureFlags : int
	{
		CopySrc = 0x1,
		CopyDst = 0x2,
		GenericSrc = 0x4,
		GenericDst = 0x8,
		CopySrcKhr = CopySrc,
		CopyDstKhr = CopyDst,
		GenericSrcKhr = GenericSrc,
		GenericDstKhr = GenericDst,
	}

	[Obsolete ("PeerMemoryFeatureFlagsKhr is deprecated, please use PeerMemoryFeatureFlags instead.")]
	[Flags]
	public enum PeerMemoryFeatureFlagsKhr : int
	{
		CopySrc = 0x1,
		CopyDst = 0x2,
		GenericSrc = 0x4,
		GenericDst = 0x8,
		CopySrcKhr = CopySrc,
		CopyDstKhr = CopyDst,
		GenericSrcKhr = GenericSrc,
		GenericDstKhr = GenericDst,
	}

@Y-Less
Copy link
Author

Y-Less commented Jun 17, 2019

One thing I should flag is this:

9b34dc0

The old generated code contained three enum flag bits whose naming didn't match any others. Most enums have the Bit part of the name removed, except for those three. After I changed some other bits of code they also lost the Bit infix, so I manually put it back for backwards-compatibility. Maybe they should be marked [Obsolete] (maybe all the aliases should).

@radekdoulik
Copy link
Contributor

I have created an api-check rule today to analyze the API changes and looks like there are still few breaking changes.

I will go thru them hopefully tomorrow to see what to do with them.

I am attaching the api-check results: api-changes.zip

@Y-Less
Copy link
Author

Y-Less commented Jun 19, 2019

Unfortunately a lot of them it isn't obvious what the correct response is. Some of them they renamed the functions (for example KHX -> KHR (which doesn't seem to have been done with any aliasing, likely as it was experimental before and now made official). Some were promoted from extensions to the main spec, I've tried to keep this backwards-compatible, but that also not always clear how. If a struct is now an alias of another one, should the old method return the new name or the old name? And how can we make them interchangable, such that the result of an older KHR function can be passed to a new renamed main spec function?

The changed base objects in that report are just my method of aliasing some things, so it is still the same base object transitively, and the removed properties are instead now inherited.

But for many of these decisions I think someone far more experienced should look at it, and compare to existing code. TBH I did this as a way to learn the library and Vulkan when I discovered that this library didn't support the new debugging interface in the main vulkan-tutorial.

Base automatically changed from master to main March 10, 2021 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants