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

Prevent stale frames from being drawn / always ensure up-to-date contents in webview #3668

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Lolle2000la
Copy link

@Lolle2000la Lolle2000la commented Dec 25, 2024

Following up on #3656

Short description

At key points where external changes enter the webview, stale images might get rendered. This ensures that a frame showing current state is always shown.

What it solves

Sometimes previous or corrupted frames get rendered, only showing the current state of the page (i.e. the back of the card or the next card) when an action that causes a repaint is done by the user (for example: selecting text).

How it solves it

At key points where the webview is being interacted with externally, a full update (which includes a repaint) is done.

Potential implications for performance

While this ensures that no more stale frames get shown, it also causes full repaints to happen quite often and in addition to the render operations QWebEngineView might do on its own. I think this is negligible in my testing, but needs to be pointed out.

Request for comments

In my testing this solved the issue, and this time it seems to do so reliably, however, please leave feedback or opinions on this. It is also a bit of a wooden-hammer-approach, but it does fix the issue.

At key points where external changes enter the webview, stale images might get rendered. This ensures that a frame showing current state is always shown.
@abdnh
Copy link
Collaborator

abdnh commented Jan 7, 2025

Didn't notice a difference or issues testing this, and it looks harmless to include. Will leave it to @dae though.

@dae
Copy link
Member

dae commented Jan 9, 2025

I presume the answer is yes, but could you confirm the issue you're trying to address also exists when the software driver is enabled?

My understanding of this rendering issue is that for most people, they either won't see the problem, or only see it with particular video drivers. The changes in this PR would thus make things better for a relatively small portion of the userbase, so it would be nice not to impose a performance penalty on unaffected users.

You've placed .update() calls in four separate locations. In your testing, were all locations required, or did you add some "just in case"?

If you experiment with scheduling an .update() for a few seconds after JavaScript is evaluated, instead of calling .update() immediately, does the corrupt display refresh correctly after those seconds elapse?

@Lolle2000la
Copy link
Author

Lolle2000la commented Jan 14, 2025

Apologies for the late reply, I am currently preparing for my exams so I am currently slower to respond.

I presume the answer is yes, but could you confirm the issue you're trying to address also exists when the software driver is enabled?

Yes, I have tried basically all driver configurations and run them for prolonged amount of times and they occured all across them, including software. Vulkan is the one where it occurs the least.

My understanding of this rendering issue is that for most people, they either won't see the problem, or only see it with particular video drivers. The changes in this PR would thus make things better for a relatively small portion of the userbase, so it would be nice not to impose a performance penalty on unaffected users.

It shouldn't have a large penalty since it essentially just causes a second frame to be rendered when the webview is externally interacted with from python code. In essence this makes sure that a frame with the current state is being put to the screen.
This is, as far as I can see, only the case when the cards back gets shown or another is displayed, as well as when the card is faded out on inactivity.

You've placed .update() calls in four separate locations. In your testing, were all locations required, or did you add some "just in case"?

self._evalWithCallback could alone fix this, because all the other APIs call onto this. My original rational was that every method affecting the webview could cause the same problems, but tracing the calls a bit more I notice that load_url and _setHtml cause _on_load_finished to be called, which in turn calls eval, which calls evalWithCallback, queues the action and then, finally, calls _evalWithCallback. I will test this. Though, due to how update works, calling it multiple times will not schedule another render if one is already queued.

If you experiment with scheduling an .update() for a few seconds after JavaScript is evaluated, instead of calling .update() immediately, does the corrupt display refresh correctly after those seconds elapse?

I will look into that and verify and share my results here. As I said in the beginning I am currently preparing for my exams so it could take a bit, but maybe someone else can also reproduce the bug.

I will also share the deck I am currently using (漢字.apkg.gz). The bug happens regularly on my Surface Pro 9 every about ~10-100 times in the 漢字::意味、音訓::意味-subdeck (less often on the other subdecks). I have sessions where this doesn't happen and sessions where it does constantly for a long time now. On my PC (Ryzen 9 3900x) this happens less but notably enough.

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.

3 participants