Closed Bug 1528820 Opened 11 months ago Closed 11 months ago

Flashing black rectangles with picture caching enabled on webrender on android

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: jnicol, Assigned: jnicol)

References

Details

Attachments

(2 files)

Noticed this today. Haven't looked in to it in detail it, but bisected it to this commit: https://hg.mozilla.org/mozilla-central/rev/f2b5a9d987e8

I can look in to this unless it's immediately obvious to anyone what the problem is.

OnePlus 6, Adreno 630. Haven't tested on other devices so don't know if device-specific.

Non-obvious at all, otherwise we'd rolled it back by now.

Summary: Flashing black rectangles on webrender on android → Flashing black rectangles with picture caching enabled on webrender on android

This is the same Adreno bug as bug 1499785 (blitting to a texture array always writes to 0th layer, not the actually bound one). Here we are blitting from the default framebuffer to the picture cache.

Prior to the commit mentioned in comment 0, picture cache tiles always ended up in separate textures as they were too large for the main texture cache. So we were blitting to a TEXTURE_2D rather than a layer of an array. That commit gave the picture cache its own texture array the appropriate size, causing the regression.

In bug 1499785 we considered blitting to an intermediate renderbuffer, then using glCopyTexSubImage to copy from that to the correct layer. As this still had issues on older Adrenos, we decided to use glCopyImageSubData instead as it would work on more devices. We cannot use glCopyImageSubData in this case, however, because we are reading from the main framebuffer rather than a texture. So we should use the intermediate renderbuffer approach. Also, I intend to put this workaround at a lower level than we did there, so that any future changes which require blitting to a texture array will not hit this bug again.

Same for blit_render_target_invert_y(). Make them wrappers around the
private function blit_render_target_impl(), which uses the currently
bound read and draw targets as before.

There is a bug on Adreno GPUs where glBlitFramebuffers always writes
to the 0th layer of a texture array, regardless of which layer is
actually attached to the draw framebuffer.

With picture caching enabled on webrender, the cached pictures were
all being drawn to the 0th layer of the picture cache texture array,
leaving the other layers blank. This was resulting in the wrong
content being drawn on one tile of the screen, and the rest being
black.

This works around this by blitting to an intermediate renderbuffer,
then using glCopyTexSubImage3D to copy from the renderbuffer to the
correct texture layer.

Depends on D22304

Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/912d0497234a
Make Device::blit_render_target() require read and draw targets as arguments. r=kvark
https://hg.mozilla.org/integration/autoland/rev/cb745568d893
Work around Adreno bug when blitting to texture array. r=kvark
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee: nobody → jnicol
You need to log in before you can comment on or make changes to this bug.