Bug 91897 – [Texmap] Performance regression in texture uploads after r121223

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Noam Rosenthal]

[Texmap] Performance regression in texture uploads after r121223

[Noam Rosenthal]

Since we're using GraphicsContext3D::extractImageData, we can't take advantage of desktop GL drivers that support the BGRA extension. This leads to much more time spent in texture uploads for Qt and GTK on desktop.

[Noam Rosenthal]

Created attachment 153579 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 153579 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153579&action=review

> Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp:155
> +const uint8_t* GraphicsContext3D::extractRawImageData(Image* image)

why not just rawImageData?

> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:490
>  #endif
>  
> +
> +
> +// Extracts the contents of the given Image without copying or formatting.

why so many newlines?

[Dongseong Hwang]

I'm sorry for this regression and thank you for fixing it.

[Noam Rosenthal]

Comment on attachment 153579 [details]
Patch

Need to rework this patch, it causes some color issues on GTK.

[Noam Rosenthal]

Created attachment 154978 [details]
Patch

[Noam Rosenthal]

Committed r123894: <http://trac.webkit.org/changeset/123894>

[Jocelyn Turcotte]

This wasn't explained here, but the reason for reverting this patch is that it introduced an additional huge performance regression since tile updates from the web process are transferred through a 2000x2000 buffer and there is no way to tell extractImageData to BGRA->RGBA only the part of the buffer that we want.

So the whole 2000x2000 buffer is converted each time we need a tiny piece of a tile to be updated.

The way to fix it would be to add a partial image extraction capability to GraphicsContext3D, but this would require an implementation for every port. WebGL doesn't need it itself and this doesn't seems like it would be worth it.

[Noam Rosenthal]

(In reply to comment #8)
> This wasn't explained here, but the reason for reverting this patch is that it introduced an additional huge performance regression since tile updates from the web process are transferred through a 2000x2000 buffer and there is no way to tell extractImageData to BGRA->RGBA only the part of the buffer that we want.
> 
> So the whole 2000x2000 buffer is converted each time we need a tiny piece of a tile to be updated.
> 
> The way to fix it would be to add a partial image extraction capability to GraphicsContext3D, but this would require an implementation for every port. WebGL doesn't need it itself and this doesn't seems like it would be worth it.

Ah, I didn't realize that was the source of the regression. Now it makes more sense!