[Webkit-unassigned] [Bug 101291] [EFL] Refactor GraphicsContext3DEFL

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 19 23:30:13 PST 2012


https://bugs.webkit.org/show_bug.cgi?id=101291





--- Comment #51 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-11-19 23:32:08 PST ---
(From update of attachment 174840)
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.

-- 
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