Fix glScissor calls in ContainerLayerOGL

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [approved-patches-landed])

Attachments

(3 attachments, 1 obsolete attachment)

Created attachment 475782 [details] [diff] [review]
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).
Attachment #475782 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #475782 - Flags: review? → review?(bas.schouten)
(Assignee)

Comment 1

7 years ago
Needs to be tested on Mac as this could easily break stuff there without being caught by reftests...
This patch actually creates the mentioned bug on mac. Is it possible that the y coordinates are platform specific somehow?
(Assignee)

Comment 3

7 years ago
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?
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.
Duplicate of this bug: 597127
(Assignee)

Comment 6

7 years ago
Created attachment 476022 [details] [diff] [review]
Fix glScissor calls in ContainerLayerOGL

Here you go.
Attachment #475782 - Attachment is obsolete: true
Attachment #476022 - Flags: review?(vladimir)
Attachment #475782 - Flags: review?(bas.schouten)
Attachment #476022 - Flags: review?(vladimir) → review+
Attachment #476022 - Flags: approval2.0+
(Assignee)

Comment 7

7 years ago
http://hg.mozilla.org/mozilla-central/rev/3e57d3b0e8f9
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The Mac #define is XP_MACOSX, not XP_MAC.
(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.
Created attachment 476571 [details] [diff] [review]
use XP_MACOSX

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

Here's a patch fixing this.
Attachment #476571 - Flags: review?(joe)
(Assignee)

Updated

7 years ago
blocking2.0: --- → ?
(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 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
Attachment #476571 - Flags: review?(joe)
Attachment #476571 - Flags: review+
Attachment #476571 - Flags: approval2.0+
http://hg.mozilla.org/mozilla-central/rev/cf2461dc5055

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

Updated

7 years ago
blocking2.0: ? → ---
Created attachment 476686 [details] [diff] [review]
Reverse Y axis when double buffering is disabled (drawing in a FBO)
Attachment #476686 - Flags: feedback?(vladimir)
Blocks: 586462
Whiteboard: [approved-patches-landed]
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago6 years ago
Resolution: --- → FIXED
Attachment #476686 - Flags: feedback?(vladimir)
You need to log in before you can comment on or make changes to this bug.