Skip to content
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

web_sys: missing set_transform with matrix function #3578

Closed
stephanemagnenat opened this issue Aug 28, 2023 · 5 comments · Fixed by #3580
Closed

web_sys: missing set_transform with matrix function #3578

stephanemagnenat opened this issue Aug 28, 2023 · 5 comments · Fixed by #3580

Comments

@stephanemagnenat
Copy link

Motivation

The function setTransform on Canvas 2D can accept a matrix on modern browsers. This is currently not exposed in web_sys. However, the sister function getTransform is exposed.

Proposed Solution

Expose a set_transform_with_matrix function, to match the get_transform ones.

@daxpedda
Copy link
Collaborator

@stephanemagnenat
Copy link
Author

stephanemagnenat commented Aug 29, 2023

No, if you look at the documentation page, there is a second version that takes a single DomMatrix parameter, and that was introduced at the same time getTransform was.

So I find it illogical that web_sys provides get_transform but not that second version of set_transform.

@daxpedda
Copy link
Collaborator

Thanks!
Made a fix here: #3580.

@stephanemagnenat
Copy link
Author

Thank you for the very fast answer!

A small question, get_transform returns a DomMatrix while the new set_transform takes a DomMatrix2dInit, are these compatible? A typical use as described here is to combined these two functions to save and restore the canvas transformation matrix.

@daxpedda
Copy link
Collaborator

daxpedda commented Aug 29, 2023

It should work, because DOMMatrixInit inherits DOMMatrix2DInit, unfortunately our WebIDL generator doesn't support inheritance on dictionaries. I think you can just use unchecked_into().

Btw, if you want to create a PR implementing this conversion I'm happy to review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants