Last Comment Bug 729726 - Use BGRA textures to back FBOs where more performant
: Use BGRA textures to back FBOs where more performant
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Jeff Gilbert [:jgilbert]
:
Mentors:
http://www.khronos.org/registry/gles/...
Depends on: 728724 739775
Blocks: 774058 743585
  Show dependency treegraph
 
Reported: 2012-02-22 14:50 PST by Jeff Gilbert [:jgilbert]
Modified: 2012-07-15 03:18 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Try to request BGRA backing texture where possible (974 bytes, patch)
2012-04-10 16:03 PDT, Jeff Gilbert [:jgilbert]
jacob.benoit.1: review+
Details | Diff | Splinter Review

Description Jeff Gilbert [:jgilbert] 2012-02-22 14:50:53 PST
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.
Comment 1 Jeff Gilbert [:jgilbert] 2012-04-09 22:40:20 PDT
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)
Comment 2 Jeff Gilbert [:jgilbert] 2012-04-09 23:00:43 PDT
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
    identical.
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)
Comment 3 Jeff Gilbert [:jgilbert] 2012-04-10 16:03:44 PDT
Created attachment 613809 [details] [diff] [review]
Try to request BGRA backing texture where possible

This depends on the patch in bug 739775.
Comment 4 Mozilla RelEng Bot 2012-05-11 15:56:13 PDT
Autoland Patchset:
	Patches: 613809
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=c4f4c157e097
Try run started, revision c4f4c157e097. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=c4f4c157e097
Comment 5 Ed Morley [:emorley] 2012-06-14 05:13:12 PDT
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!
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-06-16 07:07:24 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/10ee7a83c5a8
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-06-16 19:41:35 PDT
https://hg.mozilla.org/mozilla-central/rev/10ee7a83c5a8

Note You need to log in before you can comment on or make changes to this bug.