Bug 37772

Summary: WebGL rendering context does not activate unless accelerated compositing is enabled from settings.
Product: WebKit Reporter: Jarkko Sakkinen <jarkko.j.sakkinen>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin, commit-queue, eric, hausmann, kbr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Allows creation of WebGL rendering context if webGLEnabled flag is true and does not also require acceleratedCompositing flag also to be true.
hausmann: review-, hausmann: commit-queue-
Based on suggestion by Kenneth Russel none

Description Jarkko Sakkinen 2010-04-18 07:12:41 PDT
When QtLauncher is used (for example) web pages that have WebGL content don't load unless accelerated compositing is enabled from QWebSettings.
Comment 1 Jarkko Sakkinen 2010-04-18 07:33:04 PDT
I have an idea about cause of this bug. Posting patch as soon as I have verified that it fixes the issue.
Comment 2 Jarkko Sakkinen 2010-04-20 12:00:12 PDT
Created attachment 53861 [details]
Allows creation of WebGL rendering context if webGLEnabled flag is true and does not also require acceleratedCompositing flag also to be true.
Comment 3 Simon Hausmann 2010-04-28 12:40:19 PDT
It looks like the original change from bug https://bugs.webkit.org/show_bug.cgi?id=35759 was intentional...
Comment 4 Chris Marrin 2010-04-28 13:38:15 PDT
Comment on attachment 53861 [details]
Allows creation of WebGL rendering context if webGLEnabled flag is true and does not also require acceleratedCompositing flag also to be true.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 82feb24..22c8c71 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,17 @@
> +2010-04-20  Jarkko Sakkinen  <jarkko.sakkinen@tieto.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Qt] WebGL rendering context does not activate unless accelerated compositing is enabled from settings.
> +        https://bugs.webkit.org/show_bug.cgi?id=37772
> +        
> +        Allows WebGL rendering context creation is webGLEnabled flag
> +        is true in the document settings and does not require 
> +        also acceleratedCompositing flag also to be enabled.
> +
> +        * html/HTMLCanvasElement.cpp:
> +        (WebCore::HTMLCanvasElement::getContext):
> +
>  2010-04-19  Kevin Ollivier  <kevino@theolliviers.com>
>  
>          Fix the Mac builders for now by restoring the keepAlive function.
> diff --git a/WebCore/html/HTMLCanvasElement.cpp b/WebCore/html/HTMLCanvasElement.cpp
> index ef85323..5367040 100644
> --- a/WebCore/html/HTMLCanvasElement.cpp
> +++ b/WebCore/html/HTMLCanvasElement.cpp
> @@ -145,7 +145,7 @@ CanvasRenderingContext* HTMLCanvasElement::getContext(const String& type, Canvas
>      }
>  #if ENABLE(3D_CANVAS)    
>      Settings* settings = document()->settings();
> -    if (settings && settings->webGLEnabled() && settings->acceleratedCompositingEnabled()) {
> +    if (settings && settings->webGLEnabled()) {
>          // Accept the legacy "webkit-3d" name as well as the provisional "experimental-webgl" name.
>          // Once ratified, we will also accept "webgl" as the context name.
>          if ((type == "webkit-3d") ||

The Mac implementation requires acceleratedCompositing for WebGL rendering. It's reasonable to remove the test for acceleratedCompositing here as long as, for the Mac platform, webGLEnabled has a test to prevent it from being true unless acceleratedCompositing is also true.
Comment 5 Kenneth Russell 2010-05-09 15:28:11 PDT
Comment on attachment 53861 [details]
Allows creation of WebGL rendering context if webGLEnabled flag is true and does not also require acceleratedCompositing flag also to be true.

This change looks fine. I'm surprised Chromium's WebGL support wasn't broken by the original patch.
Comment 6 Simon Hausmann 2010-06-07 04:10:46 PDT
Comment on attachment 53861 [details]
Allows creation of WebGL rendering context if webGLEnabled flag is true and does not also require acceleratedCompositing flag also to be true.

It does not appear that this patch takes Chris' commen into account:

"The Mac implementation requires acceleratedCompositing for WebGL rendering. It's reasonable to remove the test for acceleratedCompositing here as long as, for the Mac platform, webGLEnabled has a test to prevent it from being true unless acceleratedCompositing is also true."

So landing this fix would re-open Bug 35759
Comment 7 Kenneth Russell 2010-06-07 11:31:11 PDT
(In reply to comment #6)
> (From update of attachment 53861 [details])
> It does not appear that this patch takes Chris' commen into account:
> 
> "The Mac implementation requires acceleratedCompositing for WebGL rendering. It's reasonable to remove the test for acceleratedCompositing here as long as, for the Mac platform, webGLEnabled has a test to prevent it from being true unless acceleratedCompositing is also true."
> 
> So landing this fix would re-open Bug 35759

Note that in https://bugs.webkit.org/show_bug.cgi?id=40085 a patch was landed adding an #ifdef around the test of accelerated compositing being enabled. You could add another arm to the #if with something like

#if !PLATFORM(CHROMIUM) && !PLATFORM(QT)

Or, if you're willing to do the build plumbing, you could completely generalize this into a new build flag per https://bugs.webkit.org/show_bug.cgi?id=40091 .
Comment 8 Jarkko Sakkinen 2010-06-08 01:07:29 PDT
Created attachment 58122 [details]
Based on suggestion by Kenneth Russel
Comment 9 WebKit Commit Bot 2010-06-09 19:29:11 PDT
Comment on attachment 58122 [details]
Based on suggestion by Kenneth Russel

Clearing flags on attachment: 58122

Committed r60930: <http://trac.webkit.org/changeset/60930>
Comment 10 WebKit Commit Bot 2010-06-09 19:29:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Hausmann 2010-06-10 03:29:00 PDT
WebGL is not part of the qtwebkit-2.0 branch, so I'm removing this bug from the list of bugs/patches to include in the release branch.