Fixes #3460. xxxtoxxx consistency

[tig]

Fixes

• Fixes Mixing signatures for ScreenToX between int and Point #3460
• Partially Fixes Refactor MouseEvent and MouseEventEventArgs to simplify #3029 (X/Y -> Position)

Proposed Changes/Todos

• ScreenToFrame -> takes Point
• ScreenToViewport -> takes Point

[dodexahedron]

Hey quick easy optiization:

Make them take it as in, too, so reference can be passed by default without needing to be explicitly done at the callsite.

[dodexahedron]

That, by the way, is just a good habit to get into when passing structs - especially blittable ones.

[dodexahedron]

For certain other cases, there's also now ref readonly for method parameters, in c# 12, but that is slightly different. MS has a matrix somewhere showing the effects and compatibility at the callsite for ref, ref readonly, in, and out, for the curious. I think it's linked from the what's new for c# 12.

[tig]

in -> done.

[dodexahedron]

Hm. Why building new points in a lot of those lines rather than just passing the point itself?

At the first place in a method that assigns it to something, it'll still get value copied anyway.

And with a non-referenceable ephemeral new() in a method call, it will be copied twice, making the in not apply.

[dodexahedron]

Mostly same minor comments elsewhere, so I won't repeat it all over the place.

Since consistency was the theme for this one anyway, using Offset when offsetting points is probably a good idea, and can also result in less copy activity and more inlining.

The ephemeral constructor stuff, though, is potentially tangible with how hot those code paths are.

Otherwise, YAY!

[dodexahedron]

There's also an Offset overload that takes Point, too, which is better when you already have one.

Low-level performance stuff relevant here and other hot paths

In the case of the Point struct or other blittable structs like it, especially, it's better to pass one of it, whether by value or reference, than to pass components in multiple parameters. It's an 8-byte wide struct, which can fit in one register if sent by itself, whereas 2 ints is two copies in separate registers plus a non-obvious amount of other work at the CIL level and internally in native code.

Plus, consistency of the internal layouts of our types also means less of the deconstruction and reconstruction work at boundaries.

Man, github, pick a lane: Markdown or HTML. 😆

[tig]

Hm. Why building new points in a lot of those lines rather than just passing the point itself?

You mean in the tests?

At the first place in a method that assigns it to something, it'll still get value copied anyway.

And with a non-referenceable ephemeral new() in a method call, it will be copied twice, making the in not apply.

I was focused on getting the API right; perf next.

How do you suggest I address this?

[tig]

I bit the bullet and changed MouseEvent to use Point.

[dodexahedron]

Sounds like it's probably pretty great.

I'm only half awake after a wrong number caller woke me up, so I'm useless for review right now but I'll get on that as soon as I'm up and running tomorrow. :)