Bug 114101 - Change requisite hardware checks for enabling multisample framebuffers.
Summary: Change requisite hardware checks for enabling multisample framebuffers.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-06 15:31 PDT by Roger Fong
Modified: 2013-04-09 16:04 PDT (History)
4 users (show)

See Also:


Attachments
patch (1.47 KB, patch)
2013-04-06 15:35 PDT, Roger Fong
no flags Details | Formatted Diff | Diff
patch (1.44 KB, patch)
2013-04-09 13:25 PDT, Roger Fong
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2013-04-06 15:31:05 PDT
m_maySupportMultisampling should be true by default for all platforms/hardware now except older versions of AMD.
Comment 1 Roger Fong 2013-04-06 15:35:25 PDT
Created attachment 196754 [details]
patch
Comment 2 Benjamin Poulain 2013-04-06 23:10:28 PDT
Comment on attachment 196754 [details]
patch

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

> Source/WebCore/platform/graphics/opengl/Extensions3DOpenGLCommon.cpp:95
> -    if (m_isNVIDIA || (m_isAMD && systemSupportsMultisampling))
> -        m_maySupportMultisampling = true;
> +    if (!(m_isNVIDIA || m_isIntel || (m_isAMD && systemSupportsMultisampling)))
> +        m_maySupportMultisampling = false;

It looks like the previous code would never put m_maySupportMultisampling = false.
 
What about leaving m_maySupportMultisampling uninitialized then?:
    m_maySupportMultisampling = m_isNVIDIA || m_isIntel || (m_isAMD && systemSupportsMultisampling);
Comment 3 Roger Fong 2013-04-08 13:10:11 PDT
I think what we ended up wanting was for it to be true by default for everything except for certain versions of AMD...

O wait...if I wanted to do that I guess I could of just done: 

+    if (m_isAMD && !systemSupportsMultisampling)))
+        m_maySupportMultisampling = false;
Comment 4 Dean Jackson 2013-04-09 12:19:22 PDT
(In reply to comment #3)
> I think what we ended up wanting was for it to be true by default for everything except for certain versions of AMD...
> 
> O wait...if I wanted to do that I guess I could of just done: 
> 
> +    if (m_isAMD && !systemSupportsMultisampling)))
> +        m_maySupportMultisampling = false;

Yeah, do that one!

We should assume other ports want this supported. Meanwhile, we know it isn't good on AMD before a certain version on OS X, so turn it off there. We might have to turn it off on older OS X systems for other hardware too, but we don't know for sure yet.
Comment 5 Roger Fong 2013-04-09 13:25:21 PDT
Created attachment 197163 [details]
patch
Comment 6 Roger Fong 2013-04-09 16:04:10 PDT
Committed:http://trac.webkit.org/changeset/148060