Bug 79759

Summary: [EFL] Implementation of missing functions in GraphicsContext3DPrivate
Product: WebKit Reporter: Hyowon Kim <hw1008.kim>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, lucas.de.marchi, noam, ryuan.choi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Bug Depends on: 62961    
Bug Blocks: 79766    
Attachments:
Description Flags
Patch
noam: review+, noam: commit-queue-
modified patch none

Description Hyowon Kim 2012-02-28 00:14:27 PST
The implementation of GC3DPrivate has splited into three patches in order to keep it simple and short.

1. Bug 62961 is the initial implementation including simple functions which only call GL functions through Evas_GL_API.

2. This patch contains the implementation of a little complicated functions such as getActiveAttrib(), getShaderSource() and so on.

3. The last patch for GC3DPrivate is going to include initialization and destruction using Evas_GL APIs.
Comment 1 Hyowon Kim 2012-02-28 00:21:37 PST
Created attachment 129204 [details]
Patch
Comment 2 Noam Rosenthal 2012-02-28 07:39:27 PST
Comment on attachment 129204 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129204&action=review

LGTM, except a couple of nitpicks - please fix before committing.

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:434
> +    if (m_syntheticErrors.size() > 0) {

Use .isEmpty()

> Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:463
> +        *value /= 4;

Comment for why this is /4
Comment 3 Hyowon Kim 2012-02-28 21:18:18 PST
Created attachment 129380 [details]
modified patch
Comment 4 Hyowon Kim 2012-02-28 21:19:17 PST
(In reply to comment #2)
> (From update of attachment 129204 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=129204&action=review
> LGTM, except a couple of nitpicks - please fix before committing.
> > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:434
> > +    if (m_syntheticErrors.size() > 0) {
> Use .isEmpty()
> > Source/WebCore/platform/graphics/efl/GraphicsContext3DPrivate.cpp:463
> > +        *value /= 4;
> Comment for why this is /4

It's a really important finding!

I was a little confused about OpenGL ES 2 compliance.

Evas_GL offers the OpenGL ES 2 interface, so I removed codes for desktop-GL in getIntegerv() like GraphicsContext3DOpenGLES.cpp.

Please review this modification again. Thanks.
Comment 5 Noam Rosenthal 2012-02-28 21:36:03 PST
Comment on attachment 129380 [details]
modified patch

LGTM, please have one of the EFL committers set the cq flag.
Comment 6 WebKit Review Bot 2012-03-04 18:43:51 PST
Comment on attachment 129380 [details]
modified patch

Clearing flags on attachment: 129380

Committed r109694: <http://trac.webkit.org/changeset/109694>
Comment 7 WebKit Review Bot 2012-03-04 18:44:00 PST
All reviewed patches have been landed.  Closing bug.