search

jay3332 / ril

Rust Imaging Library: A high-level imaging crate for Rust.
https://crates.io/crates/ril
MIT License
80 stars 10 forks source link

Remove unnecessary width computation in TextLayout::height() #13

Closed Sofiman closed 1 year ago

Sofiman commented 1 year ago

Hey,

This is a small PR containing improvements to the performance of TextLayout::height().

The function calls the inner function dimensions which calculates both width and height but only height is needed. The cost of calling dimension to get the height is higher and depends on the length of the text. By calling the height() function on the inner Layout (from the fontdue crate), we reduce the overhead of height.

Furthermore, from a design point of view, it could be interesting to move the width calculation to the TextLayout::width() instead of TextLayout::dimensions() and then construct the pair in dimensions using the width and height functions. If validated, I could add this change to this PR.

(Note: this speeds up the process of find out the font size that can fit a text in a width and height constrained box).

jay3332 commented 1 year ago

Thanks for the PR, this looks good as is so I can merge it at any time.

Furthermore, from a design point of view, it could be interesting to move the width calculation to the TextLayout::width() instead of TextLayout::dimensions() and then construct the pair in dimensions using the width and height functions. If validated, I could add this change to this PR.

This would be a good idea, if you don't mind, you can add it in the PR just to keep things clean.

Sofiman commented 1 year ago

I just made the changes to TextLayout::width and TextLayout::dimensions, and updated a little bit of documentation for a little more clarity. Thank you for this awesome crate, I appreciate the effort put in it!