WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
108259
Coordinated Graphics : Remove WebCoordinatedSurface dependency from CoordinatedSurface
https://bugs.webkit.org/show_bug.cgi?id=108259
Summary
Coordinated Graphics : Remove WebCoordinatedSurface dependency from Coordinat...
Jae Hyun Park
Reported
2013-01-29 16:54:13 PST
WebCoordinatedSurface dependency should be removed from CoordinatedSurface so as to share CoordinatedSurface between WebCoordinatedSurface and CoordinatedSurface of WebKit1, which will be implemented for Threaded Coordinated Graphics.
Attachments
Patch
(11.96 KB, patch)
2013-01-29 16:57 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Patch
(11.98 KB, patch)
2013-01-29 17:04 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Patch
(8.60 KB, patch)
2013-01-30 03:20 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Patch
(9.14 KB, patch)
2013-01-30 21:16 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Remove WebCoordinatedSurface dependency from CoordinatedSurface
(6.99 KB, patch)
2013-01-30 23:40 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Patch
(9.30 KB, patch)
2013-01-30 23:43 PST
,
Jae Hyun Park
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Jae Hyun Park
Comment 1
2013-01-29 16:57:58 PST
Created
attachment 185347
[details]
Patch
Jae Hyun Park
Comment 2
2013-01-29 17:04:43 PST
Created
attachment 185348
[details]
Patch
Noam Rosenthal
Comment 3
2013-01-29 23:37:45 PST
Comment on
attachment 185348
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=185348&action=review
I think CoordinatedSurface::Factory can be static in CoordinatedSurface, the same way it is in GraphicsLayer.
> Source/WebKit2/ChangeLog:16 > + that creates CoordinatedSurface. CoordinatedImageBacking and UpdateAtlas
that creates CoordinatedSurfaces.
Jae Hyun Park
Comment 4
2013-01-30 03:20:36 PST
Created
attachment 185456
[details]
Patch
Noam Rosenthal
Comment 5
2013-01-30 03:22:32 PST
Comment on
attachment 185456
[details]
Patch LGTM, please ask a WK2 owner to sign off.
Benjamin Poulain
Comment 6
2013-01-30 15:45:09 PST
Comment on
attachment 185456
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=185456&action=review
I sign off on this change, Noam can review this for WebKit2.
> Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:18 > +/* > + Copyright (C) 2013 Company 100, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Library General Public > + License as published by the Free Software Foundation; either > + version 2 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Library General Public License for more details. > + > + You should have received a copy of the GNU Library General Public License > + along with this library; see the file COPYING.LIB. If not, write to > + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, > + Boston, MA 02110-1301, USA. > + */
Not the standard header.
> Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:24 > +#include "config.h" > + > +#if USE(COORDINATED_GRAPHICS) > + > +#include "CoordinatedSurface.h"
Should be: #include "config.h" #include "CoordinatedSurface.h" #if FOOBAR
> Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:43 > +#endif // USE(COORDINATED_GRAPHICS) > + > +} // namespace WebKit
the #define does not match the namespace scope.
> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h:119 > + // CoordinatedSurface
Useless comment.
Jae Hyun Park
Comment 7
2013-01-30 21:16:51 PST
Created
attachment 185672
[details]
Patch
Noam Rosenthal
Comment 8
2013-01-30 23:29:10 PST
Comment on
attachment 185672
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=185672&action=review
> Source/WebKit2/ChangeLog:18 > + This patch introduces CoordinatedSurface::Factory, which is a function pointer > + that creates CoordinatedSurfaces. CoordinatedImageBacking and UpdateAtlas > + create a CoordinatedSurface using the function pointer passed by > + CoordinatedLayerTreeHost, instead of using CoordinatedSurface::create. This
We don't pass the function pointer to those classes, the changelog need to be updated.
Jae Hyun Park
Comment 9
2013-01-30 23:40:09 PST
Created
attachment 185687
[details]
Remove WebCoordinatedSurface dependency from CoordinatedSurface
Jae Hyun Park
Comment 10
2013-01-30 23:43:06 PST
Created
attachment 185689
[details]
Patch
Jae Hyun Park
Comment 11
2013-01-30 23:51:59 PST
(In reply to
comment #8
)
> (From update of
attachment 185672
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=185672&action=review
> > > Source/WebKit2/ChangeLog:18 > > + This patch introduces CoordinatedSurface::Factory, which is a function pointer > > + that creates CoordinatedSurfaces. CoordinatedImageBacking and UpdateAtlas > > + create a CoordinatedSurface using the function pointer passed by > > + CoordinatedLayerTreeHost, instead of using CoordinatedSurface::create. This > > We don't pass the function pointer to those classes, the changelog need to be updated.
Fixed. Thanks for the review!
WebKit Review Bot
Comment 12
2013-01-31 00:31:16 PST
Comment on
attachment 185689
[details]
Patch Clearing flags on attachment: 185689 Committed
r141384
: <
http://trac.webkit.org/changeset/141384
>
WebKit Review Bot
Comment 13
2013-01-31 00:31:22 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.
Top of Page
Format For Printing
XML
Clone This Bug