Bug 154100

Summary: [ThreadedCompositor] position:fixed elements do not have their own layers when threaded compositor is enabled.
Product: WebKit Reporter: ChangSeok Oh <changseok>
Component: WebKitGTKAssignee: ChangSeok Oh <changseok>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, gustavo, ryuan.choi, yoon
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 154066    
Attachments:
Description Flags
a test case
none
Patch
none
Patch none

ChangSeok Oh
Reported 2016-02-10 21:38:24 PST
Created attachment 271043 [details] a test case SSIA. Run the attached test case on threaded compositor enabled webkit build.
Attachments
a test case (237 bytes, text/html)
2016-02-10 21:38 PST, ChangSeok Oh 		no flags
Details
Patch (2.51 KB, patch)
2016-02-12 01:22 PST, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
Patch (2.23 KB, patch)
2016-02-12 02:00 PST, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
ChangSeok Oh
Comment 1 2016-02-12 01:22:48 PST
Created attachment 271150 [details] Patch
Gwang Yoon Hwang
Comment 2 2016-02-12 01:44:44 PST
Comment on attachment 271150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271150&action=review Nice patch, thanks! Some nitpiks in the changelog.. > Source/WebKit2/ChangeLog:9 > + embeded environment. Probably main concern was increasing video memory for fixed elements. s/embeded/embedded/, I'm not sure whether is it correct reason or not. As we talked in IRC, it is because the fixed element is not considered to be a new stacking context at first. This feature was added later because of the scrolling performance in the embedded system. > Source/WebKit2/ChangeLog:10 > + However, it is time to rethink spliting the elements out to separate layers. Currently the fixed spliting/splitting and how about adding a comma after "Currently" ? > Source/WebKit2/ChangeLog:11 > + elements are redrawn to a backing store every time layout or scroll happens. It results in bad Also maybe you missed "the" before bad effect
ChangSeok Oh
Comment 3 2016-02-12 01:58:35 PST
Comment on attachment 271150 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271150&action=review Thanks for your review! >> Source/WebKit2/ChangeLog:9 >> + embeded environment. Probably main concern was increasing video memory for fixed elements. > > s/embeded/embedded/, > I'm not sure whether is it correct reason or not. As we talked in IRC, it is because the fixed element is not considered to be a new stacking context at first. This feature was added later because of the scrolling performance in the embedded system. I see. Then Let's get rid of the whole lines. >> Source/WebKit2/ChangeLog:10 >> + However, it is time to rethink spliting the elements out to separate layers. Currently the fixed > > spliting/splitting > and how about adding a comma after "Currently" ? Yep. >> Source/WebKit2/ChangeLog:11 >> + elements are redrawn to a backing store every time layout or scroll happens. It results in bad > > Also maybe you missed "the" before bad effect Done.
ChangSeok Oh
Comment 4 2016-02-12 02:00:25 PST
Created attachment 271154 [details] Patch
ChangSeok Oh
Comment 5 2016-02-15 01:16:05 PST
Any concern?
Gwang Yoon Hwang
Comment 6 2016-02-15 01:20:20 PST
I'm okay with this patch. Maybe cgarcia can help you. :)
ChangSeok Oh
Comment 7 2016-02-15 20:49:48 PST
Comment on attachment 271154 [details] Patch Thanks.
WebKit Commit Bot
Comment 8 2016-02-15 21:39:56 PST
Comment on attachment 271154 [details] Patch Clearing flags on attachment: 271154 Committed r196623: <http://trac.webkit.org/changeset/196623>
WebKit Commit Bot
Comment 9 2016-02-15 21:40:02 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.