Closed
Bug 595055
Opened 14 years ago
Closed 14 years ago
Resizing totally broken on OpenGL
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta7+ |
People
(Reporter: joe, Assigned: joe)
References
Details
Attachments
(1 file, 3 obsolete files)
2.07 KB,
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
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.)
Attachment #473904 -
Flags: review?(vladimir)
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → joe
blocking2.0: --- → beta6+
Assignee | ||
Comment 1•14 years ago
|
||
Previous patch is obviously wrong if mGLContext is null. Just use mGLContext unconditionally.
Attachment #473904 -
Attachment is obsolete: true
Attachment #473905 -
Flags: review?(vladimir)
Attachment #473904 -
Flags: review?(vladimir)
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.
Attachment #473933 -
Flags: review?(joe)
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).
this also fixes the new tab corruption.
Assignee | ||
Comment 5•14 years ago
|
||
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!)
Attachment #473905 -
Attachment is obsolete: true
Attachment #473933 -
Attachment is obsolete: true
Attachment #474214 -
Flags: review?(vladimir)
Attachment #473933 -
Flags: review?(joe)
Attachment #473905 -
Flags: review?(vladimir)
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.
Assignee | ||
Comment 7•14 years ago
|
||
Sounds good.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
Attachment #474214 -
Flags: review?(vladimir) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•