matteodelabre / waved

Experimental driver for the reMarkable 2 E-Ink display
GNU General Public License v3.0
58 stars 6 forks source link

Implement concurrent updates #12

Open matteodelabre opened 2 years ago

raisjn commented 2 years ago

Proposal 1:

Problem: what if there is no regular frames sent to vsync thread but we have immediate updates in the queue?

Proposal 2:

Problem: we introduce more mutex locking by having to enqueue each frame individually (instead of batch generation) Benefit: The subequent frame generation is likely less work because we only need to look at the region that is dirty (the full frame is already generated) and update those areas. We may not even need the "is_consecutive" check to be performed until this update is the "top" update in the queue (and needs to fill out a full frame)

matteodelabre commented 2 years ago

Thank you for this thorough proposal. Both options seem equally sensible to me. Here are some more thoughts.

For proposal 1, having no regular frames to process while there are pending immediate updates would not be an issue (in my opinion), since we can add a condition variable for the immediate queue to wake up the vsync thread when needed.

The main issue I can see in proposal 1 is that we loose the ordering between regular updates and immediate updates, since they go to different queues and are processed in different threads. Could this cause visual glitches under some circumstances? If I understand correctly, proposal 2 does not have this issue.

Another thing is that proposal 1 would blur the existing separation of concerns between the two threads, which may make it harder to reason about the code. Proposal 2 would retain this separation though, as you mentioned, this comes with the cost of more locking.

matteodelabre commented 2 years ago

Currently working on implementing proposal 2 on branch immediate. First results on the “spiral” test (left is with old code, right is with immediate mode):

https://user-images.githubusercontent.com/1370040/174214027-1d8c1111-98f6-4a8a-9484-3bc2d5ca063d.mp4

Still needs more testing and cleanup before it’s ready.