[Webkit-unassigned] [Bug 101291] [EFL] Refactor GraphicsContext3DEFL
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 20 00:19:09 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=101291
--- Comment #52 from Kalyan <kalyan.kondapally at intel.com> 2012-11-20 00:21:04 PST ---
(In reply to comment #51)
> (From update of attachment 174840 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174840&action=review
>
> >>>>> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:113
> >>>>> + bool returnValue = m_platformContext->makeCurrent(m_platformSurface.get());
> >>>>
> >>>> Why do you use local variable here? Is below example more clear? I wonder if there is any reason.
> >>>>
> >>>> if (m_contextLostCallback && !m_platformContext->isValid()) {
> >>>> m_contextLostCallback->onContextLost();
> >>>> return false;
> >>>> }
> >>>>
> >>>> return m_platformContext->makeCurrent(m_platformSurface.get());
> >>>
> >>> This would be fine if we always had a valid call back function. This might not be true when GraphicsContext3D is used by a class other than WebGLGraphicsContext3D. We might end up in a situation where a callback is not set up. I.e in Webkit1 AcceleratedCompositingContextEfl creates GraphicsContex3D, GraphicsContex3D is also used by Texture Mapper to create a temporary wrapper for current Context.
> >>
> >> Can other classes use this function now ? If not, I'd like to recommend to use early return with "FIXME:". Then, you can fix this again when other classes start to use this function. How do you think ?
> >
> > What this function basically does is as follows:
> > It makes the context and surface associated with the GraphicsContext3D as the current GL Context and Drawable. Thus, it would be used frequently.
>
> If so, I don't object to use as it is. But, I still don't like to use bool returnValue.
> >>>> if (m_contextLostCallback && !m_platformContext->isValid()) {
> >>>> m_contextLostCallback->onContextLost();
> >>>> return false;
> >>>> }
> >>>>
> >>>> return m_platformContext->makeCurrent(m_platformSurface.get());
Sorry, I didnt check this properly before. This is completely wrong. We need to first make the context current before checking the status(if we have a valid context or not).
so basically it goes as follows:
1)Make the context current with the given surface. i.e. m_platformContext->makeCurrent(m_platformSurface.get())
2)Check if the first step succeeded and we still have a valid context.
3)return true if the above two conditions are met else false
If the changes are done in PlatformContext(as discussed above to return isValid()), then steps 1 and 2 are merged into one.
Than we have something like:
bool GraphicsContext3DPrivate::makeContextCurrent()
{
bool success = m_platformContext->makeCurrent(m_platformSurface.get());
if (!success && m_contextLostCallback) {
m_contextLostCallback->onContextLost();
// FIXME: Restore context
}
return success;
}
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list