Closed
Bug 1205475
Opened 9 years ago
Closed 7 years ago
nsIFrame::HasOpacityInternal takes a lot of time
Categories
(Core :: Web Painting, defect)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: mattwoodrow, Assigned: u459114)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
(Keywords: perf)
Attachments
(2 files, 1 obsolete file)
Checking for the presence of async animations on every frame when we try paint ends up taking a considerable amount of time. We should add a NS_FRAME_MAY_HAVE_OPACITY state bit to avoid most of this work when we can.
Comment 3•7 years ago
|
||
I don't have resource to work on this bug right now. Feel free to take this bug.
Flags: needinfo?(howareyou322)
Component: Layout → Layout: Web Painting
Keywords: perf
Updated•7 years ago
|
Assignee: howareyou322 → cku
The idea is set NS_FRAME_MAY_HAVE_OPACITY flag into frame right before animation start and remove it at the end of animation
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Add NS_FRAME_MAY_HAVE_OPACITY to element in EffectSet::AddEffect, and clear this flag in EffectSet::RemoveEffect
Comment hidden (mozreview-request) |
Attachment #8867991 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
To bad, FRAME_STATE_BIT of nsFrame is full...BooleanFlag of nsINode is full too.. I need to find anohter place to put this flag
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Attachment #8868096 -
Flags: review?(matt.woodrow)
Attachment #8868448 -
Flags: review?(matt.woodrow)
Reporter | ||
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8868096 [details] Bug 1205475 - Part 1. Hold MAY_HAVE_OPACITY_ANIM/ MAY_HAVE_TRANSFOMR_ANIM information in EffectSet. https://reviewboard.mozilla.org/r/139710/#review143756 ::: layout/base/nsLayoutUtils.cpp:517 (Diff revision 5) > } > ); > } > > +static bool > +CheckOpacityAndTransformAnim(EffectSet* effects, nsCSSPropertyID aProperty) MayHaveAnimationOfProperty?
Attachment #8868096 -
Flags: review?(matt.woodrow) → review+
Reporter | ||
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8868448 [details] Bug 1205475 - Part 2. Prevent multiple EffectSet property look up. https://reviewboard.mozilla.org/r/140050/#review143758 Nice!
Attachment #8868448 -
Flags: review?(matt.woodrow) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 23•7 years ago
|
||
Pushed by cku@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d7b25abd4866 Part 1. Hold MAY_HAVE_OPACITY_ANIM/ MAY_HAVE_TRANSFOMR_ANIM information in EffectSet. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/cf4e37d17683 Part 2. Prevent multiple EffectSet property look up. r=mattwoodrow
Comment 24•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7b25abd4866 https://hg.mozilla.org/mozilla-central/rev/cf4e37d17683
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 25•7 years ago
|
||
mozreview-review |
Comment on attachment 8868096 [details] Bug 1205475 - Part 1. Hold MAY_HAVE_OPACITY_ANIM/ MAY_HAVE_TRANSFOMR_ANIM information in EffectSet. https://reviewboard.mozilla.org/r/139710/#review144950 ::: dom/animation/EffectSet.h:263 (Diff revision 6) > + uint32_t mMayHaveOpacityAnim; > + uint32_t mMayHaveTransformAnim; Is there any strong reason why we use uint32_t instead of bool?
Assignee | ||
Comment 26•7 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #25) > Comment on attachment 8868096 [details] > Bug 1205475 - Part 1. Hold MAY_HAVE_OPACITY_ANIM/ MAY_HAVE_TRANSFOMR_ANIM > information in EffectSet. > > https://reviewboard.mozilla.org/r/139710/#review144950 > > ::: dom/animation/EffectSet.h:263 > (Diff revision 6) > > + uint32_t mMayHaveOpacityAnim; > > + uint32_t mMayHaveTransformAnim; > > Is there any strong reason why we use uint32_t instead of bool? No, my bad. It should be bool. Thank you for catching this.
Comment 27•7 years ago
|
||
Pushed by cku@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7af9cc734f9e (followup) Declare mMayHaveOpacityAnim and mMayHaveTransformAnim as bool. r=me
Comment 28•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7af9cc734f9e
You need to log in
before you can comment on or make changes to this bug.
Description
•