The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla13

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

unspecified
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
Gadzooks!
(Assignee)

Comment 2

5 years ago
Created attachment 596536 [details] [diff] [review]
Don't redraw data that we copied from the old buffer
Attachment #596536 - Flags: review?(joe)
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?
Attachment #596536 - Flags: review?(joe) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/858aab8397af
Assignee: nobody → matt.woodrow
https://hg.mozilla.org/mozilla-central/rev/858aab8397af
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.