servo / dwrote-rs

DirectWrite bindings and wrapper for Rust
11 stars 13 forks source link

Add FontFallback and TextAnalysisSource objects #32

Closed raphlinus closed 5 years ago

raphlinus commented 5 years ago

Adds wrapping for FontFallback and TextAnalysisSource, including custom implementations of the latter in Rust.

raphlinus commented 5 years ago

I haven't tested this yet, but wanted to get feedback before I spend too much more time on it.

I've found a number of issues in dwrote. For one, the use of unsafe seems quite wrong to me. For example Font::take doesn't validate its pointer arg, and among other things it's quite easy for "safe" code to pass a null pointer. Conversely, as_ptr is listed as unsafe although it just returns a pointer.

There are also some opportunities to reduce allocations by using smallvecs and the like.

I've tried to make this wrapper pretty thin, but have made some choices. For one, I'm just using a Vec for the text. Trying to wire up access to non-contiguous buffers would be painful and I'm not convinced would be a performance win. For two, I'm only allowing a single number substitution (this follows Chromium).

I'll update the PR when I have it working end-to-end, but will happily accept feedback meanwhile.

raphlinus commented 5 years ago

I have some example code that seems to work. I can add it under the examples directory, which seems not to exist.

There are possibly some other issues. @connicpu (who is adding similar functionality to directwrite-rs) suggests that the text analysis methods return &[u16] in general. This would allow utf-16 ropes for the text, but, perhaps more to the point, would have less concerns about safety because none of the buffers would be mutable. I'm certainly open to that change.

I'll continue prototyping with this but am open to more suggestions.

pcwalton commented 5 years ago

@bors-servo: r+

bors-servo commented 5 years ago

:pushpin: Commit 455d289 has been approved by pcwalton

bors-servo commented 5 years ago

:hourglass: Testing commit 455d2894360400a8a6af4fe10351974ecd6a3516 with merge 6befd95d75e47ea6079ce1d07dd0e05c203352b8...

bors-servo commented 5 years ago

:sunny: Test successful - status-appveyor Approved by: pcwalton Pushing 6befd95d75e47ea6079ce1d07dd0e05c203352b8 to master...