Bug 79196

Summary: [BlackBerry] Upstream GLES2Context.{h, cpp}
Product: WebKit Reporter: Leo Yang <leo.yang>
Component: New BugsAssignee: Leo Yang <leo.yang>
Status: RESOLVED FIXED    
Severity: Normal CC: charles.wei, rwlbuis, staikos, tonikitoo, yong.li.webkit, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73144    
Attachments:
Description Flags
Patch
rwlbuis: review-
Patch v2
none
Patch v3 rwlbuis: review+

Leo Yang
Reported 2012-02-21 22:17:12 PST
OpenGL ES 2.0 context for the blackberry porting.
Attachments
Patch (8.23 KB, patch)
2012-02-22 01:32 PST, Leo Yang 		rwlbuis: review-
DetailsFormatted DiffDiff
Patch v2 (8.33 KB, patch)
2012-02-23 01:50 PST, Leo Yang 		no flags
DetailsFormatted DiffDiff
Patch v3 (8.32 KB, patch)
2012-02-23 01:54 PST, Leo Yang 		rwlbuis: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Leo Yang
Comment 1 2012-02-22 01:32:12 PST
Created attachment 128155 [details] Patch
Rob Buis
Comment 2 2012-02-22 04:14:44 PST
Comment on attachment 128155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128155&action=review Still some things to improve. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:42 > +#include "WebPage_p.h" All needed? > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:44 > +#include <BlackBerryPlatformWindow.h> Already included in header. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:113 > + // Sniff, this is so sad. Seems a strange comment, maybe explain why it is sad > Source/WebKit/blackberry/WebKitSupport/GLES2Context.h:39 > +#include <wtf/OwnPtr.h> Is this one needed?
Leo Yang
Comment 3 2012-02-23 01:49:46 PST
(In reply to comment #2) > (From update of attachment 128155 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128155&action=review > > Still some things to improve. > > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:42 > > +#include "WebPage_p.h" > > All needed? > Yes. WebPagePrivate is being used in the constructor. > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:44 > > +#include <BlackBerryPlatformWindow.h> > > Already included in header. I'll remove it. > > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:113 > > + // Sniff, this is so sad. > > Seems a strange comment, maybe explain why it is sad > I'll make a clear comment. > > Source/WebKit/blackberry/WebKitSupport/GLES2Context.h:39 > > +#include <wtf/OwnPtr.h> > > Is this one needed? I'll remove it.
Leo Yang
Comment 4 2012-02-23 01:50:35 PST
Created attachment 128435 [details] Patch v2
Leo Yang
Comment 5 2012-02-23 01:54:22 PST
Created attachment 128436 [details] Patch v3 Removing some unnecessary blank lines.
Rob Buis
Comment 6 2012-02-23 04:30:29 PST
Comment on attachment 128436 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=128436&action=review LGTM. > Source/WebKit/blackberry/ChangeLog:1 > +2012-02-22 Leo Yang <leo.yang@torchmobile.com.cn> It is 23 feb now. > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:60 > + if (surface) Can just do if (BackingStoreCompositingSurface* surface = compositingSurface()) > Source/WebKit/blackberry/WebKitSupport/GLES2Context.cpp:91 > + if (surface) Ditto
Leo Yang
Comment 7 2012-02-23 18:15:51 PST
Committed r108711: <http://trac.webkit.org/changeset/108711>
Yong Li
Comment 8 2012-04-24 11:12:35 PDT
Do we know why the 2 files are put in WebKitSupport rather than WebCoreSupport?
Note You need to log in before you can comment on or make changes to this bug.