Closed Bug 792190 Opened 10 years ago Closed 10 years ago

Relax D3D9 driver version requirements


(Core :: Graphics, defect)

Windows 7
Not set





(Reporter: bas.schouten, Assigned: bas.schouten)



(1 file, 2 obsolete files)

Attached patch Relax D3D9 driver requirements (obsolete) — Splinter Review
We agreed to try and relax the D3D9 driver version requirements. This patch does just that, let's see how it goes.
Attachment #662279 - Flags: review?(joe)
Attachment #662279 - Attachment is patch: true
Comment on attachment 662279 [details] [diff] [review]
Relax D3D9 driver requirements

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

::: widget/windows/GfxInfo.cpp
@@ +744,4 @@
>        (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" );
> +      (nsAString&) GfxDriverInfo::GetDeviceVendor(VendorATI), GfxDriverInfo::allDevices,
> +       DRIVER_LESS_THAN, V(8,741,0,0), "10.6" );

Attachment #662279 - Flags: review?(joe) → review+
Attachment #662279 - Attachment is obsolete: true
Attachment #662613 - Flags: review?(bjacob)
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.
Attachment #662613 - Flags: review?(bjacob) → review-
Updated and comment added.
Attachment #662613 - Attachment is obsolete: true
Attachment #662656 - Flags: review?(bjacob)
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?
(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, , but it seems to be gone now.
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"
Attachment #662656 - Flags: review?(bjacob) → review+
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.