1520953 - WR should use ANGLE_provoking_vertex on D3D.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Kelsey Gilbert [:jgilbert]]

Here's the incoming cherry-pick:
https://github.com/mozilla/angle/commit/0c5a74c79daf671d2a55979ae0fa467ceda991ab

Comment 1

[Kelsey Gilbert [:jgilbert]]

Can you get this, dglastonbury?

Comment 2

[Kelsey Gilbert [:jgilbert]]

I think this needs a gleam update, since it's a new (pseudo-)extension. bholley, can you assign this?
If we don't take this before the ANGLE update, it's "ok", but we'll likely get a slight startup and possible vertex shading load regression. If that's ok, we don't need to block the ANGLE update on this for 66.

Comment 3

[Bobby Holley (:bholley)]

I tried adding GL_ANGLE_provoking_vertex to the list at [1], but it doesn't seem to show up in the generated code. Am I doing something wrong?

[1] https://searchfox.org/mozilla-central/rev/dac799c9f4e9f5f05c1071cba94f2522aa31f7eb/third_party/rust/gleam/build.rs#34

Comment 4

[Emilio Cobos Álvarez (:emilio)]

gl-generator generates stuff from XML files describing the extensions, but there's none for this one, it's not magical.

We probably need to get the scripts/gl_angle_ext.xml file from ANGLE somewhere in https://github.com/brendanzab/gl-rs, and make gleam depend on an updated version of that.

Want me to do that?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I took a stab at this, https://github.com/brendanzab/gl-rs/pull/480.

With that patch to gl_generator I can add that line to gleam and I can verify the ANGLE function is being generated.

Comment 6

[Bobby Holley (:bholley)]

Thanks Emilio!

One question from IRC that I didn't get an answer to: do we really need to do this in Rust at all? Can't we just set it from C++ when we pass the GL Context?

Comment 7

[Kelsey Gilbert [:jgilbert]]

Yeah, I guess we can do this in C++ land, though it feels very hacky to do it this way.

Comment 8

[Kelsey Gilbert [:jgilbert]]

bholley: Would you prefer we do it that way? I think I can write that patch, if so.

Comment 9

[Bobby Holley (:bholley)]

(In reply to Jeff Gilbert [:jgilbert] from comment #8)

bholley: Would you prefer we do it that way? I think I can write that patch, if so.

I'll leave it up to Emilio.

My initial reaction is that it's not worth investing a bunch of effort into generalizing gleam to handle this case, because we're going to be moving off of gleam and onto gfx-rs in the not-too-distant future. On the other hand, it looks like emilio may have already done the work in comment 5.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

I don't think it's much effort assuming my PR above is the right way to do it, but I need to get it reviewed...

Comment 11

[Kelsey Gilbert [:jgilbert]]

Is this done then? I'm going to update ANGLE as soon in 68 as possible, and this is a blocker.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

I poked in https://github.com/brendanzab/gl-rs/pull/480 again, but no response. We could either fork the crate or do it in C++, I guess. I can do the first this week if you want to go that path.

Comment 13

[Kelsey Gilbert [:jgilbert]]

Can you take this emilio?

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Pinged on IRC, and hoping to get that patch in during the week. Otherwise I'll use a local fork for now.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

https://mozilla.logbot.info/servo/20190328#c16145376 is the relevant IRC log.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

https://github.com/servo/gleam/pull/193

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1520953 - Update gleam. r=kats,jrmuizel

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1520953 - Use GL_ANGLE_provoking_vertex extension if available. r=jgilbert

You tell me if this is right, I have no Windows build available to test :)

Depends on D25602

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4aca7c222e6
Update gleam. r=kats,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/698a59556b89
Use GL_ANGLE_provoking_vertex extension if available. r=jgilbert

Comment 20

[Kelsey Gilbert [:jgilbert]]

Sweet, thanks!

Comment 21

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e4aca7c222e6
https://hg.mozilla.org/mozilla-central/rev/698a59556b89