Closed Bug 729726 Opened 8 years ago Closed 8 years ago

Use BGRA textures to back FBOs where more performant


(Core :: Canvas: WebGL, defect)

Not set





(Reporter: jgilbert, Assigned: jgilbert)





(1 file)

On some platforms, it should be faster to use BGRA-texture-backed FBOs, since we read-back, we do so in BGRA.

I am not positive this will improve performance on all platforms though. Perhaps this would be appropriate to blocklist 'slow' BGRA platforms.

Some platforms do calculations in BGRA regardless of if you request RGBA, but do a conversion on read-back. Since we also do this conversion, we thrash from BGRA to RGBA and back to BGRA.
Blocks: 743585
Interesting findings: Desktop GL does not appear to have a way to request a texture be stored in BGRA, even if it usually is. We can only request RGBA and hope for the best. In particular, EXT_bgra allows us to use format=GL_BGRA to supply data to texImage2D, and to read using readPixels, but does not allow us to request textures with internalFormat=GL_BGRA. 

GLES however offers EXT_texture_format_bgra8888, which does allow specifying GL_BGRA for the texture's internalFormat, though, as always, it'll only be a hint.

This might be useful enough to try to create a desktop GL extension for, but it seems like everyone is already doing The Right Thing(tm) and storing as BGRA. (Even llvmpipe, as seen in bug 743585)
Assignee: nobody → jgilbert
I'll also add that there doesn't appear to be any functionality for requesting BGRA renderbuffers, but this should be fine, since resolving our AA renderbuffer onto our backing texture should do any swizzling needed cheaply.

The GLES extension ANGLE_framebuffer_multisample is a concern, since we use it to create and blit+resolve our AA FBO to our backing texture, and it has the following stipulation:
    If both the draw and read framebuffers are framebuffer complete and
    either has a value of SAMPLE_BUFFERS that is greater than zero, then
    the error INVALID_OPERATION is generated if BlitFramebufferANGLE is
    called and the formats of the draw and read framebuffers are not
This should probably be clarified, since it seems like we will be unable to blit an MS RGBA renderbuffer to a BGRA texture.

Amusingly, since we can only specify RGB/RGBA(/BGRA, hopefully), and renderbuffers must specify as RGB565/RGBA4/RGBA8, blitting multisampled color renderbuffers to textures technically should never succeed. I'll see if we can't get the spec clarified. Since we control our copy of ANGLE, we could at least hard-code that it is reliable there, for now. (Though we'll need to double-check the source code)
Depends on: 739775
This depends on the patch in bug 739775.
Attachment #613809 - Flags: review?(bjacob)
Attachment #613809 - Flags: review?(bjacob) → review+
Attachment #613809 - Flags: checkin?(jgilbert)
Whiteboard: [autoland:-p all -u all]
Whiteboard: [autoland:-p all -u all] → [autoland-try:-p all -u all]
Whiteboard: [autoland-try:-p all -u all] → [autoland-in-queue]
Autoland Patchset:
	Patches: 613809
	Branch: mozilla-central => try
Try run started, revision c4f4c157e097. To cancel or monitor the job, see:
Whiteboard: [autoland-in-queue]
Many of the people pushing checkin-neededs only look at the keyword, not the per-patch flag. In the case of bugs where all patches are good to push, the keyword is preferred, since it means people can land without having to edit the bug twice (once for overall meta, once for per patch). Adding the keyword now, so this shows up on people's searches.

Unfortunately due to this, the try run is fairly out of date now, so requesting another just in case (been bitten too many times, so I have a 'recent try run for everything part of the build' policy when landing checkin-neededs). Sorry for the delay!
Keywords: checkin-needed
Whiteboard: [autoland-try:-p all -u all]
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [autoland-try:-p all -u all]
Target Milestone: --- → mozilla16
Attachment #613809 - Flags: checkin?(jgilbert)
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 774058
You need to log in before you can comment on or make changes to this bug.