725209 - BasicBufferOGL redraws it's entire content when it attempts to self copy.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Matt Woodrow (:mattwoodrow)]

We when try do a self copy with BasicBufferOGL, we allocate a new TextureImage and copy the contents across. This calls Resize() on the texture image, which sets the texture state to 'Allocated'. Then when we attempt to update this texture, it resets the draw area to that of the entire texture (via BasicTextureImage::GetUpdateRegion).

I tried just forcibly updating the texture state after doing the copy, but this sometimes gives us uninitialized memory showing through.

I'd suggest that self-copy of GL buffers has been broken since forever and we've always just happily redrawn the entire contents.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Gadzooks!

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't redraw data that we copied from the old buffer

Comment 3

[Joe Drew (not getting mail)]

Comment on attachment 596536 [details] [diff] [review]
Don't redraw data that we copied from the old buffer

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

::: gfx/layers/opengl/ThebesLayerOGL.cpp
@@ +603,5 @@
> +        if (mBufferRotation != nsIntPoint(0, 0)) {
> +        // If mBuffer is rotated, then BlitTextureImage will only be copying the bottom-right section
> +        // of the buffer. We need to invalidate the remaining sections so that they get redrawn too.
> +        // Alternatively we could teach BlitTextureImage to rearrange the rotated segments onto
> +        // the new buffer.

all these comments need to be indented or moved above

@@ +612,5 @@
> +                                   mBufferRect.y + mBufferRect.height - mBufferRotation.y);
> +
> +          // Mark the remaining quadrants (bottom-left&right, top-right) as invalid.
> +          nsIntRect bottom(mBufferRect.x, rotationPoint.y, mBufferRect.width, mBufferRotation.y);
> +          nsIntRect topright(rotationPoint.x, mBufferRect.y, mBufferRotation.x, rotationPoint.y - mBufferRect.y);

Maybe you could sketch out a little ASCII diagram so people can follow more easily?

Comment 4

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/858aab8397af

Comment 5

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/858aab8397af