1078109 - Severe SVG performance regression from tiled thebes layers on OSX on the IE test drive helicopter dem

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]:

See bug 608495 comment 16.

Comment 1

[Matt Woodrow (:mattwoodrow)]

This is a fun bug.

The compositor thread tile upload seems to be doing swizzling on the cpu before uploading the texture. That delays the unlock of the tile buffer, and the content thread has to allocate a new tile instead.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Choose the correct RGBA format

This matches the logic in UploadImageDataToTexture. If we don't do this then we end up trying to do glTexSubImage2D with a different format to what we used for glTexImage2D and sadness ensues.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Might be worth either factoring out the code that needs to match into a helper function or at least having comments both places about it needing to match...

Comment 4

[Andreas Gal :gal]

Comment on attachment 8500216 [details] [diff] [review]
Choose the correct RGBA format

I have wrangled with this code path in the driver before. This is righteous and recommended by Apple.

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8500216 [details] [diff] [review]
Choose the correct RGBA format

Review of attachment 8500216 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLTextureImage.cpp
@@ +235,5 @@
> +    GLenum format;
> +    GLenum type;
> +    if (mGLContext->GetPreferredARGB32Format() == LOCAL_GL_BGRA) {
> +        format = LOCAL_GL_BGRA;
> +        type = LOCAL_GL_UNSIGNED_INT_8_8_8_8_REV;

Assert that this is not GLES.

@@ +237,5 @@
> +    if (mGLContext->GetPreferredARGB32Format() == LOCAL_GL_BGRA) {
> +        format = LOCAL_GL_BGRA;
> +        type = LOCAL_GL_UNSIGNED_INT_8_8_8_8_REV;
> +    } else {
> +        format = LOCAL_GL_BGRA;

This should be RGBA.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Choose the correct RGBA format v2

Comment 7

[Boris Zbarsky [:bzbarsky]]

I still stand by comment 3...

Comment 8

[Matt Woodrow (:mattwoodrow)]

(In reply to Boris Zbarsky [:bz] from comment #7)
> I still stand by comment 3...

Yeah, I'm just not sure how to do that in a useful way without changing the TextureImage API significantly.

I can add a comment when I land at least.

Comment 9

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43c963b56723

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43c963b56723