Change add_native_font to accept Into<NativeFontHandle>

[matprec]

This change on add_native_font would allow more idiomatic conversion. This change is backwards compatible, so existing APIs won't need to adapt.
rust playground: poc

For context, i maintain rust-font-loader which uses wingdi instead of DirectWrite, because the winapi-rs bindings currently don't support directwrite in its full extend (e.g. detection of monospaced fonts). But DirectWrite has a interop interface, so conversion from and to LogFontW is possible. I implemented the necesseary parts for DWrote, if they land, one would be able to add a LogFontW as a NativeFontHandle.

[kvark]

@MSleepyPanda at what stage in WR would you expect the actual NativeFontHandle to be produced from this object?

[matprec]

In add_native_font, even before crossing ipc/thread boundaries:
Current:

let msg = ApiMsg::AddNativeFont(key, native_font_handle);

let msg = ApiMsg::AddNativeFont(key, native_font_handle.into());

This way, we would have minimal code change and backend stays the same (therefore things shouldn't break).

Although this is a backwards compatible and ~3 lines change, i prefer to ask first :)

[kvark]

@MSleepyPanda I don't mind those 3 lines, but it seems non-obvious to me as to why the change would help any test case. If your backend API is generic over Into<NativeFontHandle>, why can't you convert with native_font_handle.into() before calling WR?

[matprec]

I agree that one could do this as well locally - but this would increase the plug and play effect, since this API relies on external crates (not defined in webrender-/traits) and therefore would benefit from this kind of interop. This is an idiomatic pattern, at least used often in crates, to accept Into<T> in APIs, to offer out of the box interop, e.g. with different kind of collections. E.g. Rusttype accepts Into<SharedBytes<'a>>, which enables one to pass every kind of collection which implements Into<SharedBytes<'a>>.

This is more or less personal preference. I would like to see it beeing implemented in webrender because it would otherwise cause, even though small, "boilerplate" for implementors.

[kvark]

This is an idiomatic pattern, at least used often in crates, to accept Into in APIs, to offer out of the box interop, e.g. with different kind of collections.

Is it used in std?

E.g. Rusttype accepts Into<SharedBytes<'a>>, which enables one to pass every kind of collection which implements Into<SharedBytes<'a>>.

Yes, but the difference here is that SharedBytes is a special auxiliary type defined within rusttype that no one is going to use directly (or care about) anyway. And the conversions are defined within rusttype too. So Into allows to hide this type from the user and simplify the API. This is a perfectly valid pattern, but it's not exactly matching our case with NativeFontHandle, which is a type alias for FontDescriptor that you already know about and use.

All in all, I'd be perfectly fine with the PR if this is going to save more than 3 lines of code on your side ;)

[matprec]

@kvark It's kind of rare in std itself, but it is used, e.g. CString.

Thanks for elaborating, didn't look too closely into (no pun intended) how SharedBytes is defined.

Also thanks for having a look at it. PR is coming soon™️ when i finish working through the mails.

[glennw]

@MSleepyPanda Are you still planning to work on this?

[nical]

Let's close for now since there isn't any activity. Please feel free to reopen if there's any plan to move forward with this.