Bug 77922 - Apple/Safari: Enable WebGL multisampling on ATI cards for particular builds
Summary: Apple/Safari: Enable WebGL multisampling on ATI cards for particular builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks: 90649
  Show dependency treegraph
 
Reported: 2012-02-06 18:22 PST by Dean Jackson
Modified: 2012-07-05 18:53 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.57 KB, patch)
2012-02-06 19:43 PST, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2012-02-06 19:52 PST, Dean Jackson
cmarrin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2012-02-06 18:22:57 PST
Currently GraphicsContext3DOpenGLCommon validates the incoming antialias attribute and only enables it for NVIDIA cards. Chromium allows multisampling on OS X + ATI when on 10.7.2 and above.

For more info:

http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/software_rendering_list/software_rendering_list.json&exact_package=chromium&ct=rc&cd=1&q=software_rendering_list.json
http://src.chromium.org/viewvc/chrome/trunk/deps/gpu/software_rendering_list/software_rendering_list.json?view=log
Comment 1 Radar WebKit Bug Importer 2012-02-06 18:23:54 PST
<rdar://problem/10817469>
Comment 2 Kenneth Russell 2012-02-06 18:39:54 PST
zmo's responsible for Chromium's GPU blacklist and could help advise on changes in this area.

Though this is far beyond the scope of this issue, it would be fantastic if we could figure out how to share the GPU blacklist information not only between WebKit ports but also between browsers.
Comment 3 James Robinson 2012-02-06 18:45:10 PST
Firefox exposes their blacklisting info on this wiki: https://wiki.mozilla.org/Blocklisting/Blocked_Graphics_Drivers. It looks like they've taken a slightly different approach to MSAA, quoting from the version currently there:

"For MSAA, we block all ATI cards except for AMD Radeon HD 6490M (device id 0x6760) and ATI Radeon HD 4670 (device id 0x9488). See Chromium issue 83153 [1]."
Comment 4 Kenneth Russell 2012-02-06 18:48:13 PST
zmo recently verified that even older ATI hardware seems to support multisampled frame buffer objects well with 10.7.2, where the same cards hung while creating a multisampled renderbuffer on earlier versions of Mac OS. That informed Chrome's decision to stop blacklisting ATI cards on 10.7.
Comment 5 Dean Jackson 2012-02-06 19:43:31 PST
Created attachment 125754 [details]
Patch
Comment 6 Dean Jackson 2012-02-06 19:47:46 PST
(In reply to comment #2)

> Though this is far beyond the scope of this issue, it would be fantastic if we could figure out how to share the GPU blacklist information not only between WebKit ports but also between browsers.

Definitely! I expect we'll (Apple'll) will always lag on this while WebGL is disabled by default. It would be so much nicer for us if there was a shared system.
Comment 7 Early Warning System Bot 2012-02-06 19:47:47 PST
Comment on attachment 125754 [details]
Patch

Attachment 125754 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11436119
Comment 8 Dean Jackson 2012-02-06 19:49:22 PST
Comment on attachment 125754 [details]
Patch

breaks QT
Comment 9 Dean Jackson 2012-02-06 19:52:19 PST
Created attachment 125756 [details]
Patch
Comment 10 Dean Jackson 2012-02-07 09:36:36 PST
http://trac.webkit.org/changeset/106956
Comment 11 Alexey Proskuryakov 2012-02-07 10:00:20 PST
Comment on attachment 125756 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:63
> +    static SInt32 version;

I recommend always adding ASSERT(isMainThread()) in functions that use static variables.

> Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:69
> +    return version > 0x1072;

This doesn't match comments or description in ChangeLog (should be ">="?)
Comment 12 Dean Jackson 2012-02-07 10:06:44 PST
(In reply to comment #11)
> (From update of attachment 125756 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=125756&action=review
> 
> > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:63
> > +    static SInt32 version;
> 
> I recommend always adding ASSERT(isMainThread()) in functions that use static variables.

I'll fix this.

> > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:69
> > +    return version > 0x1072;
> 
> This doesn't match comments or description in ChangeLog (should be ">="?)

Indeed! Sorry.
Comment 13 Dean Jackson 2012-02-07 11:19:22 PST
Followup. http://trac.webkit.org/changeset/106964