Use an intermediate renderbuffer when blitting between texture arrays…

[jamienicol]

… on adreno devices

When the texture cache is grown we allocate a larger texture array
then copy each layer of the old array in to the new one. To perform
this copy we use use glBlitFramebuffer, with the read and draw
framebuffers bound to the correct layer in the old and new textures.

Unfortunately on Adreno devices this does not work. glBlitFramebuffer
always writes to the 0th layer of the texture bound to the draw
framebuffer. glCopyTexSubImage3D also does not work correctly - it
always reads from the 0th layer of the texture bound to the read
framebuffer.

As a workaround, we use glBlitFramebuffer to blit from the old texture
in to a temporary renderbuffer, then use glCopyTexSubImage3D to copy
from that renderbuffer to the new texture.

---

This change is 

[leeoniya]

good candidate for https://github.com/servo/webrender/wiki/Driver-issues ?

[gw3583]

@leeoniya Yep, definitely.

[gw3583]

Compile errors on CI (I didn't check the platforms / features causing it):

error[E0609]: no field `width` on type `&device::gl::Texture`
    --> webrender\src\device\gl.rs:1641:46
     |
1641 |                                          src.width as _, src.height as _);
     |                                              ^^^^^
error[E0609]: no field `height` on type `&device::gl::Texture`
    --> webrender\src\device\gl.rs:1641:62
     |
1641 |                                          src.width as _, src.height as _);
     |                                                              ^^^^^^
error[E0609]: no field `width` on type `&device::gl::Texture`
    --> webrender\src\device\gl.rs:1668:57
     |
1668 |                                               0, 0, src.width as _, src.height as _);
     |                                                         ^^^^^
error[E0609]: no field `height` on type `&device::gl::Texture`
    --> webrender\src\device\gl.rs:1668:73
     |
1668 |                                               0, 0, src.width as _, src.height as _);
     |                                                                         ^^^^^^
error[E0609]: no field `0` on type `&_`
    --> webrender\src\device\gl.rs:1673:48
     |
1673 |             self.gl.delete_renderbuffers(&[rbo.0]);
     |                                                ^
error: aborting due to 5 previous errors
For more information about this error, try `rustc --explain E0609`.
error: Could not compile `webrender`.
Caused by:

[jamienicol]

Texture.width/height got changed to Texture.size recently, but that hadn't made it to gecko yet.

[jamienicol]

I have updated the wiki page to include this bug

[kvark]

how are we sure gl::RGBA8 is compatible with whatever texture we are blitting?

[kvark]

nit: remove &

[kvark]

here and below: when matching we don't need the reference, instead we can avoid the fields moved out by marking them as Some((_, ref fbo)). IIRC, recent Rust inserts some of those ref automatically, but I'm not a big fan of that

[kvark]

note that we have 2 bugs here covered by the same flag: inability to blit to layer != 0 and inability to copy from layer != 0. The flag name itself only reflects the first bug, which is a bit unclear.

[staktrace]

For posterity: this patch fixes the issues on my Xperia XZ1 Compact (which has an Adreno 540) but doesn't fix the problems on my Xperia Z3 Compact (which has an Adreno 330).

[kvark]

@staktrace do we know what other issues Adreno 330 has in addition to these?

[staktrace]

It's a little hard to tell which of the visible artifacts are caused by this issue and which are caused by other issues. Certainly fixing this issue on Adreno 540 results in much better behaviour, so either all the visible artifacts on the 330 are caused by this problem, or there's multiple problems that only affect the 330. I don't have a good sense for how likely that is.

[Manishearth]

servo/servo#22232 is also fixed by this

[Manishearth]

@kvark any progress on this being merged? it fixes a rather annoying Servo bug too.

[jdm]

@Manishearth see #3338.

[bors-servo]

☔ The latest upstream changes (presumably #3338) made this pull request unmergeable. Please resolve the merge conflicts.

[kvark]

If I understand correctly, this can be closed in favor of #3338. Please feel free to reopen otherwise!