When resizing about:performance quickly, every once in a while a frame is painted without localizable strings

RESOLVED FIXED in Firefox 66

Status

()

defect
RESOLVED FIXED
8 months ago
5 months ago

People

(Reporter: florian, Assigned: florian, NeedInfo)

Tracking

unspecified
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(5 attachments)

I noticed that about:performance sometimes flickers when resizing the window. Using a screen recording, I saw that the flickers is caused by frames being painted without the localizable strings. See attached screenshot.
If I had to make a guess about what's going on, I would say that Fluent is replacing strings whenever the refresh driver ticks, and that sometimes we paint outside of refresh driver ticks due to receiving resize events from the OS.
Flags: needinfo?(gandalf)
We translate mutations in rAF - https://searchfox.org/mozilla-central/source/intl/l10n/DOMLocalization.jsm#650-664

Is that what you're talking about? Is there a better way to cluster translations to happen once per animation frame?
Flags: needinfo?(gandalf) → needinfo?(florian)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> We translate mutations in rAF -
> https://searchfox.org/mozilla-central/source/intl/l10n/DOMLocalization.
> jsm#650-664
> 
> Is that what you're talking about? Is there a better way to cluster
> translations to happen once per animation frame?

I suspect we don't have an animation frame when the paint is triggered by a resize notification received from the OS rather than by the refresh driver.

For this specific bug, it looks like I could just call document.l10n.translateFragment near https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/toolkit/components/aboutperformance/content/aboutPerformance.js#1020 but if possible I think it would be better to address the problem more generally, as you may see other instances of the same problem when other pieces of the UI are resized.
Flags: needinfo?(florian)
I agree we should try to tackle it in general.

So, what I'm confused is why is there a retranslation happening when you resize the window? Or is it just a coincidence that resize happens as translation happens, and in result the paint from resize happens before the paint from l10n?
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5)

> So, what I'm confused is why is there a retranslation happening when you
> resize the window? Or is it just a coincidence that resize happens as
> translation happens, and in result the paint from resize happens before the
> paint from l10n?

The latter. The about:performance table is refreshed (and so the DOM is rebuilt and retranslated) every 2s. When we refresh while the user is resizing, I think it's a matter of luck whether the next paint will be from the refresh driver (in that case strings will be correct) or from a resize event (in that case the strings will be missing for one frame).
(In reply to Florian Quèze [:florian] from comment #4)

> For this specific bug, it looks like I could just call
> document.l10n.translateFragment near
> https://searchfox.org/mozilla-central/rev/
> 17f55aee76b7c4610a974cffd3453454e0c8de7b/toolkit/components/aboutperformance/
> content/aboutPerformance.js#1020

Thinking about it some more, I could also do the DOM update from a rAF callback.
ok. so we know why. I have no idea what's the right way to solve it.

MDN doc says:

```
The window.requestAnimationFrame() method tells the browser that you wish to perform an animation and requests that the browser call a specified function to update an animation **before the next repaint**.
```

Does it mean its a bug in rAF implementation in Gecko?
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #8)
> ok. so we know why. I have no idea what's the right way to solve it.
> 
> MDN doc says:
> 
> ```
> The window.requestAnimationFrame() method tells the browser that you wish to
> perform an animation and requests that the browser call a specified function
> to update an animation **before the next repaint**.
> ```
> 
> Does it mean its a bug in rAF implementation in Gecko?

Assuming my guess in comment 2 is correct, then either there's the bug in Gecko's rAF implementation, or the MDN doc is misleading (eg. it could be that the intended behavior of rAF is to target 60 calls per second, and skip additional calls, as they shouldn't affect the user perception of the animation smoothness).
Posted file Reduced test case
Hi Boris,

Can you advise here? We're using rAF to coalesce localizations trigger by mutations so that they happen once per paint.

Florian noticed that when resizing paints can happen independently skipping rAF which results in paints without translations.

In particular, the minimized testcase shows that the "red" background can result in a paint when the user resizes the window to be larger, and the rAF guard which switches to green will not happen then.

Is that a bug in our implementation of rAF, or is it per design and we should accept that paints can happen independently?
Flags: needinfo?(bzbarsky)
I am not up to date on how that stuff works nowadays.

It does look like nsView::WillPaintWindow can get called from widget code, plausibly from resizes, and trigger painting.   I don't know how or whether that interacts with vsync, which is what drives the refresh driver.

Matt, do you know offhand, or know who might know?
Flags: needinfo?(bzbarsky) → needinfo?(matt.woodrow)
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #12)
> I am not up to date on how that stuff works nowadays.
> 
> It does look like nsView::WillPaintWindow can get called from widget code,
> plausibly from resizes, and trigger painting.   I don't know how or whether
> that interacts with vsync, which is what drives the refresh driver.
> 
> Matt, do you know offhand, or know who might know?

The details are platform-specific, but we generally do call WillPaintWindow to force an immediate out-of-band paint when resizing so that the OS doesn't end up presenting our Window with uninitialized contents.

This is independent of the vsync-triggered refresh driver update and paint cycle, and doesn't notify refresh observers (like rAF).

I'm less sure about whether this behaviour is buggy wrt the rAF spec though and the rAF spec [1] doesn't mention anything about this. The Web App APIs spec [2] defines painting from an event loop that seems to require firing rAF before an update, but doesn't really define what to do when we need to update the rendering from an external trigger.

I've tried in the past to make us flush more from the resize paint (to fix an issue with async animations) and encountered a lot of test failures, so this probably isn't super trivial to change from the gecko end.


[1] https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#animation-frames
[2] https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
Flags: needinfo?(matt.woodrow)
I tried the approach I suggested in comment 7, and the result is consistent flicker even when not resizing. I think this is because the Fluent code calls rAF before my code, and so is called first. I'm attaching this so that you can try it, as I'm afraid any browser UI using rAF would encounter the same problem.
Attachment #9024477 - Flags: feedback?(gandalf)
This worked well in my local testing but feels like wallpapering over the problem.
Attachment #9024479 - Flags: review?(gandalf)
Attachment #9024479 - Flags: review?(gandalf) → review+
Continue the investigation.
Flags: needinfo?(gandalf)
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/69de0ff59928
Use the Fluent API directly to avoid flicker when resizing the window, r=gandalf.
https://hg.mozilla.org/mozilla-central/rev/69de0ff59928
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee: nobody → florian
You need to log in before you can comment on or make changes to this bug.