Summary: | [EFL][WK2] Extract the control of page background out of color_set | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryuan Choi <ryuan.choi> | ||||||||||||||
Component: | WebKit EFL | Assignee: | Ryuan Choi <ryuan.choi> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | benjamin, buildbot, bunhere, cdumez, cmarcelo, commit-queue, enmi.lee, esprehn+autocc, glenn, gyuyoung.kim, gyuyoung.kim, jinwoo7.song, kondapallykalyan, lucas.de.marchi, luiz, noam, rniwa, sergio, simon.fraser, webkit-ews | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Ryuan Choi
2014-01-23 20:52:25 PST
After discussed more with eunmi and hyowon, I changed my mind. Instead of keeping two APIs with color_set, ewk_view_bg_color_set looks enough for the application developers. They will only want to change the default background color. All other are internal issues. Created attachment 223066 [details]
suggestion
Comment on attachment 223066 [details]
suggestion
Although we landed this, we can't have webview drawn other color.
It looks a bug of coordinated graphics or texmap when disabled DrawsBackground with transparant background.
I will investigate it more.
Created attachment 223094 [details]
suggestion
Comment on attachment 223094 [details] suggestion Attachment 223094 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4968362132111360 New failing tests: http/tests/security/contentSecurityPolicy/source-list-parsing-06.html Created attachment 223105 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 223193 [details]
suggestion
ping, noam, gyuyong. Could you take a look at this ? Comment on attachment 223193 [details] suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=223193&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:734 > + WKViewSetDrawsTransparentBackground(impl->wkView(), false); s/false/true ? Comment on attachment 223193 [details] suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=223193&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.h:900 > +EAPI void ewk_view_bg_color_set(Evas_Object *o, int r, int g, int b, int a); Any API test for this new API ? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:911 > +EAPI void ewk_view_bg_color_get(Evas_Object *o, int *r, int *g, int *b, int *a); Missing *const* in front of Evas_Object. Comment on attachment 223193 [details] suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=223193&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.h:900 > +EAPI void ewk_view_bg_color_set(Evas_Object *o, int r, int g, int b, int a); Any API test for this new API ? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:911 > +EAPI void ewk_view_bg_color_get(Evas_Object *o, int *r, int *g, int *b, int *a); Missing *const* in front of Evas_Object. Created attachment 226492 [details]
Patch
Created attachment 226496 [details]
Patch
(In reply to comment #9) > (From update of attachment 223193 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=223193&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:734 > > + WKViewSetDrawsTransparentBackground(impl->wkView(), false); > > s/false/true ? It's not important if we disable DrawsBackground. Anyway, I removed it. (In reply to comment #11) > (From update of attachment 223193 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=223193&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_view.h:900 > > +EAPI void ewk_view_bg_color_set(Evas_Object *o, int r, int g, int b, int a); > > Any API test for this new API ? I added simple one. In order to test it better, we should have capture API. > > > Source/WebKit2/UIProcess/API/efl/ewk_view.h:911 > > +EAPI void ewk_view_bg_color_get(Evas_Object *o, int *r, int *g, int *b, int *a); > > Missing *const* in front of Evas_Object. Right, I fixed. Thanks. Looks good to me. Basically I don't object to add APIs to change background color. However, I wonder whether other ports want to use this functionality. Do you know what port already support this feature or will support it ? (In reply to comment #16) > Basically I don't object to add APIs to change background color. However, I wonder whether other ports want to use this functionality. Do you know what port already support this feature or will support it ? Well, I don't know which port want this feature because basic browser does not require transparent webview. we need this to support our web application framework. Anyway, because Root layer always set ContentsOpaque to true, other ports except IOS can't render transparent background with accelerated compositing. (see http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerBacking.cpp#L308) FYI, WebCore itself can draw contents with transparent background by turning DrawsBackground off. This patch just make it working with Coordinated graphics on the EFL port. (In reply to comment #17) > (In reply to comment #16) > > Basically I don't object to add APIs to change background color. However, I wonder whether other ports want to use this functionality. Do you know what port already support this feature or will support it ? > > Well, I don't know which port want this feature because basic browser does not require transparent webview. > we need this to support our web application framework. > > Anyway, because Root layer always set ContentsOpaque to true, other ports except IOS can't render transparent background with accelerated compositing. > (see http://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayerBacking.cpp#L308) > > FYI, WebCore itself can draw contents with transparent background by turning DrawsBackground off. > This patch just make it working with Coordinated graphics on the EFL port. I see. I don't mind to land this patch from EFL port side. I think this patch needs to get review from WK2 owner and Coordinated Graphics owner. Noam or Benjamin may review this patch. Comment on attachment 226496 [details]
Patch
This patch only modifies the coordinated graphics and just enable a code block of rendering for EFL port. So, I believe port maintainer can land this. LGTM.
Committed r166566: <http://trac.webkit.org/changeset/166566> Comment on attachment 226496 [details]
Patch
clearing flags.
Landed patch after fixed merge issue.
|