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

gfx: Properly set graphics thread delay #6

Merged
merged 2 commits into from
Aug 31, 2023
Merged

Conversation

nnhien
Copy link
Contributor

@nnhien nnhien commented Aug 15, 2023

This PR properly sets the delay for the graphics drawing thread (task).

The original implementation is incorrect because when calculating the number of ticks to sleep the graphics task for, the task begins sleeping after some ticks which should be included in the calculated number have already passed.

For example, let t be the time in ms at which the frame starts to get rendered and d be a delay (in our case, the frame duration). We expect that the next frame begin rendering at t'=t+d. However, consider processing time p. The current implementation begins rendering the next frame at t'=t+p+d, since using vTaskDelay delays the current task relative to when that function is called (at time t+p) (see FreeRTOS API documentation). This time misalignment compounds for each frame, since the computation delay occurs for each frame. So if the animation has 1000 frames and extraneous computation time takes 2 ticks (for the TidByt's hardware configuration, 1 tick = 1ms), then the animation is played for 2 seconds longer than it should, an order of magnitude (!!!) in CPU time! If the computation time takes even longer (say, 4 or 5 ticks), this results in many seconds worth of time misalignment.

This fix uses xTaskDelayUntil, which is the recommended API call to schedule task delays that end on an absolute time. This allows us to exploit the RTOS' scheduler so that the next frame begins drawing at t'=t+d.

@tidbyt-bot
Copy link

tidbyt-bot commented Aug 15, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@nnhien
Copy link
Contributor Author

nnhien commented Aug 15, 2023

I have read the CLA Document and I hereby sign the CLA

@nnhien
Copy link
Contributor Author

nnhien commented Aug 15, 2023

recheck

@matslina
Copy link
Contributor

Thanks for the PR, and apologies for the slow response. This looks great!

@matslina matslina merged commit 2486250 into tidbyt:main Aug 31, 2023
2 checks passed
@tidbyt tidbyt locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants