AMD's NV_DX_interop2 binds the interop device to the current GL context

RESOLVED FIXED in Firefox 51

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

(Depends on: 2 bugs)

50 Branch
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 affected, firefox51 fixed)

Details

Attachments

(10 attachments)

58 bytes, text/x-review-board-request
Details
58 bytes, text/x-review-board-request
mtseng
: review+
Details
58 bytes, text/x-review-board-request
milan
: review+
Details
58 bytes, text/x-review-board-request
mtseng
: review+
Details
58 bytes, text/x-review-board-request
mtseng
: review+
Details
58 bytes, text/x-review-board-request
mtseng
: review+
Details
58 bytes, text/x-review-board-request
jrmuizel
: review+
Details
58 bytes, text/x-review-board-request
mattwoodrow
: review+
Details
58 bytes, text/x-review-board-request
milan
: review+
Details
58 bytes, text/x-review-board-request
milan
: review+
Details
(Assignee)

Description

2 years ago
Surprise!
NV doesn't, but AMD does. So let's add some MakeCurrents.
(Assignee)

Comment 1

2 years ago
Created attachment 8772239 [details]
Bug 1287653 - Strip EOL whitespace from gfx/thebes/gfxWindowsPlatform.cpp.

Review commit: https://reviewboard.mozilla.org/r/65096/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65096/
Attachment #8772240 - Flags: review?(mtseng)
Attachment #8772241 - Flags: review?(jmuizelaar)
Attachment #8772242 - Flags: review?(mtseng)
Attachment #8772243 - Flags: review?(mtseng)
Attachment #8772244 - Flags: review?(mtseng)
Attachment #8772245 - Flags: review?(jmuizelaar)
Attachment #8772246 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

2 years ago
Created attachment 8772240 [details]
Bug 1287653 - Use std::string::c_str instead of &std::string[0] to get a c-string. -

Review commit: https://reviewboard.mozilla.org/r/65098/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65098/
(Assignee)

Comment 3

2 years ago
Created attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

Review commit: https://reviewboard.mozilla.org/r/65100/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65100/
(Assignee)

Comment 4

2 years ago
Created attachment 8772242 [details]
Bug 1287653 - AMD associates DXOpenDevice with the current GL context. -

Review commit: https://reviewboard.mozilla.org/r/65102/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65102/
(Assignee)

Comment 7

2 years ago
Created attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

Review commit: https://reviewboard.mozilla.org/r/65108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65108/
(Assignee)

Comment 9

2 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #7)
> Created attachment 8772245 [details]
> Bug 1287653 - Use our own D3D11Device. -
> 
> Review commit: https://reviewboard.mozilla.org/r/65108/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/65108/

So I tried using the content d3d device, but that crashes in the AMD driver. (race condition? it crashes reliably on my machine) Maybe I should try the imagebridge device? Or create a new device? Maybe the content device would be OK with the multithreading protection that I heard about in another bug turned on?
Flags: needinfo?(jmuizelaar)
(In reply to Jeff Gilbert [:jgilbert] from comment #9)
> (In reply to Jeff Gilbert [:jgilbert] from comment #7)
> > Created attachment 8772245 [details]
> > Bug 1287653 - Use our own D3D11Device. -
> > 
> > Review commit: https://reviewboard.mozilla.org/r/65108/diff/#index_header
> > See other reviews: https://reviewboard.mozilla.org/r/65108/
> 
> So I tried using the content d3d device, but that crashes in the AMD driver.
> (race condition? it crashes reliably on my machine) Maybe I should try the
> imagebridge device? Or create a new device? Maybe the content device would
> be OK with the multithreading protection that I heard about in another bug
> turned on?

AMD has discouraged us from using more than one device per process so I'm not in a rush to use an additional device. If you can reproduce the crash then it should be easy to have AMD do the same and then give us some guidance on what the correct way forward should be. I've cc'd Paul to help out.
Flags: needinfo?(jmuizelaar)
(Assignee)

Comment 11

2 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> (In reply to Jeff Gilbert [:jgilbert] from comment #9)
> > (In reply to Jeff Gilbert [:jgilbert] from comment #7)
> > > Created attachment 8772245 [details]
> > > Bug 1287653 - Use our own D3D11Device. -
> > > 
> > > Review commit: https://reviewboard.mozilla.org/r/65108/diff/#index_header
> > > See other reviews: https://reviewboard.mozilla.org/r/65108/
> > 
> > So I tried using the content d3d device, but that crashes in the AMD driver.
> > (race condition? it crashes reliably on my machine) Maybe I should try the
> > imagebridge device? Or create a new device? Maybe the content device would
> > be OK with the multithreading protection that I heard about in another bug
> > turned on?
> 
> AMD has discouraged us from using more than one device per process so I'm
> not in a rush to use an additional device. If you can reproduce the crash
> then it should be easy to have AMD do the same and then give us some
> guidance on what the correct way forward should be. I've cc'd Paul to help
> out.

Alright. I'll make a build tomorrow.
Comment on attachment 8772240 [details]
Bug 1287653 - Use std::string::c_str instead of &std::string[0] to get a c-string. -

https://reviewboard.mozilla.org/r/65098/#review62196
Attachment #8772240 - Flags: review?(mtseng) → review+
Attachment #8772243 - Flags: review?(mtseng) → review+
(Assignee)

Comment 15

2 years ago
Build:
https://archive.mozilla.org/pub/firefox/try-builds/jgilbert@mozilla.com-9be245750f57c8463e962a9f1f07a8c8f0bb4d9f/try-win64-debug/firefox-50.0a1.en-US.win64.zip

Crashing page:
https://www.khronos.org/registry/webgl/sdk/tests/conformance/attribs/gl-disabled-vertex-attrib.html?webglVersion=2&quiet=0

More info:
Crashed at 00007FFADA44E3C8
> >        d3d11.dll!CDevice::ObjectLock::ObjectLock()        Unknown 
>          d3d11.dll!00007FFADA4755BD()!CDevice::ForceContinuableExceptions(int)        Unknown
>          atidxx64.dll!00007ffad8824c98()        Unknown
>          atidxx64.dll!00007ffad8824a8e()        Unknown
>          atidxx64.dll!00007ffad88248aa()        Unknown
>          atio6axx.dll!00000000651c6f99()        Unknown
>          atio6axx.dll!00000000651c81ba()        Unknown
>          atio6axx.dll!00000000651c78a1()        Unknown
>          atio6axx.dll!00000000651c68c2()        Unknown
>          atio6axx.dll!00000000651b6bd2()        Unknown
>          xul.dll!mozilla::gl::DXInterop2Device::LockObject(void * lockHandle) Line 208        C++
>          xul.dll!mozilla::gl::SharedSurface_D3D11Interop::ProducerAcquireImpl() Line 353        C++
>          xul.dll!mozilla::gl::SharedSurface::ProducerAcquire() Line 114        C++
>          xul.dll!mozilla::gl::GLScreenBuffer::Swap(const mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> & size) Line 570        C++
>          xul.dll!mozilla::gl::GLScreenBuffer::PublishFrame(const mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> & size) Line 621        C++
>          xul.dll!mozilla::WebGLContext::PresentScreenBuffer() Line 1576        C++

> 00007FFADA44E3BC  mov         rcx,rbp  
> 00007FFADA44E3BF  call        qword ptr [__guard_check_icall_fptr (07FFADA58E708h)]  
> 00007FFADA44E3C5  mov         rcx,rdi  
> >00007FFADA44E3C8  call        rbp  

> 00007FFADA4755B8  call        CDevice::ObjectLock::ObjectLock (07FFADA3E24E8h)  
> >00007FFADA4755BD  test        ebx,ebx
Flags: needinfo?(paul.blinzer)
(Assignee)

Comment 16

2 years ago
Using the ImageBridge d3d device seems to work...
(Assignee)

Comment 17

2 years ago
Comment on attachment 8772239 [details]
Bug 1287653 - Strip EOL whitespace from gfx/thebes/gfxWindowsPlatform.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65096/diff/1-2/
Attachment #8772245 - Attachment description: Bug 1287653 - Use our own D3D11Device. - → Bug 1287653 - Use ImageBridge d3d device. -
(Assignee)

Comment 18

2 years ago
Comment on attachment 8772240 [details]
Bug 1287653 - Use std::string::c_str instead of &std::string[0] to get a c-string. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65098/diff/1-2/
(Assignee)

Comment 19

2 years ago
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65100/diff/1-2/
(Assignee)

Comment 20

2 years ago
Comment on attachment 8772242 [details]
Bug 1287653 - AMD associates DXOpenDevice with the current GL context. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65102/diff/1-2/
(Assignee)

Comment 21

2 years ago
Comment on attachment 8772243 [details]
Bug 1287653 - Cleanup GLContextProviderWGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65104/diff/1-2/
(Assignee)

Comment 22

2 years ago
Comment on attachment 8772244 [details]
Bug 1287653 - Remove context sharing from WGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65106/diff/1-2/
(Assignee)

Comment 23

2 years ago
Comment on attachment 8772246 [details]
Bug 1287653 - Minor fixes in D3D11Checks. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65110/diff/1-2/
(Assignee)

Comment 24

2 years ago
Comment on attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65108/diff/1-2/

Comment 25

2 years ago
Thanks for the stacktrace. We have started investigation.
Flags: needinfo?(paul.blinzer)
Comment on attachment 8772242 [details]
Bug 1287653 - AMD associates DXOpenDevice with the current GL context. -

https://reviewboard.mozilla.org/r/65102/#review62444
Attachment #8772242 - Flags: review?(mtseng) → review+
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

https://reviewboard.mozilla.org/r/65100/#review63030

This commit message doesn't say why.
Attachment #8772241 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 30

2 years ago
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

It cleans up this code, making it easier to read and to edit for testing.
Attachment #8772241 - Flags: review- → review?(jmuizelaar)
(Assignee)

Comment 31

2 years ago
https://reviewboard.mozilla.org/r/65110/#review63032

It should be moved here because this is how clients of this code (particularly WebGL) use it. As-is, it (though unlikely) could allow through an implementation that requires the acquire be maintained throughout the Map+memcpy. This isn't what the API requires, so we shouldn't test this. In the absence of comments to the contrary, this seems like a bug in the original code here.
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

https://reviewboard.mozilla.org/r/65100/#review63044

Improve the commit message.
Attachment #8772241 - Flags: review?(jmuizelaar) → review+
Attachment #8772246 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8772246 [details]
Bug 1287653 - Minor fixes in D3D11Checks. -

https://reviewboard.mozilla.org/r/65110/#review63048

Please add the comment about why the ReleaseSync should come when it does.
(Assignee)

Comment 35

2 years ago
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65100/diff/2-3/
Attachment #8772241 - Attachment description: Bug 1287653 - Pull out d3d11 creation flags into local. - → Bug 1287653 - Pull out d3d11 creation flags into local for better readability and editability. -
(Assignee)

Comment 36

2 years ago
Comment on attachment 8772242 [details]
Bug 1287653 - AMD associates DXOpenDevice with the current GL context. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65102/diff/2-3/
(Assignee)

Comment 37

2 years ago
Comment on attachment 8772243 [details]
Bug 1287653 - Cleanup GLContextProviderWGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65104/diff/2-3/
(Assignee)

Comment 38

2 years ago
Comment on attachment 8772244 [details]
Bug 1287653 - Remove context sharing from WGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65106/diff/2-3/
(Assignee)

Comment 39

2 years ago
Comment on attachment 8772246 [details]
Bug 1287653 - Minor fixes in D3D11Checks. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65110/diff/2-3/
(Assignee)

Comment 40

2 years ago
Comment on attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65108/diff/2-3/
https://reviewboard.mozilla.org/r/65108/#review63080

How do we know that we won't get crashes when using the ImageBridge device?
(Assignee)

Comment 42

2 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #41)
> https://reviewboard.mozilla.org/r/65108/#review63080
> 
> How do we know that we won't get crashes when using the ImageBridge device?

Brief empirical testing.
It's either that or creating our own device.
(In reply to Jeff Gilbert [:jgilbert] from comment #42)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #41)
> > https://reviewboard.mozilla.org/r/65108/#review63080
> > 
> > How do we know that we won't get crashes when using the ImageBridge device?
> 
> Brief empirical testing.
> It's either that or creating our own device.

I don't think brief empirical testing is sufficient. I'm not in a rush to switch from easy reproduce instability to hard to reproduce instability. I'd like to hear what AMD suggests we do.
(Assignee)

Comment 44

2 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #43)
> (In reply to Jeff Gilbert [:jgilbert] from comment #42)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #41)
> > > https://reviewboard.mozilla.org/r/65108/#review63080
> > > 
> > > How do we know that we won't get crashes when using the ImageBridge device?
> > 
> > Brief empirical testing.
> > It's either that or creating our own device.
> 
> I don't think brief empirical testing is sufficient. I'm not in a rush to
> switch from easy reproduce instability to hard to reproduce instability. I'd
> like to hear what AMD suggests we do.

We have a reproducing build, and no timeframe from AMD. Either we take this or we block NV_dx_interop2 on AMD. If this is not resolved by 50's branch date, we obviously block the extension. However, we're running out of time to test this on Nightly. Let's take this for now, and we can watch for crashes. The crash-stacks are really obvious, so it will not be hard.
Flags: needinfo?(jmuizelaar)
Comment on attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

I'm going to defer this decision to mattwoodrow
Flags: needinfo?(jmuizelaar)
Attachment #8772245 - Flags: review?(jmuizelaar) → review?(matt.woodrow)
(In reply to Jeff Gilbert [:jgilbert] from comment #44)

> We have a reproducing build, and no timeframe from AMD. Either we take this
> or we block NV_dx_interop2 on AMD. If this is not resolved by 50's branch
> date, we obviously block the extension. However, we're running out of time
> to test this on Nightly. Let's take this for now, and we can watch for
> crashes. The crash-stacks are really obvious, so it will not be hard.

The primary concern is that the crashes we might get won't be this well understood one, but instead subtle race condition ones relating to having multiple devices. Those will be much harder to track (as we have found in the past).

How high of a priority is it to ship this extension? How many users do we expect to be using it?

I'm just about to land a patch to remove the ImageBridge device, so adding a new user really makes me sad.

Could you try emailing Paul directly and see if he responds? Having a real fix sounds much preferable than a workaround has known downsides.

If we do this, then I think we need to create a device on demand so that only users that are getting this extension have to take the risk of extra devices.

Comment 47

2 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #46)
> (In reply to Jeff Gilbert [:jgilbert] from comment #44)
> 
> > We have a reproducing build, and no timeframe from AMD. Either we take this
> > or we block NV_dx_interop2 on AMD. If this is not resolved by 50's branch
> > date, we obviously block the extension. However, we're running out of time
> > to test this on Nightly. Let's take this for now, and we can watch for
> > crashes. The crash-stacks are really obvious, so it will not be hard.
> 
> The primary concern is that the crashes we might get won't be this well
> understood one, but instead subtle race condition ones relating to having
> multiple devices. Those will be much harder to track (as we have found in
> the past).
> 
> How high of a priority is it to ship this extension? How many users do we
> expect to be using it?
> 
> I'm just about to land a patch to remove the ImageBridge device, so adding a
> new user really makes me sad.
> 
> Could you try emailing Paul directly and see if he responds? Having a real
> fix sounds much preferable than a workaround has known downsides.
> 
> If we do this, then I think we need to create a device on demand so that
> only users that are getting this extension have to take the risk of extra
> devices.

Matt, Jeff, I am monitoring this ticket but since I have no substantial new info, I have not commented yet. We are working on resolving this issue. I can't state a particular driver version yet when this issue is addressed, all I can say it will likely a little after build 50 branches. Will update the thread when I can provide more information.
(In reply to Paul Blinzer from comment #47)
> 
> Matt, Jeff, I am monitoring this ticket but since I have no substantial new
> info, I have not commented yet. We are working on resolving this issue. I
> can't state a particular driver version yet when this issue is addressed,
> all I can say it will likely a little after build 50 branches. Will update
> the thread when I can provide more information.

Oh awesome, thanks for that Paul.

Jeff, is waiting for a driver release and then restricting this extension to users with those drivers an acceptable solution, or do we really want this enough to push for the workaround?
(Assignee)

Comment 49

2 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #48)
> (In reply to Paul Blinzer from comment #47)
> > 
> > Matt, Jeff, I am monitoring this ticket but since I have no substantial new
> > info, I have not commented yet. We are working on resolving this issue. I
> > can't state a particular driver version yet when this issue is addressed,
> > all I can say it will likely a little after build 50 branches. Will update
> > the thread when I can provide more information.
> 
> Oh awesome, thanks for that Paul.
> 
> Jeff, is waiting for a driver release and then restricting this extension to
> users with those drivers an acceptable solution, or do we really want this
> enough to push for the workaround?

I would like to try having a separate device if there's no clear reason this should incur crashes. The separate device has a very narrow usecase.
I need to add a blacklist entry for dx_interop2 so that we can turn it off if things go south.

I'm also willing to do this in the opposite order:
* dx_interop2 blacklist (since we need this anyway)
* add separate device as workaround, created only when needed. (webgl2 only, basically)
That seems reasonable to me.
(Assignee)

Comment 51

2 years ago
Created attachment 8775845 [details]
Bug 1287653 - Blacklist dx_interop2. -

Review commit: https://reviewboard.mozilla.org/r/67894/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67894/
Attachment #8772246 - Attachment description: Bug 1287653 - Minor fixes to DoesD3D11TextureSharingWorkInternal. - → Bug 1287653 - Minor fixes in D3D11Checks. -
Attachment #8772241 - Attachment description: Bug 1287653 - Pull out d3d11 creation flags into local for better readability and editability. - → Bug 1287653 - Add blacklist entry for dx_interop2. -
Attachment #8772245 - Attachment description: Bug 1287653 - Use ImageBridge d3d device. - → Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.
Attachment #8775845 - Flags: review?(milan)
Attachment #8772246 - Flags: review+ → review?(matt.woodrow)
Attachment #8772241 - Flags: review+ → review?(milan)
Attachment #8772245 - Flags: review?(matt.woodrow) → review?(jmuizelaar)
(Assignee)

Comment 52

2 years ago
Comment on attachment 8772239 [details]
Bug 1287653 - Strip EOL whitespace from gfx/thebes/gfxWindowsPlatform.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65096/diff/2-3/
(Assignee)

Comment 53

2 years ago
Comment on attachment 8772240 [details]
Bug 1287653 - Use std::string::c_str instead of &std::string[0] to get a c-string. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65098/diff/2-3/
(Assignee)

Comment 54

2 years ago
Comment on attachment 8772242 [details]
Bug 1287653 - AMD associates DXOpenDevice with the current GL context. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65102/diff/3-4/
(Assignee)

Comment 55

2 years ago
Comment on attachment 8772243 [details]
Bug 1287653 - Cleanup GLContextProviderWGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65104/diff/3-4/
(Assignee)

Comment 56

2 years ago
Comment on attachment 8772244 [details]
Bug 1287653 - Remove context sharing from WGL. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65106/diff/3-4/
(Assignee)

Comment 57

2 years ago
Comment on attachment 8772246 [details]
Bug 1287653 - Minor fixes in D3D11Checks. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65110/diff/3-4/
(Assignee)

Comment 58

2 years ago
Comment on attachment 8772241 [details]
Bug 1287653 - Add blacklist entry for dx_interop2. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65100/diff/3-4/
(Assignee)

Comment 59

2 years ago
Comment on attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65108/diff/3-4/
Comment on attachment 8772245 [details]
Bug 1287653 - Strip EOL whitespace from /widget/nsIGfxInfo.idl.

https://reviewboard.mozilla.org/r/65108/#review65074
Attachment #8772245 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 61

2 years ago
Created attachment 8776687 [details]
Bug 1287653 - Add feature to GetPrefNameForFeature. -

Review commit: https://reviewboard.mozilla.org/r/68400/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68400/
Attachment #8776687 - Flags: review?(milan)
Comment on attachment 8775845 [details]
Bug 1287653 - Blacklist dx_interop2. -

https://reviewboard.mozilla.org/r/67894/#review65538

::: gfx/thebes/gfxUtils.cpp:1443
(Diff revision 1)
> +
> +  int32_t status;
> +  if (!NS_SUCCEEDED(gfxUtils::ThreadSafeGetFeatureStatus(gfxInfo, feature,
> +                                                         *out_blacklistId, &status)))
> +  {
> +    out_blacklistId->AssignLiteral("");

Is this meant to be an empty string?

::: modules/libpref/init/all.js:4402
(Diff revision 1)
>  #endif
>  pref("gl.require-hardware", false);
>  #ifdef XP_MACOSX
>  pref("gl.multithreaded", true);
>  #endif
> +pref("gl.ignore-dx-interop2-blacklist", false);

Do we really want to show this in about:config by default?
Attachment #8775845 - Flags: review?(milan) → review+
(Assignee)

Comment 68

2 years ago
(In reply to Milan Sreckovic [:milan] (PTO July 28-29) from comment #62)
> Comment on attachment 8775845 [details]
> Bug 1287653 - Blacklist dx_interop2. -
> 
> https://reviewboard.mozilla.org/r/67894/#review65538
> 
> ::: gfx/thebes/gfxUtils.cpp:1443
> (Diff revision 1)
> > +
> > +  int32_t status;
> > +  if (!NS_SUCCEEDED(gfxUtils::ThreadSafeGetFeatureStatus(gfxInfo, feature,
> > +                                                         *out_blacklistId, &status)))
> > +  {
> > +    out_blacklistId->AssignLiteral("");
> 
> Is this meant to be an empty string?
Yep, because we couldn't find any blacklist entry.
> 
> ::: modules/libpref/init/all.js:4402
> (Diff revision 1)
> >  #endif
> >  pref("gl.require-hardware", false);
> >  #ifdef XP_MACOSX
> >  pref("gl.multithreaded", true);
> >  #endif
> > +pref("gl.ignore-dx-interop2-blacklist", false);
> 
> Do we really want to show this in about:config by default?
Yes, we want an easy way to re-test this.
(Assignee)

Updated

2 years ago
Depends on: 1291582

Comment 69

2 years ago
mozreview-review
Comment on attachment 8772246 [details]
Bug 1287653 - Minor fixes in D3D11Checks. -

https://reviewboard.mozilla.org/r/65110/#review67096
Attachment #8772246 - Flags: review?(matt.woodrow) → review+
(Assignee)

Updated

2 years ago
Depends on: 1290989

Comment 70

2 years ago
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/066c58cd308f
Strip EOL whitespace from gfx/thebes/gfxWindowsPlatform.cpp.
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd3a281cf9a9
Use std::string::c_str instead of &std::string[0] to get a c-string. - r=mtseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd397f74a2fc
AMD associates DXOpenDevice with the current GL context. - r=mtseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5af6964c802
Cleanup GLContextProviderWGL. - r=mtseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/749f1c8bf7b9
Remove context sharing from WGL. - r=mtseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/e39ccca212f0
Minor fixes in D3D11Checks. - r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ea9b966bda7
Add blacklist entry for dx_interop2. - r=milan
https://hg.mozilla.org/integration/mozilla-inbound/rev/c35e56294f2b
Strip EOL whitespace from /widget/nsIGfxInfo.idl.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3167ef50f1ff
Blacklist dx_interop2. - r=milan
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc145173aba7
Add feature to GetPrefNameForFeature. - r=milan
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e665764ca14
Spew VENDOR/RENDERER. - r=mtseng
Depends on: 1296651
Depends on: 1296644
No longer depends on: 1296996
(Assignee)

Updated

2 years ago
Blocks: 1345648
You need to log in before you can comment on or make changes to this bug.