[Webkit-unassigned] [Bug 139028] [GTK] Enable depth 32 for the RedirectedXCompositeWindow

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=139028

--- Comment #10 from Julien Isorce <j.isorce at samsung.com> ---
Comment on attachment 242197
  --> https://bugs.webkit.org/attachment.cgi?id=242197
gtk: support RGBA end-to-end compositing when using RedirectedXCompositeWindow

View in context: https://bugs.webkit.org/attachment.cgi?id=242197&action=review

>> Source/WebKit2/ChangeLog:3
>> +        gtk: support RGBA end-to-end compositing when using RedirectedXCompositeWindow
> 
> Please, use the bug title here and move the bug url, see other changelog entries.

Done

>> Source/WebKit2/ChangeLog:12
>> +        wants to interact with the alpha channel at compositing time.
> 
> Do you have a test case or is there any website that are not rendering correctly because of this?

I do not think this is useful when webkit is used for classic desktop webbrowser (though maybe for :root background=transparent, body: background=transparent). It is more for transparent UI.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:264
>> +        bool supportAlpha = depth == 32 && (!priv->redirectedWindow || !priv->redirectedWindow->hasAlpha());
> 
> I think here you should check if the widget visual is the screen rgba visual.
> 
> visual == gdk_screen_get_rgba_visual(gdk_display_get_default_screen(display)));

It now matches visual and depth from the parent winID in all cases.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:269
>> +                    supportAlpha);
> 
> Why are we creating this here again? If it's too early in constructed, we can create the redirected window when realized, but not in both places, I would say.

Exactly it was to early, i.e. before the parent was created. Ok thx, I now only left the creation when realized.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:271
>> +                    priv->pageProxy->setAcceleratedCompositingWindowId(priv->redirectedWindow->windowID());
> 
> This is also done when the page is created

Done only one time now.

>> Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.cpp:203
>> +        XMapWindow(display, m_parentWindow);
> 
> Do we really need two paths here? or can we set the window attrs, the mask, etc depending on whether alpha is supported or not and then create the window and call XMapWindow using the same code?

You are right, I kept the code before my changes except I pass the winID application instead of the root windID. So that depth and visual matches the parent.
A little note is that once the window is created I change the parent again to be the root window. So that no need to take care of destroying the child before the parent. Anyway they do not need to depend on each other.

>> Source/WebKit2/UIProcess/gtk/RedirectedXCompositeWindow.cpp:245
>> +        XFreeColormap(m_display, m_colormap);
> 
> Could we check m_colormap instead of m_hasAlpha and then we don't need to keep m_hasAlpha at all?

I removed both.

>> Tools/ChangeLog:3
>> +        gtk: support RGBA end-to-end compositing when using RedirectedXCompositeWindow
> 
> Same here about changelog formatting.

Done

>> Tools/MiniBrowser/gtk/main.c:69
>> +    }
> 
> I don't understand why this is a user decision, doesn't it depend on whether it's supported or not?

You are right, for MiniBrowser let's just set a rgba visual if available.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141201/e109a2c4/attachment-0002.html>