koreader / crengine

This is the KOReader CREngine fork. It cross-pollinates with the official CoolReader repository at https://github.com/buggins/coolreader, in case you were looking for that one.
70 stars 45 forks source link

Some LVFormatter::measureText() changes #574

Open bbshelper opened 2 months ago

bbshelper commented 2 months ago

I'm finalizing a major optimization to LVFormatter::measureText(), so I'd like these smaller ones to be reviewed and merged first.

The first 2 commits have around 0.5% and 0.2% performance gain in my Load and Render test.


This change is Reviewable

bbshelper commented 2 months ago

I'm not sure these microoptimizations are worth your time and the 60 minutes I had to spend on reviewing to get back into the code

I'm not sure either :) I must have spent more time optimizing crengine than what I can ever save in reading on e-ink in my lifetime.

and made sure you did not change the behaviour :/

I try very hard not to. I've setup a regression test, which is quite useful in catching corner cases. That said, I don't really know a thing about bidi and harfbuzz, and they are not covered by my regression test.

I'm really fearing that :\ (If you PR that and don't hear from me, it's because I'll soon be on vacation, not because of fear :))

I promise that the new code is clearer than the current one. It's a major optimization and not a rewrite. :)