Bug 61802

Summary: [Qt][WK2] Add QGLWidget viewport support to MiniBrowser
Product: WebKit Reporter: Viatcheslav Ostapenko <ostap73>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, webkit.review.bot
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Add QGLWidget viewport mode to minibrowser app.
none
Fix style problem
hausmann: review+
Fix issues from Simon review. none

Viatcheslav Ostapenko
Reported 2011-05-31 13:26:19 PDT
Add command line parameter and menu item to MiniBrowser application enabling use of QGLWidget for browser viewport.
Attachments
Add QGLWidget viewport mode to minibrowser app. (6.26 KB, patch)
2011-05-31 13:30 PDT, Viatcheslav Ostapenko 		no flags
DetailsFormatted DiffDiff
Fix style problem (6.27 KB, patch)
2011-05-31 13:39 PDT, Viatcheslav Ostapenko 		hausmann: review+
DetailsFormatted DiffDiff
Fix issues from Simon review. (4.86 KB, patch)
2011-05-31 16:21 PDT, Viatcheslav Ostapenko 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Viatcheslav Ostapenko
Comment 1 2011-05-31 13:30:52 PDT
Created attachment 95472 [details] Add QGLWidget viewport mode to minibrowser app.
WebKit Review Bot
Comment 2 2011-05-31 13:34:05 PDT
Attachment 95472 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/MiniBrowser/qt/B..." exit_code: 1 Tools/MiniBrowser/qt/BrowserView.cpp:48: Missing space before ( in if( [whitespace/parens] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Viatcheslav Ostapenko
Comment 3 2011-05-31 13:39:08 PDT
Created attachment 95476 [details] Fix style problem
Simon Hausmann
Comment 4 2011-05-31 15:04:26 PDT
Comment on attachment 95476 [details] Fix style problem View in context: https://bugs.webkit.org/attachment.cgi?id=95476&action=review r=me At some point we should probably make that simply the default... > Tools/MiniBrowser/qt/BrowserView.cpp:49 > + setViewport(new QGLWidget()); Nitpick, here you use new GLWidget() and further down just "new QGLWidget" > Tools/MiniBrowser/qt/BrowserView.cpp:76 > +void BrowserView::toggleGLViewport(bool useQGLWidgetViewport) > +{ > +#if defined(QT_CONFIGURED_WITH_OPENGL) > + setViewport(useQGLWidgetViewport ? new QGLWidget : 0); > +#endif > +} setViewport is a public method, so this code doesn't actually have to live in BrowserView but could also be done from the outside.
Viatcheslav Ostapenko
Comment 5 2011-05-31 15:59:18 PDT
(In reply to comment #4) > (From update of attachment 95476 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95476&action=review > > r=me > > At some point we should probably make that simply the default... > > > Tools/MiniBrowser/qt/BrowserView.cpp:49 > > + setViewport(new QGLWidget()); > > Nitpick, here you use new GLWidget() and further down just "new QGLWidget" > > > Tools/MiniBrowser/qt/BrowserView.cpp:76 > > +void BrowserView::toggleGLViewport(bool useQGLWidgetViewport) > > +{ > > +#if defined(QT_CONFIGURED_WITH_OPENGL) > > + setViewport(useQGLWidgetViewport ? new QGLWidget : 0); > > +#endif > > +} > > setViewport is a public method, so this code doesn't actually have to live in BrowserView but could also be done from the outside. Ooops! It's already committed. Should I make those changes as separate patch?
Viatcheslav Ostapenko
Comment 6 2011-05-31 16:00:53 PDT
Removed from commit queue to address some issues mentioned by Simon.
Viatcheslav Ostapenko
Comment 7 2011-05-31 16:21:06 PDT
Created attachment 95504 [details] Fix issues from Simon review.
Andreas Kling
Comment 8 2011-06-01 07:34:44 PDT
Comment on attachment 95504 [details] Fix issues from Simon review. LGTM
WebKit Commit Bot
Comment 9 2011-06-01 10:53:49 PDT
The commit-queue encountered the following flaky tests while processing attachment 95504 [details]: http/tests/websocket/tests/frame-length-longer-than-buffer.html bug 61837 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10 2011-06-01 10:55:20 PDT
Comment on attachment 95504 [details] Fix issues from Simon review. Clearing flags on attachment: 95504 Committed r87824: <http://trac.webkit.org/changeset/87824>
WebKit Commit Bot
Comment 11 2011-06-01 10:55:27 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.