596907 - Fix glScissor calls in ContainerLayerOGL

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Fix glScissor calls in ContainerLayerOGL

Account for the fact that glScissor counts y coordinates starting from the bottom.

This fixes the bug making a black rectangle being overlaid over the bottom of the firefox main window, the same size as the GUI stuff at the top.

Introduced GLIntRect to make that clean; could have used nsIntRect but didn't necessarily want to implicitly rely on its internal data layout never changing, and on PRInt32==GLint (althought the latter point is really unlikely to ever change).

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Needs to be tested on Mac as this could easily break stuff there without being caught by reftests...

Comment 2

[Matt Woodrow (:mattwoodrow)]

This patch actually creates the mentioned bug on mac. Is it possible that the y coordinates are platform specific somehow?

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Heh, that's what it's sounding like!
On one end, glScissor is always expecting coords starting from the bottom.
On the other end, what is the y-axis for the clipRect and visibleRect?

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

On Mac, window coords match OpenGL coords -- Y increases up from the bottom.  Everywhere else, Y increases down.  So we need to add something to nsIWidget that will tell us what direction the coordinate space goes (or, actually, in the interim we can just do some #ifdef XP_MAC ery).

Also, please don't introduce GLIntRect; a one-off type just complicates matters here, because you have to go back and look at its trivial definition while reading the code.

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Fix glScissor calls in ContainerLayerOGL

Here you go.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/3e57d3b0e8f9

Comment 8

[Matt Woodrow (:mattwoodrow)]

This has regressed behaviour on both mac and ubuntu for me. We're also failing a lot more tests in the RGL suite on tinderbox because of it.

It appears XP_MAC isn't defined in ContainerLayerOGL.cpp even on mac.

What OS (and probably more importantly, which widget implementation) did you originally observe this on? It appears ubuntu has the same behaviour as mac (changing the #ifdef to #if 1 fixes things).

Comment 9

[Joe Drew (not getting mail)]

The Mac #define is XP_MACOSX, not XP_MAC.

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to comment #8)
> This has regressed behaviour on both mac and ubuntu for me. We're also failing
> a lot more tests in the RGL suite on tinderbox because of it.

Oh. Can you explain me where you see them? I wasn't aware of that.

> 
> It appears XP_MAC isn't defined in ContainerLayerOGL.cpp even on mac.
> 
> What OS (and probably more importantly, which widget implementation) did you
> originally observe this on? It appears ubuntu has the same behaviour as mac
> (changing the #ifdef to #if 1 fixes things).

I observed this on Fedora 13 / x86-64. The widgets are the default i.e. GTK. This patch did fix it for me here.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: use XP_MACOSX

(In reply to comment #9)
> The Mac #define is XP_MACOSX, not XP_MAC.

Here's a patch fixing this.

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to comment #8)
> What OS (and probably more importantly, which widget implementation) did you
> originally observe this on? It appears ubuntu has the same behaviour as mac
> (changing the #ifdef to #if 1 fixes things).

What graphics driver are you using? If it's Mesa-based (i.e. if it's a free driver) are you using Mesa 7.8 ? I'm asking because I got this answer on #kwin:

[09:53] <fredrikh> bjacob: there's a bug in mesa 7.8 where glCopySubImage() inverts the y axis when it's used with a bound framebuffer object
[09:53] <fredrikh> it's fixed in 7.9
[09:54] <fredrikh> i'm not aware of anything like that with glScissor(), but it could be a similar issue

So it would be worth trying with Mesa 7.9. On my side, I'm using the NVIDIA driver which bypasses Mesa altogether.

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 476571 [details] [diff] [review]
use XP_MACOSX

r+a=shaver for define fix, may the gods of module purity have mercy on my soul

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/cf2461dc5055

Keeping this bug open until things work for Matt on Ubuntu.

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Reverse Y axis when double buffering is disabled (drawing in a FBO)