1149728 - Upstream 'Treat ANGLE default FB as RGB(A)'

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

We should upstream this. Not doing so makes updating ANGLE difficult. Jeff can you do this?

Comment 1

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)
> We should upstream this. Not doing so makes updating ANGLE difficult. Jeff
> can you do this?

Yes.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Jeff, do you have an idea of when you'll get to this?

Comment 3

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> Jeff, do you have an idea of when you'll get to this?

This coming week.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Rebase ping

Comment 5

[Kelsey Gilbert [:jgilbert]]

Is this problem still present in the rebase?
I've heard ANGLE's gone through a bunch of changes, so this may no longer be necessary.

If a standard antialiased scene renders fine without this change, we don't need it.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #5)
> Is this problem still present in the rebase?
> I've heard ANGLE's gone through a bunch of changes, so this may no longer be
> necessary.
> 
> If a standard antialiased scene renders fine without this change, we don't
> need it.

From the quick testing that I did with the rebase, it appears we may still need this.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Yeah, it still looks to be needed. Turning on antialiasing stops the drawing with my ANGLE update without this patch. Any idea when you'll get a chance to look at this?

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Ping? If you're not going to get a chance to look at this I can try.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

With jgilbert's current patch I get assertion failure in BlitTextureToTexture trying to blit a zero sized surface that we think is 200x200

Comment 10

[Kelsey Gilbert [:jgilbert]]

With my current changes, it seems to work both for d3d11 and d3d9. (d3d9 was failing to composite RGBA TextureClients) I'm sending it up to try.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Gilbert [:jgilbert] from comment #10)
> With my current changes, it seems to work both for d3d11 and d3d9. (d3d9 was
> failing to composite RGBA TextureClients) I'm sending it up to try.

Link?

Comment 12

[Jeff Muizelaar [:jrmuizel]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=25dbd7fe151e

Comment 13

[Jeff Muizelaar [:jrmuizel]]

It's worth noting that 25dbd7fe151e doesn't contain all of the ANGLE changes from upstream.

Part of this is because it seems based on:
https://github.com/jrmuizel/angle/tree/minimal-changes-on-chrome-2466
instead of
https://github.com/jrmuizel/angle/tree/changes-on-chrome-2466

Comment 14

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #13)
> It's worth noting that 25dbd7fe151e doesn't contain all of the ANGLE changes
> from upstream.
> 
> Part of this is because it seems based on:
> https://github.com/jrmuizel/angle/tree/minimal-changes-on-chrome-2466
> instead of
> https://github.com/jrmuizel/angle/tree/changes-on-chrome-2466

Even https://github.com/jrmuizel/angle/tree/changes-on-chrome-2466 doesn't have all of the our changes from https://github.com/mozilla/angle

Comment 15

[Kelsey Gilbert [:jgilbert]]

Misc notes:
ANGLEr2466 updates its "ANGLE_platform_angle[_d3d]" EGL extensions to include the EGL_ prefix. Updating to r2466 means adding the EGL_ prefix to the extension strings in GLLibraryEGL.cpp. (Skipping this causes GLContext to return false for IsANGLE)

r2466 changes the enum value for LOCAL_EGL_DXGI_KEYED_MUTEX_ANGLE from 0x3209 to 0x33A2. (Omitting this caused compositing to fail) (LOCAL_EGL_D3D_TEXTURE_2D_SHARE_HANDLE_ANGLE remains the same)

Comment 16

[Kelsey Gilbert [:jgilbert]]

https://github.com/jdashg/gecko-dev/tree/changes-on-chrome-2466

I believe that this branch satisfies these requirements, though I went about it the other way around. I made us use BGRA formats on ANGLE.

I'm hitting EGL_BAD_MATCHes that should be fixed by bug 1191042.

Comment 17

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Gecko-changes.patch

Comment 18

[Kelsey Gilbert [:jgilbert]]

Attachment: 0003-Move-CreateXForOffscreen-functions.patch

Comment 19

[Kelsey Gilbert [:jgilbert]]

Attachment: 0004-Use-BGRA-on-ANGLE.patch

Comment 20

[Jeff Muizelaar [:jrmuizel]]

What is the status of these patches?

Comment 21

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #20)
> What is the status of these patches?

It's hard to tell without bug 1191042. They seem to work locally, and conceptually they are a solid direction. I have not run through the conformance suite with them locally, and the reftest suite does not complete due to EGL_BAD_MATCH issues.

I'm reviving my work on bug 1191042, which I had assumed landed, but was not clearly marked as backed-out.

Comment 22

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8657354 [details] [diff] [review]
0004-Use-BGRA-on-ANGLE.patch

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

This seems to work great.

Comment 23

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Add-assert-for-keyed-mutex.patch

Comment 24

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Move-CreateXForOffscreen-functions.patch

r=jrmuizel

Comment 25

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8666199 [details] [diff] [review]
0001-Add-assert-for-keyed-mutex.patch

Eh, we don't need review for adding this assert.

Comment 26

[Kelsey Gilbert [:jgilbert]]

Attachment: 0003-Use-BGRA-on-ANGLE.patch

r=jrmuizel

Comment 27

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8666199 [details] [diff] [review]
0001-Add-assert-for-keyed-mutex.patch

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

I believe this is wrong. We don't get keyed mutexes on D3D9 ANGLE

Comment 28

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f99fe06748c3

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f99fe06748c3