Summary: | [WK2][EFL] Implement accelerated compositing on WK2 Efl port | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | YoungTaeck Song <youngtaeck.song> | ||||||||||||||||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | gyuyoung.kim, gyuyoung.kim, hausmann, igor.oliveira, joone, kenneth, lucas.de.marchi, noam, rakuco, sw0524.lee, webkit.review.bot, yoon | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | Other | ||||||||||||||||||||||
OS: | Linux | ||||||||||||||||||||||
Bug Depends on: | 89837, 89838, 89839 | ||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||
Attachments: |
|
Description
YoungTaeck Song
2012-06-24 17:28:56 PDT
Created attachment 149213 [details]
patch
What a huge patch. It'd be good if Igor could take a look at the AC-specific parts. As for the rest, you shouldn't need to add those new files to the build system conditionally, as their code should already be protected by #ifdefs. Plus, isn't it possible to make all this conditional and not require OpenGL and the rest? Comment on attachment 149213 [details] patch Attachment 149213 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13029940 Comment on attachment 149213 [details]
patch
It depends other bugs.
So clear the flags.
Though it seems this patch is to support accelerated compositing, this patch touches many files. I think this patch is able to be separated into more smaller patches in order to be reviewed by us. If possible, it would good to file a meta bug for this feature. Comment on attachment 149213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=149213&action=review I agree with gyuyoung. But I write some comment for this patch. > Source/WebKit2/PlatformEfl.cmake:98 > + "${WTF_DIR}/wtf" IS it already in WebKit2/CMakeLists.txt ? > Source/WebKit2/PlatformEfl.cmake:161 > + LIST(APPEND WebKit2_INCLUDE_DIRECTORIES > + "${WEBCORE_DIR}/platform/graphics/surfaces" > + "${WEBCORE_DIR}/platform/graphics/texmap" > + "${WEBKIT2_DIR}/WebProcess/WebPage/web" > + "${WEBKIT2_DIR}/WebProcess/WebPage/web/efl" > + ${OPENGL_INCLUDE_DIR} > + ) > + LIST (APPEND WebKit2_LIBRARIES > + ${OPENGL_LIBRARIES} > + ) > + LIST (APPEND WebProcess_LIBRARIES > + ${OPENGL_LIBRARIES} > + ) Some of them looks not platform specific. so can we move them to CMakeLists.txt > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:306 > + ewk_view_did_change_contents_size(m_viewWidget, size); I am bad with naming. But WebKit/Efl is using XXX_changed such as ewk_view_load progress_changed. > Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:46 > -private: > - PageClientImpl(WebContext*, WebPageGroup*, Evas_Object*); > - > // PageClient > virtual PassOwnPtr<DrawingAreaProxy> createDrawingAreaProxy(); Why do you move many APIs including createDrawingAreaProxy to public section? > Source/WebKit2/UIProcess/PageClient.h:136 > +#if PLATFORM(QT) || PLATFORM(EFL) > + virtual void didChangeContentsSize(const WebCore::IntSize&) = 0; > +#endif Is this for UI_SIDDE_COMPOSITING or platform specific codes? > Source/WebKit2/WebProcess/WebPage/web/efl/LayerTreeHostEfl.cpp:21 > + Unnecessary empty line. > Source/cmake/FindOpenGL.cmake:2 > +# - Try to find OpenGL > +# Once done this will define I am not sure, but at least, cmake 2.8.7 have FindOpenGl.cmake as default. Should we really have this? > Source/cmake/OptionsEfl.cmake:153 > +IF (ENABLE_WEBKIT2) > + SET(WTF_USE_TILED_BACKING_STORE 1) > + ADD_DEFINITIONS(-DWTF_USE_TILED_BACKING_STORE=1) We should not have webkit2 specific options. Now we build both webkit1 and webkit2 as a default. > Source/cmake/OptionsEfl.cmake:174 > + FIND_PACKAGE(OpenGL REQUIRED) Indeed, I am not sure whether we should have opengl dependency as a default. > Tools/MiniBrowser/efl/main.c:102 > - app->ee = ecore_evas_new(0, 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT, 0); > + app->ee = ecore_evas_new("opengl_x11", 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT, 0); We should not fix the engine to opengl_x11. Comment on attachment 149213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=149213&action=review Set r- because of mentioned comments. > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:33 > + , m_scaleFactor(1) Is 1.0 correct ? > Source/cmake/FindOpenGL.cmake:41 > + IF (APPLE) Is this correct macro ? I didn't see (APPLE) macro in CMake before. Comment on attachment 149213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=149213&action=review >> Source/cmake/FindOpenGL.cmake:41 >> + IF (APPLE) > > Is this correct macro ? I didn't see (APPLE) macro in CMake before. Yes, it's defined by CMake itself. See `cmake --help-variables' or cmakevars(1). Created attachment 152931 [details]
patch
(In reply to comment #2) > What a huge patch. It'd be good if Igor could take a look at the AC-specific parts. As for the rest, you shouldn't need to add those new files to the build system conditionally, as their code should already be protected by #ifdefs. Plus, isn't it possible to make all this conditional and not require OpenGL and the rest? I'm Sorry about a huge patch. I splited this patch into 6 patches. New patch 152931 is still big. But Most are definitions or cmake work. And removed conditional code in cmake files. (In reply to comment #5) > Though it seems this patch is to support accelerated compositing, this patch touches many files. I think this patch is able to be separated into more smaller patches in order to be reviewed by us. If possible, it would good to file a meta bug for this feature. I'm Sorry about a huge patch. I splited this patch into 6 patches. (In reply to comment #6) Thank you for kind review. > (From update of attachment 149213 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=149213&action=review > > I agree with gyuyoung. But I write some comment for this patch. > > > Source/WebKit2/PlatformEfl.cmake:98 > > + "${WTF_DIR}/wtf" > > IS it already in WebKit2/CMakeLists.txt ? removed. > > > Source/WebKit2/PlatformEfl.cmake:161 > > + LIST(APPEND WebKit2_INCLUDE_DIRECTORIES > > + "${WEBCORE_DIR}/platform/graphics/surfaces" > > + "${WEBCORE_DIR}/platform/graphics/texmap" > > + "${WEBKIT2_DIR}/WebProcess/WebPage/web" > > + "${WEBKIT2_DIR}/WebProcess/WebPage/web/efl" > > + ${OPENGL_INCLUDE_DIR} > > + ) > > + LIST (APPEND WebKit2_LIBRARIES > > + ${OPENGL_LIBRARIES} > > + ) > > + LIST (APPEND WebProcess_LIBRARIES > > + ${OPENGL_LIBRARIES} > > + ) > > Some of them looks not platform specific. so can we move them to CMakeLists.txt fixed. > > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:306 > > + ewk_view_did_change_contents_size(m_viewWidget, size); > > I am bad with naming. > But WebKit/Efl is using XXX_changed such as ewk_view_load progress_changed. > fixed. > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:46 > > -private: > > - PageClientImpl(WebContext*, WebPageGroup*, Evas_Object*); > > - > > // PageClient > > virtual PassOwnPtr<DrawingAreaProxy> createDrawingAreaProxy(); > > Why do you move many APIs including createDrawingAreaProxy to public section? > I followed Qt ports. But I removed this code in patch 152931. > > Source/WebKit2/UIProcess/PageClient.h:136 > > +#if PLATFORM(QT) || PLATFORM(EFL) > > + virtual void didChangeContentsSize(const WebCore::IntSize&) = 0; > > +#endif > > Is this for UI_SIDDE_COMPOSITING or platform specific codes? > Qt port is using this code for UI_SIDDE_COMPOSITING and platform specific purpose. Efl is just using for UI_SIDDE_COMPOSITING. > > Source/WebKit2/WebProcess/WebPage/web/efl/LayerTreeHostEfl.cpp:21 > > + > > Unnecessary empty line. fixed. > > > Source/cmake/FindOpenGL.cmake:2 > > +# - Try to find OpenGL > > +# Once done this will define > > I am not sure, but at least, cmake 2.8.7 have FindOpenGl.cmake as default. > > Should we really have this? Thank. I didn't know that cmake has FindOpenGl.cmake as default. removed this code. > > > Source/cmake/OptionsEfl.cmake:153 > > +IF (ENABLE_WEBKIT2) > > + SET(WTF_USE_TILED_BACKING_STORE 1) > > + ADD_DEFINITIONS(-DWTF_USE_TILED_BACKING_STORE=1) > > We should not have webkit2 specific options. > > Now we build both webkit1 and webkit2 as a default. > fixed > > Source/cmake/OptionsEfl.cmake:174 > > + FIND_PACKAGE(OpenGL REQUIRED) > > Indeed, I am not sure whether we should have opengl dependency as a default. I moved this code inside WTF_USE_UI_SIDE_COMPOSITING. > > > Tools/MiniBrowser/efl/main.c:102 > > - app->ee = ecore_evas_new(0, 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT, 0); > > + app->ee = ecore_evas_new("opengl_x11", 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT, 0); > > We should not fix the engine to opengl_x11. I added a MiniBrowser's option for evas engine. (In reply to comment #7) Thank you for kind review. > (From update of attachment 149213 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=149213&action=review > > Set r- because of mentioned comments. > > > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:33 > > + , m_scaleFactor(1) > > Is 1.0 correct ? > Yes, it is. > > Source/cmake/FindOpenGL.cmake:41 > > + IF (APPLE) > > Is this correct macro ? I didn't see (APPLE) macro in CMake before. I removed this file. Comment on attachment 152931 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=152931&action=review > Source/WebKit2/PlatformEfl.cmake:51 > + UIProcess/API/efl/ViewportProcessor.cpp Nit: Upper-case file has placed above lower-case file. > Source/WebKit2/PlatformEfl.cmake:145 > + ${OPENGL_LIBRARIES} Nit : It looks OPENGL_LIBRARIES needs to be moved to above SQLITE_LIBRARIES for alphabetical order. > Source/WebKit2/PlatformEfl.cmake:161 > + ${OPENGL_LIBRARIES} ditto. > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:54 > + page()->pageGroup()->preferences()->setAcceleratedDrawingEnabled(true); Is it better to use m_page instead of page() ? Is there any reason ? > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:296 > +#if USE(UI_SIDE_COMPOSITING) Why do you use this macro function inside? Is this public API? I think we have to use macro function outside except for public APIs. > Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:109 > + virtual void didChangeContentsSize(const WebCore::IntSize&); Don't you need to use USE(UI_SIDE_COMPOSITING) as well ? > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:67 > + // adjust VisibleContentRect Nit : s/adjust/Adjust/g > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.h:47 > + ViewportProcessor(PageClientImpl*); Use *explicit* keyword. > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.h:51 > + float m_scaleFactor; Nit : I think it is better to move to below m_viewportSize because of gathering similar type variables. > Tools/MiniBrowser/efl/CMakeLists.txt:33 > + ${OPENGL_LIBRARIES} ditto. > Tools/WebKitTestRunner/PlatformEfl.cmake:46 > + ${OPENGL_LIBRARIES} ditto. Created attachment 153162 [details]
Patch
(In reply to comment #14) Thanks for your review. > (From update of attachment 152931 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152931&action=review > > > Source/WebKit2/PlatformEfl.cmake:51 > > + UIProcess/API/efl/ViewportProcessor.cpp > > Nit: Upper-case file has placed above lower-case file. > fixed. > > Source/WebKit2/PlatformEfl.cmake:145 > > + ${OPENGL_LIBRARIES} > > Nit : It looks OPENGL_LIBRARIES needs to be moved to above SQLITE_LIBRARIES for alphabetical order. > > > Source/WebKit2/PlatformEfl.cmake:161 > > + ${OPENGL_LIBRARIES} > > ditto. > fixed. > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:54 > > + page()->pageGroup()->preferences()->setAcceleratedDrawingEnabled(true); > > Is it better to use m_page instead of page() ? Is there any reason ? > fixed. > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:296 > > +#if USE(UI_SIDE_COMPOSITING) > > Why do you use this macro function inside? Is this public API? I think we have to use macro function outside except for public APIs. > > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.h:109 > > + virtual void didChangeContentsSize(const WebCore::IntSize&); > > Don't you need to use USE(UI_SIDE_COMPOSITING) as well ? > didChangeContentsSize is port specific and not just for USE(UI_SIDE_COMPOSITING). We shared Qt's didChangeContentsSize, and Qt's didChangeContentsSize has both UI_SIDE_COMPOSITING and port specific. So I removed USE(UI_SIDE_COMPOSITING) at PageClientImpl.cpp:296 and outside ewk_view_contents_size_changed. > > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:67 > > + // adjust VisibleContentRect > > Nit : s/adjust/Adjust/g > fixed. > > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.h:47 > > + ViewportProcessor(PageClientImpl*); > > Use *explicit* keyword. > fixed. > > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.h:51 > > + float m_scaleFactor; > > Nit : I think it is better to move to below m_viewportSize because of gathering similar type variables. fixed. > > > Tools/MiniBrowser/efl/CMakeLists.txt:33 > > + ${OPENGL_LIBRARIES} > > ditto. > > > Tools/WebKitTestRunner/PlatformEfl.cmake:46 > > + ${OPENGL_LIBRARIES} > > ditto. fixed. Comment on attachment 153162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153162&action=review > Source/WebKit2/Shared/WebCoreArgumentCoders.h:70 > +#if PLATFORM(QT) || PLATFORM(EFL) It looks like the en/de-coders for types like FloatPoint3D that this section forward-declares are wrapped in #if USE(UI_SIDE_COMPOSITING), so perhaps this #if PLATFORM(QT) should also be changed to use UI_SIDE_COMPOSITING instead? Created attachment 153234 [details]
Patch
(In reply to comment #17) > (From update of attachment 153162 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=153162&action=review > > > Source/WebKit2/Shared/WebCoreArgumentCoders.h:70 > > +#if PLATFORM(QT) || PLATFORM(EFL) > > It looks like the en/de-coders for types like FloatPoint3D that this section forward-declares are wrapped in #if USE(UI_SIDE_COMPOSITING), so perhaps > this #if PLATFORM(QT) should also be changed to use UI_SIDE_COMPOSITING instead? Thank you for kind review. I changed PLATFORM(QT) to UI_SIDE_COMPOSITING. And removed some classes that don't be used. Comment on attachment 153234 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153234&action=review > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:33 > +ViewportProcessor::ViewportProcessor(PageClientImpl* pageClientImpl) I don't like the name :-) plus it seems to be slightly related to the QtViewportHandler which though does much more. Anyway there are plans to make the QtViewportHandler cross platform Noam can you please look at this? Comment on attachment 153234 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153234&action=review the changes to UI-side compositing look good to me. If this was peer reviewed you can have my r+ (minus the nitpicks), or kenneth can r+ it :) > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:54 > + m_page->pageGroup()->preferences()->setAcceleratedDrawingEnabled(true); I don't think this is right. Created attachment 153940 [details]
Patch
(In reply to comment #20) > (From update of attachment 153234 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=153234&action=review > > > Source/WebKit2/UIProcess/API/efl/ViewportProcessor.cpp:33 > > +ViewportProcessor::ViewportProcessor(PageClientImpl* pageClientImpl) > > I don't like the name :-) plus it seems to be slightly related to the QtViewportHandler which though does much more. Anyway there are plans to make the QtViewportHandler cross platform Thank you for kind review. And sorry too late. I changed this class name to EflViewportProcessor. About making the QtViewportHandler cross platform, Almost are filled with Qt specific code and Qt related codes in QtViewportHandler.cpp. So I think it is good to do not extracting common code from QtViewportHandler. (In reply to comment #22) > (From update of attachment 153234 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=153234&action=review > > the changes to UI-side compositing look good to me. If this was peer reviewed you can have my r+ (minus the nitpicks), or kenneth can r+ it :) > > > Source/WebKit2/UIProcess/API/efl/PageClientImpl.cpp:54 > > + m_page->pageGroup()->preferences()->setAcceleratedDrawingEnabled(true); > > I don't think this is right. Thank you for kind review. And sorry too late. I removed this. Comment on attachment 153940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153940&action=review > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:18 > + */ It looks UIProcess/API/efl/ has used Apple BSD license. > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.h:4 > + This library is free software; you can redistribute it and/or ditto. (In reply to comment #26) > (From update of attachment 153940 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=153940&action=review > > > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:18 > > + */ > > It looks UIProcess/API/efl/ has used Apple BSD license. We decided that use this licence to new file. Please see ewk_view.cpp. > > > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.h:4 > > + This library is free software; you can redistribute it and/or > > ditto. ditto. Created attachment 155194 [details]
Patch
Comment on attachment 155194 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155194&action=review > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:83 > + if (m_visibleContentRect.x() > contentsSize.width() - m_visibleContentRect.width()) > + m_visibleContentRect.setX(contentsSize.width() - m_visibleContentRect.width()); > + if (m_visibleContentRect.x() < 0) > + m_visibleContentRect.setX(0); > + if (m_visibleContentRect.y() > contentsSize.height() - m_visibleContentRect.height()) > + m_visibleContentRect.setY(contentsSize.height() - m_visibleContentRect.height()); > + if (m_visibleContentRect.y() < 0) > + m_visibleContentRect.setY(0); This code is hard to read. Is this an intersect? Or is this a copy from some Qt code? If it's an intersection, please use IntRect::intersect. If not, please add some comments to clarify. (In reply to comment #29) > (From update of attachment 155194 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=155194&action=review > > > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:83 > > + if (m_visibleContentRect.x() > contentsSize.width() - m_visibleContentRect.width()) > > + m_visibleContentRect.setX(contentsSize.width() - m_visibleContentRect.width()); > > + if (m_visibleContentRect.x() < 0) > > + m_visibleContentRect.setX(0); > > + if (m_visibleContentRect.y() > contentsSize.height() - m_visibleContentRect.height()) > > + m_visibleContentRect.setY(contentsSize.height() - m_visibleContentRect.height()); > > + if (m_visibleContentRect.y() < 0) > > + m_visibleContentRect.setY(0); > > This code is hard to read. Is this an intersect? Or is this a copy from some Qt code? > If it's an intersection, please use IntRect::intersect. If not, please add some comments to clarify. Thanks for kind review. And I'm very sorry too late. This code is for moving visibleContentRect inside content rect when visibleContentRect is out of the content rect. Efl's webview doesn't know contents size, so We have to use this code. I'll use more clear comments at next patch. Comment on attachment 155194 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155194&action=review > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:58 > + renderer->paintToCurrentGLContext(matrix, 1.0f, rect); Though there was a little argument in EFL port, webkit coding style guides that floating point literals aren't appeneded .0, .f and .0f to . > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:1328 > + EWK_VIEW_PRIV_GET(smartData, priv); To prevent crash, I think you need to use "EWK_VIEW_PRIV_GET_OR_RETURN(smartData, priv)" Created attachment 156606 [details]
Patch
(In reply to comment #31) Thanks for kind review. > (From update of attachment 155194 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=155194&action=review > > > Source/WebKit2/UIProcess/API/efl/EflViewportHandler.cpp:58 > > + renderer->paintToCurrentGLContext(matrix, 1.0f, rect); > > Though there was a little argument in EFL port, webkit coding style guides that floating point literals aren't appeneded .0, .f and .0f to . fixed. > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:1328 > > + EWK_VIEW_PRIV_GET(smartData, priv); > > To prevent crash, I think you need to use "EWK_VIEW_PRIV_GET_OR_RETURN(smartData, priv)" fixed. Comment on attachment 156606 [details] Patch Attachment 156606 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13435704 Created attachment 156639 [details]
Patch
Comment on attachment 156639 [details]
Patch
As a basic implementation for accelerated compositing for WK2, looks good to me. But, I think this patch needs to be reviewed by proper reviewers again. I think Kenneth and Noam are them.
Comment on attachment 156639 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156639&action=review Looks good! Please fix Changelog wording before landing. > Source/WebKit2/ChangeLog:9 > + These codes are based on COORDINATED_GRAPHICS. These codes -> This implementation Created attachment 157093 [details]
Patch
(In reply to comment #37) Thank you for everything you've done for me. :) > (From update of attachment 156639 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156639&action=review > > Looks good! Please fix Changelog wording before landing. > > > Source/WebKit2/ChangeLog:9 > > + These codes are based on COORDINATED_GRAPHICS. > > These codes -> This implementation fixed. (In reply to comment #39) > (In reply to comment #37) > Thank you for everything you've done for me. :) Thanks for considering to use coordinated graphics and making it work for EFL! The more code we share the better, and if what it takes is a couple of reviews I'm fine with it... Comment on attachment 157093 [details] Patch Clearing flags on attachment: 157093 Committed r124989: <http://trac.webkit.org/changeset/124989> All reviewed patches have been landed. Closing bug. (In reply to comment #40) > (In reply to comment #39) > > (In reply to comment #37) > > Thank you for everything you've done for me. :) > Thanks for considering to use coordinated graphics and making it work for EFL! > The more code we share the better, and if what it takes is a couple of reviews I'm fine with it... It is a pleasure and honor for me to use your great work! This patch is first step of WK2 EFL COORDINATED_GRAPHICS. I'll advance this feature step by step. And I'll request your review about new patches after I accept r+ from Efl committers, :) Thank you again! (In reply to comment #43) > (In reply to comment #40) > > (In reply to comment #39) > > > (In reply to comment #37) > > > Thank you for everything you've done for me. :) > > Thanks for considering to use coordinated graphics and making it work for EFL! > > The more code we share the better, and if what it takes is a couple of reviews I'm fine with it... > > It is a pleasure and honor for me to use your great work! > This patch is first step of WK2 EFL COORDINATED_GRAPHICS. > I'll advance this feature step by step. > > And I'll request your review about new patches after I accept r+ from Efl committers, :) > > Thank you again! Did you see the nice documentation? http://trac.webkit.org/wiki/CoordinatedGraphicsSystem Kenneth (In reply to comment #44) > (In reply to comment #43) > > (In reply to comment #40) > > > (In reply to comment #39) > > > > (In reply to comment #37) > > > > Thank you for everything you've done for me. :) > > > Thanks for considering to use coordinated graphics and making it work for EFL! > > > The more code we share the better, and if what it takes is a couple of reviews I'm fine with it... > > > > It is a pleasure and honor for me to use your great work! > > This patch is first step of WK2 EFL COORDINATED_GRAPHICS. > > I'll advance this feature step by step. > > > > And I'll request your review about new patches after I accept r+ from Efl committers, :) > > > > Thank you again! > > Did you see the nice documentation? http://trac.webkit.org/wiki/CoordinatedGraphicsSystem > > Kenneth Yes! I already saw it. Thanks for the great documentation. My co-workers also read with effect. If we have some questions or suggestions, we will attach this documentation soon. |