Relax D3D9 driver version requirements

RESOLVED FIXED in mozilla18

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

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

Tracking

unspecified
mozilla18
x86_64
Windows 7
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

7 years ago
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 @@
>      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
Attachment #662279 - Flags: review?(joe) → review+
Assignee

Comment 2

7 years ago
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-
Assignee

Comment 4

7 years ago
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, http://developer.amd.com/drivers/ccc/Pages/default.aspx , 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+
https://hg.mozilla.org/mozilla-central/rev/259206b2c7b2
Status: ASSIGNED → RESOLVED
Closed: 7 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.