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

Implement frame callbacks and a new memory pool #221

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Conversation

LGFae
Copy link
Owner

@LGFae LGFae commented Feb 22, 2024

Instead of using multi pools, we are double buffering our WlBuffer so that we don't have to busy loop forever.

Fixes #208 and #220.

@LGFae
Copy link
Owner Author

LGFae commented Feb 22, 2024

The issue our CI is accusing is solved in PR #209, so it can be ignored for now.

@LGFae LGFae linked an issue Feb 22, 2024 that may be closed by this pull request
@jeLee6gi
Copy link

This fixes #208 for me

@LGFae
Copy link
Owner Author

LGFae commented Feb 23, 2024

Thanks, but unfortunately right now this is not really obeying the protocol properly. I am fixing that right now.

@LGFae LGFae force-pushed the go-back-to-slot-pool branch 2 times, most recently from 8e46f3a to 96a6279 Compare February 23, 2024 18:32
@LGFae LGFae marked this pull request as draft February 23, 2024 20:51
@LGFae LGFae linked an issue Feb 23, 2024 that may be closed by this pull request
@LGFae LGFae marked this pull request as ready for review February 24, 2024 12:42
@LGFae LGFae changed the title Undo most of commit #0105341 and go back to slot pools Implement frame callbacks and a new memory pool Feb 24, 2024
@LGFae
Copy link
Owner Author

LGFae commented Feb 24, 2024

I believe this is now ready. I've tested in every configuration that was previously exploding.

I will most likely be merging this on Monday, to give some time for anyone who would like to test this. I can also use a slightly different setup on Monday, just to confirm that everything works in yet another machine.

The branch's name is now woefully inadequate, since we didn't really go back to slot-pools, but oh well.

@jeLee6gi
Copy link

I tested this in niri with two outputs of different size (and different refresh rates fwiw) and everything seemed to worked just fine, cheers!

@0bCdian
Copy link

0bCdian commented Feb 26, 2024

I tested this pr and everything seems to be working, no more glitches!

if wallpaper.has_animation_id(self) {
self.transition_done.store(true, Ordering::Release);
}
}
}

struct FrameCallbackHandler {
cvar: Condvar,
time: Mutex<Option<u32>>,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth adding a comment that this time, that is received in the frame callback, doesn't really mean anything and in particular should not be used for any kind of frame timing. (For that purpose, you can use presentation-time + some logic to predict when a frame will be presented.)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point.

@YaLTeR
Copy link

YaLTeR commented Feb 26, 2024

Gave this a quick test on my setup, seems to work!

We were only using a single WlBuffer per output, busy-waiting until it
was released by the compositor. The problem is that compositors are now
beginning to never release the buffers, keeping them around for some
functionality. This means we have to double-buffer our WlBuffers. Note
that, in order to be future proof, we would like to have something that
automatically triples, or quadruple buffers, as needed.

Now, if we are using two WlBuffers, in order for our animations and
transitions to work, we will need to copy the previous buffer's content
onto the new one at every surface commit.

From my understanding (which could be completely wrong), sctk's current
pools do not let us that. This is because we can't access the content of
any current active buffer without some shenanigans. Therefore, I've
implemented a new memory pool that simply gives us the first free
(non-active) buffer it can find, and allocates more memory automatically
when it can't find any. This means we will automatically double or triple
buffer (or any other number of buffers), as needed.

Now, this leads to a problem: if we don't wait for the compositor, we
will often try to draw before it is ready. If that happens, the buffers
will still be active, and we will have to allocate more. In order to
prevent that, I've implemented proper frame callbacks. This has also had
the benefit of making animations much more smooth, so I suppose this
should have been considered a bug.

There was a problem with the MSRV regarding some structs visibility,
which is why everything has been annotated with pub(super) or
pub(crate).
@LGFae LGFae merged commit d174d13 into main Feb 26, 2024
9 checks passed
@LGFae LGFae deleted the go-back-to-slot-pool branch February 26, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants