792190 - Relax D3D9 driver version requirements

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Relax D3D9 driver requirements

We agreed to try and relax the D3D9 driver version requirements. This patch does just that, let's see how it goes.

Comment 1

[Joe Drew (not getting mail)]

Comment on attachment 662279 [details] [diff] [review]
Relax D3D9 driver requirements

Review of attachment 662279 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/GfxInfo.cpp
@@ +744,4 @@
>      APPEND_TO_DRIVER_BLOCKLIST( DRIVER_OS_WINDOWS_VISTA,
>        (nsAString&) GfxDriverInfo::GetDeviceVendor(VendorNVIDIA), GfxDriverInfo::allDevices,
>        GfxDriverInfo::allFeatures, nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION,
> +      DRIVER_LESS_THAN, V(6,14,11,8265), "182.65" );

Can you cite where you saw that the version number for driver version 182.65 for Vista and 7 is in fact 6.14 (vs 8.17 or whatever on Vista and 7)? ISTR different OSes having different versions.

If this is indeed the case, can you change these entries to be: one DRIVER_OS_ALL entry with V(6,14,11,8265), and two DRIVER_OS_WINDOWS_{VISTA,7} each with DIRECT2D on V(8,17,12,5721). Alternately do as below with AMD and block Direct2D on ALL with that version.

@@ +771,5 @@
> +      DRIVER_LESS_THAN, V(8,62,0,0), "9.6" );
> +    APPEND_TO_DRIVER_BLOCKLIST( DRIVER_OS_ALL,
> +      (nsAString&) GfxDriverInfo::GetDeviceVendor(VendorATI), GfxDriverInfo::allDevices,
> +      nsIGfxInfo::FEATURE_DIRECT2D, nsIGfxInfo::FEATURE_BLOCKED_DRIVER_VERSION,
> +       DRIVER_LESS_THAN, V(8,741,0,0), "10.6" );

indentation

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Relax D3D9 driver requirements v2

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 662613 [details] [diff] [review]
Relax D3D9 driver requirements v2

Review of attachment 662613 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/GfxInfo.cpp
@@ +755,1 @@
>        DRIVER_LESS_THAN, V(8,17,12,5721), "257.21" );

Correct me if I'm wrong, but it used to be that the first matching rule determined the result. So the above allFeatures rule matching versions >= 182.65 would result in whitelisting, and this rule would never take effect. We used to have a comment about that in this file, saying that special cases should be placed before general cases, but apparently it's gone.

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Relax D3D9 driver requirements v3

Updated and comment added.

Comment 5

[Joe Drew (not getting mail)]

Benoit, we aren't sure about the versions in this patch; where did you get them originally? Do you have a reference for NVIDIA driver versions?

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Joe Drew (:JOEDREW! \o/) from comment #5)
> Benoit, we aren't sure about the versions in this patch; where did you get
> them originally?
> Do you have a reference for NVIDIA driver versions?

For NVIDIA, I don't remember at all. For ATI, there used to be a document, http://developer.amd.com/drivers/ccc/Pages/default.aspx , but it seems to be gone now.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 662656 [details] [diff] [review]
Relax D3D9 driver requirements v3

Review of attachment 662656 [details] [diff] [review]:
-----------------------------------------------------------------

I have no idea about the particular driver versions either, so that has to be cleared, but r+ on the code itself assuming the numbers are right.

Also I realized that I shouldn't have r-'d above: the case where a problem would happen was when a whitelist rule would be hidden by a more general blacklist rule; since here all the rules are blacklisting, order doesn't actually matter (the outcome is the same no matter what rule applies first).

::: widget/windows/GfxInfo.cpp
@@ +735,5 @@
>  {
>    if (!mDriverInfo->Length()) {
>      /*
> +     * It should be noted here that more restrictive rules on certain features
> +     * should be inserted -before- more generalized restriction. As the first

"restrictive rule" as opposed to "more generalized restriction" is confusing

I would oppose "special" to "general"

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/259206b2c7b2