search

linebender / glazier

Deprecated Rust Window Creation Library
Apache License 2.0
207 stars 32 forks source link

Pass `PointerEvent`/`MouseEvent` by value, not ref. #129

Closed waywardmonkeys closed 1 year ago

waywardmonkeys commented 1 year ago

This makes it match KeyboardEvent.

waywardmonkeys commented 1 year ago

I'll fix the macOS build error in 12-ish hours, but if there's a lack of interest in having this change at all, please feel free to say so before then. :)

DJMcNab commented 1 year ago

This does seem more consistent. It appears that PointerEvent never allocates, so there's no concern about that forcing non-reuse.

I'd be inclined to only do this for PointerEvent, such that the changes to the backends are minimal, as the mouse events are legacy anyway.

As a small note, I'd generally call this "by value" rather than "by move" - by move is more C++ parlance, as I understand it.

waywardmonkeys commented 1 year ago

I've got a draft PR already that moves Windows from creating MouseEvent to creating PointerEvent and locally, I have changes that do the same for macOS (but they aren't complete yet). Web doesn't build currently, so I'm hoping to have MouseEvent removed by sometime this month as other PRs land.

You're right about the terminology. I forget occasionally as I switch between languages.

I've updated the commit message to say "by value" instead of "by move".

waywardmonkeys commented 1 year ago

I've got the changes queued up now for landing on main that remove the old MouseEvent stuff entirely. So I'll be rebasing this shortly.

waywardmonkeys commented 1 year ago

Done and ready for review, @jneem or anyone else.