Bug 79759 - [EFL] Implementation of missing functions in GraphicsContext3DPrivate
Summary: [EFL] Implementation of missing functions in GraphicsContext3DPrivate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 62961
Blocks: 79766
  Show dependency treegraph
 
Reported: 2012-02-28 00:14 PST by Hyowon Kim
Modified: 2012-03-04 18:44 PST (History)
5 users (show)

See Also:


Attachments
Patch (7.66 KB, patch)
2012-02-28 00:21 PST, Hyowon Kim
noam: review+
noam: commit-queue-
Details | Formatted Diff | Diff
modified patch (6.98 KB, patch)
2012-02-28 21:18 PST, Hyowon Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.