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

[BW Clock Lite] New version using bitmap font making it quicker #2440

Merged
merged 7 commits into from Mar 7, 2023

Conversation

thyttan
Copy link
Collaborator

@thyttan thyttan commented Jan 2, 2023

@peerdavid BW Clock loads quicker if using bitmap fonts, unsuprisingly :p I'm thinking I could maybe add app customization so the user can choose between "Fancier Looks" or "Faster Loading". What do you think?

The changes as they are now is what I have tested performance with. It is not intended for merging as is.

@peerdavid
Copy link
Contributor

Oh wow thats great. Is there a huge difference how it looks when "fast loading"? If not I would guess fast loading should be the default right?

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 3, 2023

I've tested using bitmap fonts as mentioned, I'll try with vector fonts as well which will probably resemble the Manrope font better.

Either way I'm thinking the new way of fast loading, introduced with #2421, will be implemented both if the user chooses the Manrope font or a more 'performant' font. In case I was a little unclear.

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 3, 2023

Here's a graphics dump with bitmap font:
download

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 3, 2023

Here's with vector font:
download (1)

@peerdavid
Copy link
Contributor

peerdavid commented Jan 4, 2023

Yea it looks quite different, I honestly liked the original font -- in this case I prefer the "slow" solution which we should still provide -- so what I mean to provide an option or so? what do you think?

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 4, 2023

Yes, I think that would work well using app customization via the app loader as mentioned in the opening comment :)

I don't know how that's done yet, but I can try to learn if you don't particularly feel you want to implement it? ;)

@gfwilliams
Copy link
Member

BW Clock loads quicker if using bitmap fonts, unsuprisingly :p

Do you have any numbers?

Just thinking aloud here - if all the clock_info code apart from rendering ends up as part of the clock_info library, it feels like there's not actually that much code in the clock itself - and you'd likely have to change all the positioning for different fonts anyway.

It might be worth moving all the complicated stuff out into utility libraries so it's just 90% rendering code and then just forking the app?

Another option is to look at loading fonts from Storage. It could be done quite easily, and while rendering might be a smidge slower, startup time would be a lot better.

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 5, 2023

Do you have any numbers?

Not more than counting in my head, shy of maybe 0.7 seconds vs shy of 0.5 seconds. I could record a video for comparison. I don't know how to do timings programmatically.

It might be worth moving all the complicated stuff out into utility libraries so it's just 90% rendering code and then just forking the app?

I don't understand precisely what this means, so I don't have an opinion on it, maybe @peerdavid has one?

Another option is to look at loading fonts from Storage. It could be done quite easily, and while rendering might be a smidge slower, startup time would be a lot better.

That sounds like it could be sensible to me.

I will probably stop working on this for a while. So don't feel too rushed to help with it.

@gfwilliams
Copy link
Member

Not more than counting in my head,

I'd be very vary of making quite big sweeping changes without any numbers to back them up. Just add t=getTime() at the start of the app, and print(getTime()-t) at the end - and you'll get the time in seconds it took to load the app.

I don't understand precisely what this means,

I just mean that if there's very little code in the clock app itself (so it's just ~100 lines of code that is rendering the clock face) then potentially it just makes sense to create a copy that doesn't use fonts, since almost everything in the clock would change anyway in order to tweak the font positions.

@thyttan
Copy link
Collaborator Author

thyttan commented Jan 8, 2023

Just add t=getTime() at the start of the app, and print(getTime()-t) at the end

Timings based on PR #2471:
Regular load(): 1.58590698242 s
Fast loading: 0.91567993164 s

Timings when using bitmap font in this PR:
Regular load(): 1.22711181640 s
Fast loading: 0.55581665039 s

Delta ≃ -0.36 s

This was tested without pretokenization or minification.

I just mean that if there's very little code in the clock app itself (so it's just ~100 lines of code that is rendering the clock face) then potentially it just makes sense to create a copy that doesn't use fonts, since almost everything in the clock would change anyway in order to tweak the font positions.

Ok! I'll maybe do that then. But your idea of loading fonts from storage seems more alluring to me, although I don't know how to do that right now and so don't know how much work it would require to achieve.

thyttan added 3 commits February 25, 2023 18:43
… to a standard bitmap one. Don't invert theme as this doesn't work very well with fastloading. Do an initial fillRect on the clock info area since it would wait for clock info before drawing out the previous app. Change all occurences of var to let.
@thyttan thyttan changed the title [BW Clock] No font included [BW Clock Lite] No font included Feb 25, 2023
@thyttan thyttan changed the title [BW Clock Lite] No font included [BW Clock Lite] New version using bitmap font making it quicker Feb 25, 2023
@gfwilliams
Copy link
Member

Do you think this is ready to merge?

It's still marked as draft but as it's a separate app I'd think it's fine to pull in?

@thyttan
Copy link
Collaborator Author

thyttan commented Mar 7, 2023

I've been meaning to tweak font sizes and positions, but will probably not get around to it soon.

I've used it for a while myself.

So sure, go ahead 👍

@gfwilliams gfwilliams marked this pull request as ready for review March 7, 2023 12:42
@gfwilliams
Copy link
Member

Ok, thanks!

@gfwilliams gfwilliams merged commit 4f8717c into espruino:master Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants