595055 - Resizing totally broken on OpenGL

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Joe Drew (not getting mail)]

Attachment: Set the correct context when deleting textures

Right now, resizing a window that's accelerated using OpenGL draws garbage. The reason this happens is because, in the BasicTextureImage destructor, we preferentially set the shared context as current. Unfortunately, all of our previous state was set on the normal mGLContext, meaning important things like the current viewport get unset, causing us not to draw.

This patch just reverses the order we choose our contexts in, but I'd be willing to use a different approach to not make the wrong context current if it'd be cleaner. (One thing that has come to my mind is a resize call on TexImage.)

Comment 1

[Joe Drew (not getting mail)]

Attachment: fixed-er

Previous patch is obviously wrong if mGLContext is null. Just use mGLContext unconditionally.

Comment 2

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

Attachment: how about this instead?

Nice!

How about this instead, though?

Fixes the problem on win32 too btw, now just leaves some flashing to black in between rendering normal frames, but that goes away when I turn on double buffering.

Comment 3

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

Also Cc'ing Benoit, since this would have been caught by the debug GLContext that he's working on whose utility I was dubious about all day :-)  (it'll catch among other things calling a function from one GLContext while another is current).

Comment 4

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

this also fixes the new tab corruption.

Comment 5

[Joe Drew (not getting mail)]

Attachment: fix resize a little more

I think that your patch is a little wrong if both contexts have been destroyed. This patch is built on current birch (I pushed my previous patch to birch) and fixes the problem I saw.

I also don't really know what the shared context is for, clearly, so I think you're a better person to review it. (And explain it!)

Comment 6

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

well, I thought about that afterwards, but I'm pretty sure it'll be correct in either case.  A shared context is a "parent" context that we can use to cleanup any resources; it's associated with a random offscreen framebuffer and is effectively global.  On some platforms we might not be able to share though, but if we can, it can speed up some things (like it can let us do texture loads on a thread etc).

So we look if the real context was destroyed (e.g. the widget went away), and if so, then we need to use the shared context to delete the texture.  If we have no shared context then we're done, because the only context that had access to the texture is gone (and thus the resources were released).  I don't think we'll ever be in a situation where we have a destroyed shared context, though maybe; replacing the if condition with if (ctx && !ctx->IsDestroyed()) is probably the right thing to do.

I'll push the followup to brich.

Comment 7

[Joe Drew (not getting mail)]

Sounds good.

Comment 8

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

http://hg.mozilla.org/mozilla-central/rev/508d0a50827c