825995 - ColorLayers are clipped incorrectly with OpenGL

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Nick Cameron [:nrc]]

Bug 797664 broke the scissor rect logic in GLContext so that ColorLayers can paint over the top of the browser chrome or can get clipped out of the bottom 65 pixels etc.

Patch coming up...

Comment 1

[Nick Cameron [:nrc]]

Attachment: patch

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 697118 [details] [diff] [review]
patch

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

This may be functionally equivalent, but we should probably stick to reversing the regression caused by pulling the flip-y (and such) logic out of raw_fScissor and putting it in fScissor. To that end, just change these two raw_fScissor calls to fScissor and that should do it.

Comment 3

[Kelsey Gilbert [:jgilbert]]

I checked to make sure that this is the only such regression, and it looks like it is, thankfully.

Comment 4

[Nick Cameron [:nrc]]

(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> Comment on attachment 697118 [details] [diff] [review]
> patch
> 
> Review of attachment 697118 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This may be functionally equivalent, but we should probably stick to
> reversing the regression caused by pulling the flip-y (and such) logic out
> of raw_fScissor and putting it in fScissor. To that end, just change these
> two raw_fScissor calls to fScissor and that should do it.

Are you happy with the redundant ScissorRect().SetRect expressions in fScissor from these call sites? I don't think there is any functional change, but it seems a little bit of a waste. I don't suppose this code is particularly hot though.

Comment 5

[Nick Cameron [:nrc]]

Attachment: patch

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 697274 [details] [diff] [review]
patch

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

Great, thanks for finding and fixing this.

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Nick Cameron [:nrc] from comment #4)
> (In reply to Jeff Gilbert [:jgilbert] from comment #2)
> > Comment on attachment 697118 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 697118 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > This may be functionally equivalent, but we should probably stick to
> > reversing the regression caused by pulling the flip-y (and such) logic out
> > of raw_fScissor and putting it in fScissor. To that end, just change these
> > two raw_fScissor calls to fScissor and that should do it.
> 
> Are you happy with the redundant ScissorRect().SetRect expressions in
> fScissor from these call sites? I don't think there is any functional
> change, but it seems a little bit of a waste. I don't suppose this code is
> particularly hot though.

The redundant calls probably aren't necessary, so maybe we should pull them out. Want to throw together that patch as well?

Comment 8

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3c620f1d136

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c3c620f1d136