Closed Bug 797013 Opened 7 years ago Closed 7 years ago

CheckInterfaceSupport fails on some hardware in metrofx

Categories

(Core :: Graphics: Layers, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: jimm, Assigned: bbondy)

References

(Depends on 1 open bug)

Details

(Whiteboard: [metro-mvp] [LOE:1])

Attachments

(2 files, 1 obsolete file)

We currently force accel on in metro, but failures can still crop up when we initialize the d3d layers backend. When this happens, layer manager in MetroWidget is left as null, and ultimately the browser just crashes. 

VMWare 8.0 is a good test case with the Win8 RTM. 

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxWindowsPlatform.cpp#572

More generally we need a better fall back when we can't create needed rendering resources.
I think we have to enumerate more than the first adapter, and then return once we find the first one that supports dx10.  I think we only check the first adapter currently.
Whiteboard: metro-beta
Blocks: 797201
Whiteboard: metro-beta → [metro-mvp]
Whiteboard: [metro-mvp] → [metro-mvp] [LOE:1]
Assignee: nobody → netzen
Attached patch Patch v1 (obsolete) — Splinter Review
I couldn't find a good reason why CheckInterfaceSupport was failing, but I did find that simply calling D3D10CreateDevice1 directly works.  So that function is failing with interface not supported, but it is supported.  I did find some strange notes in the documentation, but nothing specific to the problem we're seeing on Windows 8.  It only mentions DX11 interfaces, but we're using DX10 interfaces here.  Maybe it's a bug in Win8 or something.  The workaround seems to be just to try creating the object though which is what this patch does.

With this patch Metro builds work in vmware workstation 8.0.  Without it, it does not work and ends in a crash.

> Note  You can use CheckInterfaceSupport only to check whether a Direct3D 
> 10.x interface is supported, and only on Windows Vista SP1 and later 
> versions of the operating system. If you try to use CheckInterfaceSupport 
> to check whether a Direct3D 11.x and later version interface is supported, 
> CheckInterfaceSupport returns DXGI_ERROR_UNSUPPORTED. Therefore, do not use 
> CheckInterfaceSupport. Instead, to verify whether the operating system supports 
> a particular interface, try to create the interface. 

http://msdn.microsoft.com/en-us/library/windows/desktop/bb174524%28v=vs.85%29.aspx
Attachment #677273 - Flags: review?(bas)
(In reply to Brian R. Bondy [:bbondy] from comment #3)
> Created attachment 677273 [details] [diff] [review]
> Patch v1
> 
> I couldn't find a good reason why CheckInterfaceSupport was failing, but I
> did find that simply calling D3D10CreateDevice1 directly works.  So that
> function is failing with interface not supported, but it is supported.  I
> did find some strange notes in the documentation, but nothing specific to
> the problem we're seeing on Windows 8.  It only mentions DX11 interfaces,
> but we're using DX10 interfaces here.  Maybe it's a bug in Win8 or
> something.  The workaround seems to be just to try creating the object
> though which is what this patch does.
> 
> With this patch Metro builds work in vmware workstation 8.0.  Without it, it
> does not work and ends in a crash.
> 
> > Note  You can use CheckInterfaceSupport only to check whether a Direct3D 
> > 10.x interface is supported, and only on Windows Vista SP1 and later 
> > versions of the operating system. If you try to use CheckInterfaceSupport 
> > to check whether a Direct3D 11.x and later version interface is supported, 
> > CheckInterfaceSupport returns DXGI_ERROR_UNSUPPORTED. Therefore, do not use 
> > CheckInterfaceSupport. Instead, to verify whether the operating system supports 
> > a particular interface, try to create the interface. 
> 
> http://msdn.microsoft.com/en-us/library/windows/desktop/bb174524%28v=vs.
> 85%29.aspx

Startup time reasons mean we can't just take this patch. Although D3D10 hardware is getting more common we'd need to measure ts carefully first on non D3D10 Windows systems.
So I think failures are fast, but I think we should do one of 2 things here:

1) Add a pref that remembers that you have neither DX10 nor DX10.1 support. Should this ever expire like on hardware change? If so do you know the best way to detect this change?
or 2) Conditionally only do this in Metro. 

The patch may actually lead to a win for non startup reasons since acceleration will be used. But ya I see the startup concern.
Another alternative I thought of is doing #1 as per Comment 5, but reset the pref if CheckInterface actually succeeds.
Duplicate of this bug: 798375
Duplicate of this bug: 805339
I found the bug we previously talked about this and set it to block this.  Bug 725508.  I'm going to go with Comment 6 I think, unless you have objections.
Depends on: 725508
Duplicate of this bug: 797990
I'm new to nightlies and would like to help out. I can't get the 'Metro' version to load here at all.

What logs / info can I send to help you lot?

MaFt
Thanks for the offer but the problem is already identified. I'm actually working on a new patch for this now and this will likely land this week.
Attached patch Patch v2Splinter Review
So this patch is similar to the last one, but it doesn't have the potential startup perf hit. 

In particular it will only ever try to create the D10 device once in failure conditions. If that fails we will never check again... that is unless CheckInterfaceSupport returns true. Then we'll check again.
Attachment #677273 - Attachment is obsolete: true
Attachment #677273 - Flags: review?(bas)
Attachment #678977 - Flags: review?(bas)
Comment on attachment 678977 [details] [diff] [review]
Patch v2

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

Hrm, I wonder if we can also have this pref reset if the adapter/driver changes :s. I'm a little worried someone on a device where this fails. But maybe that group is too small to really worry about.
I'm not sure how to detect that case, but if they have a device that always fails the CheckInterfaceSupport call, then they are in the same boat as before this patch.  So this patch doesn't make it worse.  It just opens the door to more devices.

Maybe we could store the last "adapter description" as seen in about:support in registry and then if it is different from what exists currently we would reset?
Maybe ditto adapter drivers.
If you agree that this patch makes it no worse, could we do that in another bug for both adapter description and adapter drivers? I can still do it this week but a lot of people are waiting on this fix.
Comment on attachment 678977 [details] [diff] [review]
Patch v2

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

Hrm, I wonder if we can also have this pref reset if the adapter/driver changes :s. I'm a little worried someone on a device where this fails. But maybe that group is too small to really worry about.

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +565,5 @@
>                // They won't get acceleration.
>                return;
>              }
> +
> +            bool checkDX10 = 

nit: whitespace
Attachment #678977 - Flags: review?(bas) → review+
Depends on: 810176
https://hg.mozilla.org/mozilla-central/rev/07eea74cea2b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 810549
No longer depends on: 810176
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.