Screen paint/refresh update issue on video element (even if pause video)

RESOLVED FIXED in Firefox 45

Status

()

Core
Audio/Video: Playback
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Alice0775 White, Unassigned)

Tracking

({power})

Trunk
mozilla45
Unspecified
Windows 7
power
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Steps to reproduce:
1. set nglayout.debug.paint_flashing = true
2. Open data:text/html;charset=utf-8,<video autoplay="autoplay" controls="" loop="loop" preload="none" height="350" width="610"><source src="https://videos.cdn.mozilla.net/uploads/mozillaorg/Firefox_global_final.webm" type="video/webm"></video>
3. Wait until become looping
   --- Observe, unnecessary screen paint/refresh starts
4. Pause video
   --- Observe, unnecessary screen paint/refresh continued

Actual Results:
Unnecessary screen paint/refresh starts when the video loops to playback
Screen paints/refreshes continuously even if the video is pausing

Expected Results:
nnecessary screen paint/refresh should not start.
Screen should stop paint/refreshe, because the video is pausing.
(Reporter)

Updated

2 years ago
Blocks: 1213344
(Reporter)

Comment 1

2 years ago
Created attachment 8672281 [details]
screencast, what happens
(Reporter)

Updated

2 years ago
Keywords: power
(Reporter)

Comment 2

2 years ago
Actually, it is not necessary the loop attribute. 
Moving seekbar causes the screen unnecessary paint/refresh.
No longer blocks: 449157
Is there a way to tell where in the stack this is happening, and how expensive the spurious repaints are?
Flags: needinfo?(jmuizelaar)
Priority: -- → P2
Summary: Screen paint/refresh update issue with loop attribute on video element (even if pause video) → Screen paint/refresh update issue on video element (even if pause video)
(Reporter)

Updated

2 years ago
No longer blocks: 1213344
Component: Audio/Video → Audio/Video: Playback
Robert - is this a compositor issue?
Flags: needinfo?(roc)
No. This is pretty clearly a layout bug involving the spinner causing unnecessary invalidation. Looking into it.
Flags: needinfo?(roc)
https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml
https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/media/videocontrols.css

After seeking we get into a state where the statusIcon element has type="throbber", so it gets the animated throbber image. Its statusOverlay parent has the fadeOut attribute set, so is opacity 0, which is why you can't see the throbber.

Arguably, after the fade-out transition has finished the videocontrols themselves should remove the throbber state so we're not continuously animating the image. However, it's best if we automatically avoid repainting for animated content inside an opacity:0 element. I think DLBI should already be doing that so I need to figure why that's not working.
Created attachment 8685749 [details]
MozReview Request: Bug 1213582. Skip display items in ProcessDisplayItems if we only need items for event regions, and this item isn't one and doesn't have descendants. r=mattwoodrow

Bug 1213582. Skip display items in ProcessDisplayItems if we only need items for event regions, and this item isn't one and doesn't have descendants. r=mattwoodrow
Attachment #8685749 - Flags: review?(matt.woodrow)
Created attachment 8685750 [details]
MozReview Request: Bug 1213582. Don't flatten away opacity:0 containers. r=mattwoodrow

Bug 1213582. Don't flatten away opacity:0 containers. r=mattwoodrow
Attachment #8685750 - Flags: review?(matt.woodrow)
Comment on attachment 8685749 [details]
MozReview Request: Bug 1213582. Skip display items in ProcessDisplayItems if we only need items for event regions, and this item isn't one and doesn't have descendants. r=mattwoodrow

https://reviewboard.mozilla.org/r/24879/#review22413
Attachment #8685749 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8685750 [details]
MozReview Request: Bug 1213582. Don't flatten away opacity:0 containers. r=mattwoodrow

https://reviewboard.mozilla.org/r/24881/#review22415
Attachment #8685750 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1935f0c639b3c52a0046d1eab7888fbda65325a
Bug 1213582. Skip display items in ProcessDisplayItems if we only need items for event regions, and this item isn't one and doesn't have descendants. r=mattwoodrow

https://hg.mozilla.org/integration/mozilla-inbound/rev/55a30946c6fba7e287099f956065df7508ab1a8a
Bug 1213582. Don't flatten away opacity:0 containers. r=mattwoodrow

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c1935f0c639b
https://hg.mozilla.org/mozilla-central/rev/55a30946c6fb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Flags: needinfo?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.