Last Comment Bug 758048 - Use accelerated layers for <panels>
: Use accelerated layers for <panels>
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Nicolas Silva [:nical]
:
Mentors:
Depends on: 751370
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-23 15:55 PDT by Justin Dolske [:Dolske]
Modified: 2012-06-22 03:43 PDT (History)
9 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Allows acceleration of xul panels (6.65 KB, patch)
2012-05-23 19:12 PDT, Nicolas Silva [:nical]
netzen: review+
Details | Diff | Splinter Review
Allows acceleration of xul panels. (6.67 KB, patch)
2012-05-31 10:56 PDT, Nicolas Silva [:nical]
netzen: review+
Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2012-05-23 15:55:07 PDT
+++ This bug was initially created as a clone of Bug #751370 +++

Bug 751370 was filed to "not use accelerated layers for small popups windows on Linux". The patch disables layers for <panel> too, though, which can contain arbitrarily complex content. We don't do that in Firefox (yet?), but I suppose some addon might use a <panel> for something that would benefit from this.

We should probably just exempt <panel> from the bug 751370 fix. (Or possibly make it an optin/optout thing? Not sure that complexity is worth it.)
Comment 1 Nicolas Silva [:nical] 2012-05-23 19:12:29 PDT
Created attachment 626672 [details] [diff] [review]
Allows acceleration of xul panels

This small patch does two things: 
 - it moves the PopupType() getter (and the mPopupType member) from windows's nsWindow class to the cross platform base class (nsBaseWidget), which makes more sense to me since the attribute is as relevent on linux or mac as it is on windows.
 - it uses the popup type when choosing whether or not we disable acceleration (allowing xul panels to be accelerated).
Comment 2 Nicolas Silva [:nical] 2012-05-23 19:17:18 PDT
I just pushed the patch to try servers (I'd be surprised that it breaks something though).
Comment 3 Nicolas Silva [:nical] 2012-05-25 10:16:30 PDT
try results look good.
Comment 4 Justin Dolske [:Dolske] 2012-05-29 14:14:42 PDT
Comment on attachment 626672 [details] [diff] [review]
Allows acceleration of xul panels

(I can't review this, bouncing over.)
Comment 5 Benoit Girard (:BenWa) 2012-05-29 14:49:52 PDT
Comment on attachment 626672 [details] [diff] [review]
Allows acceleration of xul panels

This is touching widget code, none of which is cocoa. Joe suggested bbondy to review this.
Comment 6 Brian R. Bondy [:bbondy] 2012-05-29 16:17:40 PDT
Comment on attachment 626672 [details] [diff] [review]
Allows acceleration of xul panels

Review of attachment 626672 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!
Comment 7 Brian R. Bondy [:bbondy] 2012-05-29 16:19:04 PDT
I had a nit in another open window:

> bool isSmallPopup = ((mWindowType == eWindowType_popup)
> 		&& (mPopupType != ePopupTypePanel));

nit: Please put the && on the previous line and left-align to the right hand side of the equal sign.
Comment 8 Nicolas Silva [:nical] 2012-05-31 10:56:54 PDT
Created attachment 628808 [details] [diff] [review]
Allows acceleration of xul panels.

> > bool isSmallPopup = ((mWindowType == eWindowType_popup)
> > 		&& (mPopupType != ePopupTypePanel));
>
> nit: Please put the && on the previous line and left-align to the right hand > side of the equal sign.

Fixed :)
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-06-21 13:49:01 PDT
Comment on attachment 628808 [details] [diff] [review]
Allows acceleration of xul panels.

Just use checkin-needed.
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-06-21 17:49:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5fb5ad11849
Comment 11 Ed Morley [:emorley] 2012-06-22 03:43:48 PDT
https://hg.mozilla.org/mozilla-central/rev/e5fb5ad11849

Note You need to log in before you can comment on or make changes to this bug.