Skip to content

Silk.NET 3.0 Proposals #539

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

Merged
merged 17 commits into from
Jul 17, 2022
Merged

Silk.NET 3.0 Proposals #539

merged 17 commits into from
Jul 17, 2022

Conversation

Perksey
Copy link
Member

@Perksey Perksey commented Jul 3, 2021

What is this?

This will be the PR that we take to the big Silk.NET 3.0 working group meeting. All proposals will have their own individual PRs into this branch and PR, and will be merged once they've passed preliminary review. This PR will not be merged until the attendees of the working group meeting are happy.

What proposals are included in this?

@Perksey Perksey marked this pull request as draft July 3, 2021 18:51
@Perksey Perksey added this to the 3.0 milestone Jul 9, 2021
Perksey and others added 7 commits July 10, 2021 21:51
* Create Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Ready for WG review?
* Created Propostal - Enhanced Input Events.md

* Update Proposal - Enhanced Input Events.md

* Update Proposal - Enhanced Input Events.md

* Added ctors - Enhanced Input Events.md

* Applied suggestions - Enhanced Input Events.md

Co-authored-by: Dylan Perks <[email protected]>

Co-authored-by: Dylan Perks <[email protected]>
Perksey and others added 3 commits July 28, 2021 18:59
* Create Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update documentation/proposals/Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update documentation/proposals/Proposal - Multi-Backend Input.md

Co-authored-by: Kai Jellinghaus <[email protected]>

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

Co-authored-by: Kai Jellinghaus <[email protected]>
* Create Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* Apply suggestions from code review

* Update documentation/proposals/Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* missed a closing parenthesis
@Perksey Perksey marked this pull request as ready for review August 3, 2021 22:14
@Perksey
Copy link
Member Author

Perksey commented Aug 5, 2021

Discussed in Working Group meeting:

SilkTouch

  • SilkTouch for 2.0 is very hard to use
  • A lot of code
  • Will explode the repo a lot, but will also improve compile times because everything's already there and no need to generate at compile time
  • ClangSharp is used by win32metadata (official c#, rust bindings) and generally accurate for parsing header files
    • very correct, battle tested, more reliable than BuildTools 2.0
  • Just use ReadOnlySpan (implicit conversion from string)
    • does our userbase know this?
  • Too many overloads could cause confusion/lack of visibility
    • promote "best practice"
    • include exposed native api
  • Only overload what we determine as "best practice", discourage per-parameter overloading
    • One permutation per "overload style"? i.e. one function with all spans
    • Special "intermediary" types don't really make sense as it loses compile-time safety and has other compiler-level issues
  • Establish a baseline of overloads
    • Scrap ArrayOverloads
    • Scrap RefOverloads
  • Group? i.e. only create overloads based on a particular style
  • overloader needs more review/work, postpone to another meeting
    • number of overloads is a big concern right now

Input

  • Document the change in the enhanced input events (migration guide?)
  • Record struct ASAP when available to us, unanimous agreement
    • no need for meeting clarification
  • Make IsConnected less meaningful
  • Return IReadOnlyList<T>
  • Make input states structs (immutable)
    • have a State readonly property
    • have methods for all settable things (where necessary)
  • @HurricanKai To rework Multi Backend Input the proposal slightly

Windowing

not discussed

SDP

not discussed

* Added ISurface.PreUpdate event

The ISurface.PreUpdate event would be used to run things like Input, which need to be updated before the user code's Update is called.

In the current state of the library, depending on how the user hooks to the events, it is possible for the input to hook to the IView.Update after the user code, which would result in the user code not seeing the latest input (until the next update).

* Changed PreUpdate event type to be just Action
Perksey and others added 2 commits February 13, 2022 18:08
* Rename Proposal - Enhanced Input Events.md to (Superseded) Proposal - Enhanced Input Events.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Generation of Library Sources and PInvoke Mechanisms.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - Multi-Backend Input.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - 3.0 & 3.X Software Development Plan.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Windowing 3.0.md

* Update Proposal - Multi-Backend Input.md

* Apply suggestions from code review

Co-authored-by: Kai Jellinghaus <[email protected]>

* Apply suggestions from code review

* Big updates further separating states and devices

* Move ClipboardText to IKeyboard

* Update Proposal - Multi-Backend Input.md

* Update documentation/proposals/Proposal - Multi-Backend Input.md

Co-authored-by: ThomasMiz <[email protected]>

* Add actors

* Add clicks again

* Update Proposal - Multi-Backend Input.md

* Add back connection changed

* Remove Raisin

* IMouse -> IInputDevice for ConnectionChanged

* Review fixes

* Further clarification

* Further clarification

* Rename to IInputHandler

* Apply suggestions from self review

* Update documentation/proposals/Proposal - Multi-Backend Input.md

* Update documentation/proposals/Proposal - Multi-Backend Input.md

Co-authored-by: ThomasMiz <[email protected]>

* Update Proposal - Multi-Backend Input.md

* Update documentation/proposals/Proposal - Multi-Backend Input.md

Co-authored-by: ThomasMiz <[email protected]>

* Remove Up lists

* Fix usage example to compile

Co-authored-by: Kai Jellinghaus <[email protected]>
Co-authored-by: ThomasMiz <[email protected]>
@Perksey
Copy link
Member Author

Perksey commented Feb 25, 2022

SDP

  • Approved.
  • Support Eto.Forms?
    • Not really used or requested compared to the others, maybe as a community thing.
  • There were some questions about the bindings libraries and how the generator differences are going to be consolidated.

@Perksey
Copy link
Member Author

Perksey commented Feb 25, 2022

Windowing

  • Approved.
  • Questions around windows handles
    • What Win32 handles should be available?
      • Mostly what we have for 2.X today, but as a struct
    • StructLayout = Auto for WindowHandles so that people can't depend on the memory layout (so we can add more handles!)
  • Is glue mandatory?
    • Nope, you can use GetOrCreate but SilKEntryPoint will be recommended.

@Perksey
Copy link
Member Author

Perksey commented Feb 25, 2022

Input

  • Approved, but do make the joystick buttons direction instead of Xbox-opinionated ABXY.
  • Structs are good for inputs. Help with versioning and more performant if you pass more than ~4-5 args.
  • Both event-driven input and state-based input available.
    • Call Update with an IInputHandler to get events in the order they were received, pass null to just use the state-based API
    • State will always be updated regardless of null
  • Some examples on using gamepads, keyboards, and mice in the same game would be appreciated.
    • (Silk is fine with you using all simultaneously, you don't need to tell it to switch)
  • e.g. what if you want to assert that spacebar was hit exactly 2 times, or one before a frame and one after a frame?
    • If spacebar was pressed between two frames, you will get up and down events for each press.
    • Examples would be appreciated.
  • Engines are moving towards an "input action" model, where you have something like "Jump" and then have buttons mapped to that.
    • If a user wants to jump, is A/X/Space pressed? It's an inverse mapping to what we have today.
    • Possible extension point/on top of this API.
  • Enums are potentially opinionated for naming.
    • Perhaps we could have something like the Key struct for joysticks as well?
    • GLFW doesn't support anything other that the buttons we've exposed today.
    • Not sure that having an integer helps if we haven't named it at all.
    • Introducing multiple names for a single enum should be fine.
    • Most trivial example is Nintendo controllers vs the Xbox controllers, A/B and X/Y are swapped (could be confusing!). Either going to assume that A = Nintendo's A (middle right) or A = Xbox A (bottom button)
    • Rename to be positional rather than opinionated.
    • More nuanced buttons can be via the joystick API, and gamepad kept a standard controller. We can't accomodate for everything, so we need to make concessions and be opinionated for gamepad, and leave the more exotic stuff for joystick.
    • We could just have ISteamControllerGamepad on top of IGamepad for controllers with touch pads, for example.
    • Take keyboards: you have integer-identified buttons but no keyboard layout is the same. An API is built on top of that abstract concept and query "what does that button represent" - on the high-level side we could have a nicer abstraction.
    • Comes back to that concept of button codes.
    • Nice and easy API for the common case, need for esoteric features like touchpads then you can actually go and add that support, you just need to do some more additional work.
    • Difficult to say where this starts and ends: we might want to expose all of the buttons, but we also want to expose low-level features then we might want to expose all of the motors, then all the lights, etc etc: where does it end?!
    • Difficult to tell what we can and can't support, and comes back to what the backends can actually do.
    • Basically every OS exposes a "here's the raw PCI device ID, and here's a byte array of the data and what HID input it came from" - low-level API is very primitive
    • Could always add another input backend.
    • "Easier to get a smaller thing done now than have a larger thing in the works forever" - @redhacker
    • What we have today is a lot of what most people will want to use. A low-level thing could be built as a parallel.
    • Think about low-level input in the future in addition to this
    • Could we expose at least the vendor ID/device ID to the developer (in raw form or common vendor enums) to mitigate this for now.
    • Notably, the same vendor/device information by Vulkan/DirectX and any audio device - it's a very common concept.
    • Think about exposing vendor and device IDs
      • Potentially modifying GLFW to get us that

@Perksey
Copy link
Member Author

Perksey commented Feb 25, 2022

SilkTouch

  • Approved (call conv modifier discussion notwithstanding), but we must come back to the overloader - it's a bit too early to decide on something solid as there's no perfect rule that we know of to generate them at this time - we can only get this through experimentation!
  • Why use an enum and custom attribute rather than reusing UnmanagedCallConv and the typeof(CallConv*) types that C#/.NET have standardized on for moving forward?
    • We don't really have control over those types.
    • For [contrived] example, what if we want a JavaScript calling convention?
    • We can't just hack up a "calling convention" the runtime doesn't support, MemberFunction for example was just something that happened to work on the Windows ABI
    • We could use CallModifiers to, for example, influence codegen to call into IJsInProcessRuntime and call JavaScript code - this isn't necessarily just an ABI-based concept. It could support other scenarios.
    • It makes more sense to separate these, as these are sort of associated with DllImport and that side of the calling process, and a "JavaScript" convention concept (as discussed before).
    • It's difficult to represent right now, because we have the native API attribute
    • We could/should change this to use the CallConv types instead
    • Direct advantages
      • As the runtime versions in the future, it will continue to add CallConv types. These types are the official way moving forward to represent any calling convention information for the rutnime going forward.
      • SilkTouch has to go out of this way to map this and understand this anyway, unless we just have the types then we can specify them as-is - SilkTouch doesn't even need to understand these.
      • "[DllImport] is effectively magic" - @tannergooding
      • Just change Modifiers to a CallConv type array
  • The overload problem does need to be solved in some way. Some functions have absurd amounts of overloads (particularly in assimp)
    • We want to scrap a bunch of overloads as well. A lot of this is only generating a bunch of "important" overloads.
    • Was there consideration for a source generator approach to opt-in to the friendliest variant that they want?
      • Yes, kind of. We don't have a formal proposal as we only just thought of this today.
      • We need to bake the most basic overloads into the assembly itself.
      • We'd like to have a source generator.
      • We want SilkTouch to be productized, and find a way to remap types per their liking and use overloads etc.
      • We should experiment with this and report back in a future community meeting.

@Perksey
Copy link
Member Author

Perksey commented Jul 17, 2022

All of the working group review comments have been rectified. Can this be merged now? Trying to get all non-meta PRs in my name closed off.

@HurricanKai HurricanKai merged commit 63304c4 into main Jul 17, 2022
@HurricanKai HurricanKai deleted the proposal/all-3.0-proposals branch July 17, 2022 13:33
Beyley pushed a commit that referenced this pull request Aug 24, 2022
This was referenced Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants