Skip to content

Refactor trig functions into LUT#476

Merged
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
jonvmey:trig-lut
Jul 11, 2021
Merged

Refactor trig functions into LUT#476
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
jonvmey:trig-lut

Conversation

@jonvmey
Copy link
Copy Markdown
Contributor

@jonvmey jonvmey commented Jul 10, 2021

Replaced the use of the standard library trig functions with a LUT-based
implementation instead. The standard library implementations produce
more accurate results but the usage here doesn't need that. This ends up
saving nearly 7kB of binary size.

@jonvmey
Copy link
Copy Markdown
Contributor Author

jonvmey commented Jul 10, 2021

Final tally for binary size change with this diff is exactly 7000 bytes less. I still need to load this on my watch tomorrow to verify that everything's good but I think it's all correct.

@Riksu9000
Copy link
Copy Markdown
Contributor

There already seems to exist this in LVGL. Can you use this instead?
https://github.com/joaquimorg/lvgl/blob/23430cf20e32294549fff9b2879a9466dacc19bb/src/lv_misc/lv_math.c

@Riksu9000
Copy link
Copy Markdown
Contributor

There seem to be some issues with this.

VID_20210710_120654.mp4

@jonvmey
Copy link
Copy Markdown
Contributor Author

jonvmey commented Jul 10, 2021

Yeah, that definitely looks wrong, I'll take another look to see what I messed up.

The implementation in LVGL doesn't appear to be part of its exposed API so I wasn't sure if we wanted to use that directly.

@Avamander
Copy link
Copy Markdown
Collaborator

The implementation in LVGL doesn't appear to be part of its exposed API so I wasn't sure if we wanted to use that directly.

If they remove it I guess we can think of a solution. For now I suspect it'd be fine. Quite a bit less to maintain.

Replaced the use of the standard library trig functions with a LUT-based
implementation instead. The standard library implementations produce
more accurate results but the usage here doesn't need that. This ends up
saving nearly 7kB of binary size.
@jonvmey
Copy link
Copy Markdown
Contributor Author

jonvmey commented Jul 11, 2021

Updated the changes to use LVGL's _lv_trigo_sin function instead. The issue @Riksu9000 mentioned should also be fixed now.

@Riksu9000
Copy link
Copy Markdown
Contributor

Seems to work fine now.

text goes down 7160 and data goes up 4 compared to develop.

@JF002
Copy link
Copy Markdown
Collaborator

JF002 commented Jul 11, 2021

Looks good to me, thanks for this PR, this nice review and these 7160 free bytes in memory ;)

@JF002 JF002 merged commit 4f378e8 into InfiniTimeOrg:develop Jul 11, 2021
@JF002 JF002 added this to the Version 1.3 milestone Jul 11, 2021
@jonvmey jonvmey mentioned this pull request Dec 16, 2021
5 tasks
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.

5 participants