1083902 - Limit draw buffer size to 4K

Status: NEW

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Limit draw buffer size to 4K

Chrome does this (It's good for us to match them to minimize risk of only us running into problems) and it will let us avoid bug 1079400

Comment 1

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8506256 [details] [diff] [review]
Limit draw buffer size to 4K

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

No, let's just try to fix the bug before jumping to limit texture size.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #1)
> Comment on attachment 8506256 [details] [diff] [review]
> Limit draw buffer size to 4K
> 
> Review of attachment 8506256 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> No, let's just try to fix the bug before jumping to limit texture size.

It's a driver crash. The best we can do is limit the texture size more specifically. I don't think it's worth exposing ourselves to these problem when Chrome's not. Can you justify the additional risk?

Comment 3

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> (In reply to Jeff Gilbert [:jgilbert] from comment #1)
> > Comment on attachment 8506256 [details] [diff] [review]
> > Limit draw buffer size to 4K
> > 
> > Review of attachment 8506256 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > No, let's just try to fix the bug before jumping to limit texture size.
> 
> It's a driver crash. The best we can do is limit the texture size more
> specifically. I don't think it's worth exposing ourselves to these problem
> when Chrome's not. Can you justify the additional risk?

Yes, because apps need more than 4k. Relevantly, Apple just release a 5k machine.
I've done a bunch of debugging in the past to keep this working, and while it's non-zero work, it does have benefit. I need a lot more detail about this driver bug before we should completely give up on it. It used to work fine. What changed?

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #3)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> > (In reply to Jeff Gilbert [:jgilbert] from comment #1)
> > > Comment on attachment 8506256 [details] [diff] [review]
> > > Limit draw buffer size to 4K
> > > 
> > > Review of attachment 8506256 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > No, let's just try to fix the bug before jumping to limit texture size.
> > 
> > It's a driver crash. The best we can do is limit the texture size more
> > specifically. I don't think it's worth exposing ourselves to these problem
> > when Chrome's not. Can you justify the additional risk?
> 
> Yes, because apps need more than 4k. Relevantly, Apple just release a 5k
> machine.

8k would probably work fine too...

> I've done a bunch of debugging in the past to keep this working, and while
> it's non-zero work, it does have benefit. I need a lot more detail about
> this driver bug before we should completely give up on it. It used to work
> fine. What changed?

The Nvidia D3D11 driver does a null dereference when you create a depth buffer of size MAX_TEXTURE_SIZE-1 with 4x multisampling. Using D3D11 is what is different from before.

Comment 5

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> (In reply to Jeff Gilbert [:jgilbert] from comment #3)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> > > (In reply to Jeff Gilbert [:jgilbert] from comment #1)
> > > > Comment on attachment 8506256 [details] [diff] [review]
> > > > Limit draw buffer size to 4K
> > > > 
> > > > Review of attachment 8506256 [details] [diff] [review]:
> > > > -----------------------------------------------------------------
> > > > 
> > > > No, let's just try to fix the bug before jumping to limit texture size.
> > > 
> > > It's a driver crash. The best we can do is limit the texture size more
> > > specifically. I don't think it's worth exposing ourselves to these problem
> > > when Chrome's not. Can you justify the additional risk?
> > 
> > Yes, because apps need more than 4k. Relevantly, Apple just release a 5k
> > machine.
> 
> 8k would probably work fine too...
> 
> > I've done a bunch of debugging in the past to keep this working, and while
> > it's non-zero work, it does have benefit. I need a lot more detail about
> > this driver bug before we should completely give up on it. It used to work
> > fine. What changed?
> 
> The Nvidia D3D11 driver does a null dereference when you create a depth
> buffer of size MAX_TEXTURE_SIZE-1 with 4x multisampling. Using D3D11 is what
> is different from before.

What's the biggest size we don't get this problem for?

Comment 6

[Jeff Muizelaar [:jrmuizel]]

It starts crashing at 10817x10817

Comment 7

[Jeff Muizelaar [:jrmuizel]]

but 16384x1 works fine. I expect the crashing point has to do with available graphics memory.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

What would you suggest as the way to solve this?

Comment 9

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> What would you suggest as the way to solve this?

Do a driver-specific workaround like usual.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #9)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> > What would you suggest as the way to solve this?
> 
> Do a driver-specific workaround like usual.

Is limiting it to all NVIDIA drivers sufficiently specific?

Comment 11

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> (In reply to Jeff Gilbert [:jgilbert] from comment #9)
> > (In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> > > What would you suggest as the way to solve this?
> > 
> > Do a driver-specific workaround like usual.
> 
> Is limiting it to all NVIDIA drivers sufficiently specific?

I suppose. (All ANGLE+NVIDIA) Eventually we should revisit this.


We may be running afoul of resource size limits. D3D11 only requires support for texture resources up to Clamp(dedicated_vram/4, 128MB, 2048MB). A 4-sample RGBA 10kx10k texture is 1,600MB.

Have you tried creating a debug context, and checking for errors? It may point us towards what we need to query for the limits.

It sounds like this should be an ANGLE driver workaround, where it should just return OOM for this driver for such large resources. I can write this patch.

Comment 12

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #11)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #10)
> > (In reply to Jeff Gilbert [:jgilbert] from comment #9)
> > > (In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> > > > What would you suggest as the way to solve this?
> > > 
> > > Do a driver-specific workaround like usual.
> > 
> > Is limiting it to all NVIDIA drivers sufficiently specific?
> 
> I suppose. (All ANGLE+NVIDIA) Eventually we should revisit this.
> 
> 
> We may be running afoul of resource size limits. D3D11 only requires support
> for texture resources up to Clamp(dedicated_vram/4, 128MB, 2048MB). A
> 4-sample RGBA 10kx10k texture is 1,600MB.

Yes, I think that may be what's happening.

> Have you tried creating a debug context, and checking for errors? It may
> point us towards what we need to query for the limits.

Creating a debug device still crashes and doesn't give any additional information about the problem.

> It sounds like this should be an ANGLE driver workaround, where it should
> just return OOM for this driver for such large resources. I can write this
> patch.

That would be great.

Comment 13

[Kelsey Gilbert [:jgilbert]]

https://code.msdn.microsoft.com/windowsdesktop/DirectX-Video-Memory-ee7d8319